From e7386696b6b6c5ae442ab0a67ea8bdbfc7a4cf1e Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Thu, 28 May 2026 16:28:10 +0000 Subject: [PATCH 01/24] fix(java_opts): preserve quoting and prevent JAVA_OPTS glob expansion (#1301) - Replace xargs-based JAVA_OPTS normalization with newline-only normalization to preserve quotes and backslashes. - Protect user-provided JAVA_OPTS during opts-file eval expansion by using a placeholder and restoring it after eval. - Introduce JavaExecCommand() and switch Java Main/Spring Boot start commands to eval "exec ... $JAVA_OPTS ..." so shell globbing/word-splitting does not corrupt args. - Add regression tests for quoted values, cron/glob expressions, multiple quoted args, backslashes, and start-command invocation behavior. - Clean up overlapping/no-op test code and improve test readability. --- src/java/containers/container.go | 12 +++ src/java/containers/java_main.go | 9 +-- src/java/containers/java_main_test.go | 61 +++++++++++++++ src/java/containers/spring_boot.go | 13 +--- src/java/containers/spring_boot_test.go | 16 ++++ src/java/frameworks/java_opts_writer.go | 18 +++-- src/java/frameworks/java_opts_writer_test.go | 79 +++++++++++++++++--- 7 files changed, 176 insertions(+), 32 deletions(-) diff --git a/src/java/containers/container.go b/src/java/containers/container.go index e16fefe05..84cadd8de 100644 --- a/src/java/containers/container.go +++ b/src/java/containers/container.go @@ -127,6 +127,18 @@ func (r *Registry) RegisterStandardContainers() { r.Register(NewJavaMainContainer(r.context)) } +// JavaExecCommand builds a start command of the form: +// +// eval "exec $JAVA_HOME/bin/java $JAVA_OPTS " +// +// Wrapping the argument to eval in double quotes prevents bash from +// glob-expanding or word-splitting $JAVA_OPTS before eval sees it. +// eval then re-parses the string, honouring any embedded quotes in $JAVA_OPTS. +// javaArgs must be buildpack-generated command fragments (not untrusted input). +func JavaExecCommand(javaArgs string) string { + return `eval "exec $JAVA_HOME/bin/java $JAVA_OPTS ` + javaArgs + `"` +} + // This script is used to process the CLASSPATH assembled from various framework scripts sourced from profile.d // to further create symlinks to the corresponding framework dependencies in WEB-INF/lib, BOOT-INF/lib and where ever // needed thus they are available for application classloading diff --git a/src/java/containers/java_main.go b/src/java/containers/java_main.go index 99570f339..374c8250a 100644 --- a/src/java/containers/java_main.go +++ b/src/java/containers/java_main.go @@ -292,13 +292,11 @@ func (j *JavaMainContainer) Release() (string, error) { // JBP_CONFIG_JAVA_MAIN java_main_class takes precedence over manifest Main-Class. // Use classpath mode so the configured class is actually invoked (not the manifest's). if cfg.JavaMainClass != "" { - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", cfg.JavaMainClass, args), nil + return JavaExecCommand(fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", cfg.JavaMainClass, args)), nil } if j.jarFile != "" { - // JAR has its own Main-Class in the manifest — java -jar handles it - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -jar %s%s", j.jarFile, args), nil + return JavaExecCommand(fmt.Sprintf("-jar %s%s", j.jarFile, args)), nil } // Classpath mode: need an explicit main class @@ -311,6 +309,5 @@ func (j *JavaMainContainer) Release() (string, error) { j.context.Log.Debug("Main Class %s found in JAVA_MAIN_CLASS", mainClass) } - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", mainClass, args), nil + return JavaExecCommand(fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", mainClass, args)), nil } diff --git a/src/java/containers/java_main_test.go b/src/java/containers/java_main_test.go index 2a0d0f0cd..07e798a36 100644 --- a/src/java/containers/java_main_test.go +++ b/src/java/containers/java_main_test.go @@ -290,6 +290,10 @@ var _ = Describe("Java Main Container", func() { }) Describe("buildClasspath", func() { + expectQuotedEval := func(cmd string) { + Expect(cmd).To(MatchRegexp(`eval "exec .*\$JAVA_OPTS`)) + } + Context("with JARs in root and lib/", func() { BeforeEach(func() { os.WriteFile(filepath.Join(buildDir, "app.jar"), []byte("fake"), 0644) @@ -351,6 +355,63 @@ var _ = Describe("Java Main Container", func() { Expect(cmd).To(ContainSubstring(".")) }) }) + + // Regression tests for issue #1301: start command must use eval "exec ... $JAVA_OPTS" + // (quoted string) so that glob chars in JAVA_OPTS are not expanded by bash before eval. + Context("with JAR file (eval quoting)", func() { + BeforeEach(func() { + Expect(createJar( + filepath.Join(buildDir, "app.jar"), + "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", + )).To(Succeed()) + container.Detect() + }) + + It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectQuotedEval(cmd) + }) + }) + + Context("with JAVA_MAIN_CLASS env variable (eval quoting)", func() { + BeforeEach(func() { + os.Setenv("JAVA_MAIN_CLASS", "com.example.Main") + os.WriteFile(filepath.Join(buildDir, "Main.class"), []byte("fake"), 0644) + container.Detect() + }) + + AfterEach(func() { + os.Unsetenv("JAVA_MAIN_CLASS") + }) + + It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectQuotedEval(cmd) + }) + }) + + Context("with JBP_CONFIG_JAVA_MAIN java_main_class (eval quoting)", func() { + BeforeEach(func() { + os.Setenv("JBP_CONFIG_JAVA_MAIN", "{java_main_class: com.example.Main}") + Expect(createJar( + filepath.Join(buildDir, "app.jar"), + "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", + )).To(Succeed()) + container.Detect() + }) + + AfterEach(func() { + os.Unsetenv("JBP_CONFIG_JAVA_MAIN") + }) + + It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectQuotedEval(cmd) + }) + }) }) Describe("Finalize", func() { diff --git a/src/java/containers/spring_boot.go b/src/java/containers/spring_boot.go index ac6119c1c..e7a693c61 100644 --- a/src/java/containers/spring_boot.go +++ b/src/java/containers/spring_boot.go @@ -256,21 +256,16 @@ func (s *SpringBootContainer) Release() (string, error) { // Verify this is actually a Spring Boot application if s.isSpringBootExplodedJar(buildDir) { - // True Spring Boot exploded JAR - use main class from manifest or fallback to JarLauncher based on spring-boot version launcherClass := s.getLauncherClass(buildDir) - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp $PWD/.${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s", launcherClass), nil + return JavaExecCommand(fmt.Sprintf("-cp $PWD/.${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s", launcherClass)), nil } - // Exploded JAR but NOT Spring Boot - use Main-Class from MANIFEST.MF mainClass, err := s.readMainClassFromManifest(buildDir) if err != nil { s.context.Log.Debug("Could not read MANIFEST.MF: %s", err.Error()) } if mainClass != "" { - // Use classpath from BOOT-INF/classes and BOOT-INF/lib - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - return fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS -cp $HOME${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER}:$HOME/BOOT-INF/classes:$HOME/BOOT-INF/lib/* %s", mainClass), nil + return JavaExecCommand(fmt.Sprintf("-cp $HOME${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER}:$HOME/BOOT-INF/classes:$HOME/BOOT-INF/lib/* %s", mainClass)), nil } return "", fmt.Errorf("exploded JAR found but no Main-Class in MANIFEST.MF") @@ -292,8 +287,8 @@ func (s *SpringBootContainer) Release() (string, error) { jarFile = jar } - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - cmd := fmt.Sprintf("eval exec $JAVA_HOME/bin/java $JAVA_OPTS ${CONTAINER_SECURITY_PROVIDER:+-Dloader.path=$CONTAINER_SECURITY_PROVIDER} -jar %s", jarFile) + // Use eval with quoted string to prevent glob-expansion of $JAVA_OPTS (#1301) + cmd := JavaExecCommand(fmt.Sprintf("${CONTAINER_SECURITY_PROVIDER:+-Dloader.path=$CONTAINER_SECURITY_PROVIDER} -jar %s", jarFile)) return cmd, nil } diff --git a/src/java/containers/spring_boot_test.go b/src/java/containers/spring_boot_test.go index 752dd1ec7..f3239a7ce 100644 --- a/src/java/containers/spring_boot_test.go +++ b/src/java/containers/spring_boot_test.go @@ -122,6 +122,10 @@ var _ = Describe("Spring Boot Container", func() { }) Describe("Release", func() { + expectQuotedEval := func(cmd string) { + Expect(cmd).To(MatchRegexp(`eval "exec .*\$JAVA_OPTS`)) + } + Context("with exploded JAR (BOOT-INF)", func() { BeforeEach(func() { os.MkdirAll(filepath.Join(buildDir, "BOOT-INF"), 0755) @@ -136,6 +140,12 @@ var _ = Describe("Spring Boot Container", func() { Expect(err).NotTo(HaveOccurred()) Expect(cmd).To(ContainSubstring("JarLauncher")) }) + + It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectQuotedEval(cmd) + }) }) Context("with Spring Boot JAR", func() { @@ -151,6 +161,12 @@ var _ = Describe("Spring Boot Container", func() { Expect(cmd).To(ContainSubstring("java")) Expect(cmd).To(ContainSubstring("app-boot.jar")) }) + + It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectQuotedEval(cmd) + }) }) Context("with no Spring Boot JAR found", func() { diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index 4bfd0a6d1..2459b355e 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -74,8 +74,8 @@ func CreateJavaOptsAssemblyScript(ctx *common.Context) error { # Save original JAVA_OPTS from environment (user-provided) # Normalize to single line: YAML block scalars (>) may introduce newlines -# xargs trims leading/trailing whitespace and collapses internal spaces -USER_JAVA_OPTS=$(echo "$JAVA_OPTS" | tr '\n' ' ' | tr -s ' ' | xargs) +# Only convert newlines to spaces — do not use xargs which strips quotes and backslashes +USER_JAVA_OPTS=$(echo "$JAVA_OPTS" | tr '\n' ' ') # Start building new JAVA_OPTS JAVA_OPTS="" @@ -86,18 +86,26 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then # Read content and expand runtime variables opts_content=$(cat "$opts_file") - # Expand $DEPS_DIR, $HOME, $JAVA_OPTS using bash parameter expansion. + # Expand $DEPS_DIR and $HOME using bash parameter expansion. # sed-based substitution breaks when these values contain the sed delimiter (|), # backslashes, ampersands, or newlines — all valid in JAVA_OPTS and paths. opts_content="${opts_content//\$DEPS_DIR/$DEPS_DIR}" opts_content="${opts_content//\$HOME/$HOME}" - opts_content="${opts_content//\$JAVA_OPTS/$USER_JAVA_OPTS}" - + + # Shield $JAVA_OPTS from eval: replace with a placeholder first, + # then substitute the actual value AFTER eval so that quotes and + # backslashes in the user-provided JAVA_OPTS are never exposed to eval. + _user_java_opts_placeholder='__JAVA_OPTS_BUILDPACK_PLACEHOLDER__' + opts_content="${opts_content//\$JAVA_OPTS/$_user_java_opts_placeholder}" + # Expand any remaining environment variables in opts content via eval. # Note: eval executes commands, but .opts files are written by the buildpack # at staging time and run within the container context. # This matches how the Ruby buildpack naturally expanded variables via shell. opts_content=$(eval "echo \"$opts_content\"") + + # Now safely substitute JAVA_OPTS after eval (preserves quotes and backslashes) + opts_content="${opts_content//$_user_java_opts_placeholder/$USER_JAVA_OPTS}" if [ -n "$opts_content" ]; then JAVA_OPTS="$JAVA_OPTS $opts_content" diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index e092ec5df..59ed8e3ce 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -53,17 +54,8 @@ var _ = Describe("Java Opts Writer", func() { os.RemoveAll(depsDir) }) - Describe("Basic options", func() { - It("writes JAVA_OPTS correctly", func() { - javaOpts := "-Xmx512M -Xms256M" - os.Setenv("JAVA_OPTS", javaOpts) - - Expect(os.Getenv("JAVA_OPTS")).To(Equal(javaOpts)) - }) - }) - Describe("CreateJavaOptsAssemblyScript", func() { - runScript := func(javaOpts string, optsFileContent string) (string, error) { + setupScript := func(javaOpts string, optsFileContent string) string { err := frameworks.CreateJavaOptsAssemblyScript(ctx) Expect(err).NotTo(HaveOccurred()) @@ -71,8 +63,11 @@ var _ = Describe("Java Opts Writer", func() { Expect(os.MkdirAll(optsDir, 0755)).To(Succeed()) Expect(os.WriteFile(filepath.Join(optsDir, "42_agent.opts"), []byte(optsFileContent), 0644)).To(Succeed()) - scriptPath := filepath.Join(depsDir, "0", "profile.d", "00_java_opts.sh") - cmd := exec.Command("bash", "-c", "source "+scriptPath+" && echo \"$JAVA_OPTS\"") + return filepath.Join(depsDir, "0", "profile.d", "00_java_opts.sh") + } + + runWithEnv := func(scriptPath, javaOpts, bashExpr string) (string, error) { + cmd := exec.Command("bash", "-c", "source "+scriptPath+" && "+bashExpr) cmd.Env = append(os.Environ(), "JAVA_OPTS="+javaOpts, "DEPS_DIR="+depsDir, @@ -82,6 +77,22 @@ var _ = Describe("Java Opts Writer", func() { return string(output), err } + runScript := func(javaOpts string, optsFileContent string) (string, error) { + scriptPath := setupScript(javaOpts, optsFileContent) + return runWithEnv(scriptPath, javaOpts, "echo \"$JAVA_OPTS\"") + } + + // runStartCommand simulates the actual JVM invocation: + // eval "exec $JAVA_HOME/bin/java $JAVA_OPTS -jar app.jar" + // Returns the argument list java would receive (one arg per line). + runStartCommand := func(javaOpts string, optsFileContent string) (string, error) { + scriptPath := setupScript(javaOpts, optsFileContent) + // Simulate: eval "exec java $JAVA_OPTS" — quoted string prevents bash glob-expansion. + // eval then re-parses the string, honouring embedded quotes in $JAVA_OPTS. + return runWithEnv(scriptPath, javaOpts, + `eval "set -- $JAVA_OPTS"; printf '%s\n' "$@"`) + } + It("handles multiline JAVA_OPTS from YAML block scalar without sed error", func() { // Reproduce the manifest pattern: // JAVA_OPTS: > @@ -117,5 +128,49 @@ var _ = Describe("Java Opts Writer", func() { Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) Expect(output).To(ContainSubstring("-Djava.security.properties=" + depsDir + "/0/security.properties")) }) + + // Regression tests for issue #1301: xargs strips quotes, breaking quoted JVM args + It("preserves quoted value with spaces in JAVA_OPTS", func() { + // JAVA_OPTS='-Dfoo="bar baz"' — xargs removes the quotes from USER_JAVA_OPTS, + // so when eval exec java $JAVA_OPTS is called, -Dfoo=bar and baz become separate args + output, err := runScript(`-Dfoo="bar baz"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dfoo="bar baz"`)) + }) + + It("preserves cron expression with glob characters in JAVA_OPTS", func() { + // JAVA_OPTS='-DcronSched="0 */7 * * * *"' — xargs strips quotes, then * expands via glob + // when eval exec java $JAVA_OPTS is invoked, corrupting the cron expression + output, err := runScript(`-DcronSched="0 */7 * * * *"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-DcronSched="0 */7 * * * *"`)) + }) + + It("preserves multiple quoted args in JAVA_OPTS", func() { + // Multiple quoted values — xargs strips all quotes, each space-containing value splits + output, err := runScript(`-Dfoo="bar baz" -Dother="qux quux"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dfoo="bar baz"`)) + Expect(output).To(ContainSubstring(`-Dother="qux quux"`)) + }) + + It("preserves backslashes in JAVA_OPTS values", func() { + // xargs treats backslash as escape char: C:\path\to\app -> C:pathtoapp + // Affects regex patterns and any path using backslash notation + output, err := runScript(`-DregEx="[a-z]+(.*)" -Dpattern=foo\|bar`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-DregEx="[a-z]+(.*)"`)) + Expect(output).To(ContainSubstring(`foo\|bar`)) + }) + + // Full invocation cycle test for issue #1301: + // Verifies that the quoted eval "exec ... $JAVA_OPTS" form delivers the correct + // argument to java — glob chars in $JAVA_OPTS are not expanded. + It("does not glob-expand * in cron expression when invoking java", func() { + output, err := runStartCommand(`-DcronSched="0 */7 * * * *"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + // Java receives exactly one arg: -DcronSched=0 */7 * * * * + Expect(strings.TrimSpace(output)).To(Equal("-DcronSched=0 */7 * * * *")) + }) }) }) From 91fbd552e96a3cb131d07140653b9af02533cf9a Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Fri, 5 Jun 2026 15:28:17 +0000 Subject: [PATCH 02/24] fix(java_opts): use printf for USER_JAVA_OPTS normalization --- src/java/frameworks/java_opts_writer.go | 2 +- src/java/frameworks/java_opts_writer_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index 2459b355e..4e4822048 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -75,7 +75,7 @@ func CreateJavaOptsAssemblyScript(ctx *common.Context) error { # Save original JAVA_OPTS from environment (user-provided) # Normalize to single line: YAML block scalars (>) may introduce newlines # Only convert newlines to spaces — do not use xargs which strips quotes and backslashes -USER_JAVA_OPTS=$(echo "$JAVA_OPTS" | tr '\n' ' ') +USER_JAVA_OPTS=$(printf '%%s' "$JAVA_OPTS" | tr '\n' ' ') # Start building new JAVA_OPTS JAVA_OPTS="" diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index 59ed8e3ce..898144cf5 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -163,6 +163,12 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring(`foo\|bar`)) }) + It("preserves leading -n argument in JAVA_OPTS", func() { + output, err := runScript(`-n -Dfoo=bar`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-n -Dfoo=bar`)) + }) + // Full invocation cycle test for issue #1301: // Verifies that the quoted eval "exec ... $JAVA_OPTS" form delivers the correct // argument to java — glob chars in $JAVA_OPTS are not expanded. From 5503020b618d701896c9c553873b67e796e4ee85 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Fri, 5 Jun 2026 15:32:30 +0000 Subject: [PATCH 03/24] fix(java_opts): use printf in eval expansion path --- src/java/frameworks/java_opts_writer.go | 2 +- src/java/frameworks/java_opts_writer_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index 4e4822048..963730bd5 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -102,7 +102,7 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then # Note: eval executes commands, but .opts files are written by the buildpack # at staging time and run within the container context. # This matches how the Ruby buildpack naturally expanded variables via shell. - opts_content=$(eval "echo \"$opts_content\"") + opts_content=$(eval "printf '%%s' \"$opts_content\"") # Now safely substitute JAVA_OPTS after eval (preserves quotes and backslashes) opts_content="${opts_content//$_user_java_opts_placeholder/$USER_JAVA_OPTS}" diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index 898144cf5..cc748d436 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -79,7 +79,7 @@ var _ = Describe("Java Opts Writer", func() { runScript := func(javaOpts string, optsFileContent string) (string, error) { scriptPath := setupScript(javaOpts, optsFileContent) - return runWithEnv(scriptPath, javaOpts, "echo \"$JAVA_OPTS\"") + return runWithEnv(scriptPath, javaOpts, `printf '%s\n' "$JAVA_OPTS"`) } // runStartCommand simulates the actual JVM invocation: @@ -129,6 +129,12 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring("-Djava.security.properties=" + depsDir + "/0/security.properties")) }) + It("preserves literal -n from opts file content", func() { + output, err := runScript("", "-n") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(strings.TrimSpace(output)).To(Equal("-n")) + }) + // Regression tests for issue #1301: xargs strips quotes, breaking quoted JVM args It("preserves quoted value with spaces in JAVA_OPTS", func() { // JAVA_OPTS='-Dfoo="bar baz"' — xargs removes the quotes from USER_JAVA_OPTS, From 083de4e554113a546bba35eb0260f71e6fb36f71 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Fri, 5 Jun 2026 15:36:18 +0000 Subject: [PATCH 04/24] fix(java_opts): escape replacement chars in placeholder substitution --- src/java/frameworks/java_opts_writer.go | 6 ++++-- src/java/frameworks/java_opts_writer_test.go | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index 963730bd5..d207a70ec 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -104,8 +104,10 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then # This matches how the Ruby buildpack naturally expanded variables via shell. opts_content=$(eval "printf '%%s' \"$opts_content\"") - # Now safely substitute JAVA_OPTS after eval (preserves quotes and backslashes) - opts_content="${opts_content//$_user_java_opts_placeholder/$USER_JAVA_OPTS}" + # Now safely substitute JAVA_OPTS after eval (preserves quotes, backslashes, and ampersands) + _escaped_user_java_opts="${USER_JAVA_OPTS//\\/\\\\}" + _escaped_user_java_opts="${_escaped_user_java_opts//&/\\&}" + opts_content="${opts_content//$_user_java_opts_placeholder/$_escaped_user_java_opts}" if [ -n "$opts_content" ]; then JAVA_OPTS="$JAVA_OPTS $opts_content" diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index cc748d436..86dcb3f5c 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -175,6 +175,18 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring(`-n -Dfoo=bar`)) }) + It("preserves ampersand in JAVA_OPTS values during placeholder substitution", func() { + output, err := runScript(`-Dfoo=a&b`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dfoo=a&b`)) + }) + + It("preserves backslashes in JAVA_OPTS values during placeholder substitution", func() { + output, err := runScript(`-Dpath=C:\tmp\app`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dpath=C:\tmp\app`)) + }) + // Full invocation cycle test for issue #1301: // Verifies that the quoted eval "exec ... $JAVA_OPTS" form delivers the correct // argument to java — glob chars in $JAVA_OPTS are not expanded. From 7183dc8ecb1874a253ca2eead327dc7b6505a04d Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Fri, 5 Jun 2026 15:50:58 +0000 Subject: [PATCH 05/24] test/docs: clarify JavaExecCommand comment and align Release test placement (#1301) --- src/java/containers/container.go | 3 +- src/java/containers/java_main_test.go | 121 +++++++++++++------------- 2 files changed, 63 insertions(+), 61 deletions(-) diff --git a/src/java/containers/container.go b/src/java/containers/container.go index 84cadd8de..06a7b84b7 100644 --- a/src/java/containers/container.go +++ b/src/java/containers/container.go @@ -134,7 +134,8 @@ func (r *Registry) RegisterStandardContainers() { // Wrapping the argument to eval in double quotes prevents bash from // glob-expanding or word-splitting $JAVA_OPTS before eval sees it. // eval then re-parses the string, honouring any embedded quotes in $JAVA_OPTS. -// javaArgs must be buildpack-generated command fragments (not untrusted input). +// javaArgs may include app/config-derived values (e.g. jar paths, Main-Class, +// configured arguments), so callers must ensure it is shell-safe. func JavaExecCommand(javaArgs string) string { return `eval "exec $JAVA_HOME/bin/java $JAVA_OPTS ` + javaArgs + `"` } diff --git a/src/java/containers/java_main_test.go b/src/java/containers/java_main_test.go index 07e798a36..ebd094507 100644 --- a/src/java/containers/java_main_test.go +++ b/src/java/containers/java_main_test.go @@ -154,6 +154,10 @@ var _ = Describe("Java Main Container", func() { }) Describe("Release", func() { + expectQuotedEval := func(cmd string) { + Expect(cmd).To(MatchRegexp(`eval "exec .*\$JAVA_OPTS`)) + } + Context("with JAR file", func() { BeforeEach(func() { Expect(createJar( @@ -280,6 +284,63 @@ var _ = Describe("Java Main Container", func() { }) }) + // Regression tests for issue #1301: start command must use eval "exec ... $JAVA_OPTS" + // (quoted string) so that glob chars in JAVA_OPTS are not expanded by bash before eval. + Context("with JAR file (eval quoting)", func() { + BeforeEach(func() { + Expect(createJar( + filepath.Join(buildDir, "app.jar"), + "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", + )).To(Succeed()) + container.Detect() + }) + + It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectQuotedEval(cmd) + }) + }) + + Context("with JAVA_MAIN_CLASS env variable (eval quoting)", func() { + BeforeEach(func() { + os.Setenv("JAVA_MAIN_CLASS", "com.example.Main") + os.WriteFile(filepath.Join(buildDir, "Main.class"), []byte("fake"), 0644) + container.Detect() + }) + + AfterEach(func() { + os.Unsetenv("JAVA_MAIN_CLASS") + }) + + It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectQuotedEval(cmd) + }) + }) + + Context("with JBP_CONFIG_JAVA_MAIN java_main_class (eval quoting)", func() { + BeforeEach(func() { + os.Setenv("JBP_CONFIG_JAVA_MAIN", "{java_main_class: com.example.Main}") + Expect(createJar( + filepath.Join(buildDir, "app.jar"), + "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", + )).To(Succeed()) + container.Detect() + }) + + AfterEach(func() { + os.Unsetenv("JBP_CONFIG_JAVA_MAIN") + }) + + It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + expectQuotedEval(cmd) + }) + }) + Context("without main class or JAR", func() { It("returns error", func() { _, err := container.Release() @@ -290,10 +351,6 @@ var _ = Describe("Java Main Container", func() { }) Describe("buildClasspath", func() { - expectQuotedEval := func(cmd string) { - Expect(cmd).To(MatchRegexp(`eval "exec .*\$JAVA_OPTS`)) - } - Context("with JARs in root and lib/", func() { BeforeEach(func() { os.WriteFile(filepath.Join(buildDir, "app.jar"), []byte("fake"), 0644) @@ -356,62 +413,6 @@ var _ = Describe("Java Main Container", func() { }) }) - // Regression tests for issue #1301: start command must use eval "exec ... $JAVA_OPTS" - // (quoted string) so that glob chars in JAVA_OPTS are not expanded by bash before eval. - Context("with JAR file (eval quoting)", func() { - BeforeEach(func() { - Expect(createJar( - filepath.Join(buildDir, "app.jar"), - "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", - )).To(Succeed()) - container.Detect() - }) - - It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { - cmd, err := container.Release() - Expect(err).NotTo(HaveOccurred()) - expectQuotedEval(cmd) - }) - }) - - Context("with JAVA_MAIN_CLASS env variable (eval quoting)", func() { - BeforeEach(func() { - os.Setenv("JAVA_MAIN_CLASS", "com.example.Main") - os.WriteFile(filepath.Join(buildDir, "Main.class"), []byte("fake"), 0644) - container.Detect() - }) - - AfterEach(func() { - os.Unsetenv("JAVA_MAIN_CLASS") - }) - - It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { - cmd, err := container.Release() - Expect(err).NotTo(HaveOccurred()) - expectQuotedEval(cmd) - }) - }) - - Context("with JBP_CONFIG_JAVA_MAIN java_main_class (eval quoting)", func() { - BeforeEach(func() { - os.Setenv("JBP_CONFIG_JAVA_MAIN", "{java_main_class: com.example.Main}") - Expect(createJar( - filepath.Join(buildDir, "app.jar"), - "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", - )).To(Succeed()) - container.Detect() - }) - - AfterEach(func() { - os.Unsetenv("JBP_CONFIG_JAVA_MAIN") - }) - - It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { - cmd, err := container.Release() - Expect(err).NotTo(HaveOccurred()) - expectQuotedEval(cmd) - }) - }) }) Describe("Finalize", func() { From 9741147d8edc90fdfed2297342a77ce29a1777d5 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Fri, 5 Jun 2026 17:40:24 +0000 Subject: [PATCH 06/24] refactor(java_opts): hoist invariant escaping and add double-backslash regression test --- src/java/frameworks/java_opts_writer.go | 20 ++++++++++------ src/java/frameworks/java_opts_writer_test.go | 25 ++++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index d207a70ec..f45a742a8 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -80,6 +80,15 @@ USER_JAVA_OPTS=$(printf '%%s' "$JAVA_OPTS" | tr '\n' ' ') # Start building new JAVA_OPTS JAVA_OPTS="" +# Escape replacement-special chars once; these values are loop-invariant. +_escaped_deps_dir="${DEPS_DIR//\\/\\\\}" +_escaped_deps_dir="${_escaped_deps_dir//&/\\&}" +_escaped_home="${HOME//\\/\\\\}" +_escaped_home="${_escaped_home//&/\\&}" +_user_java_opts_placeholder='__JAVA_OPTS_BUILDPACK_PLACEHOLDER__' +_escaped_user_java_opts="${USER_JAVA_OPTS//\\/\\\\}" +_escaped_user_java_opts="${_escaped_user_java_opts//&/\\&}" + if [ -d "$DEPS_DIR/%s/java_opts" ]; then for opts_file in "$DEPS_DIR/%s/java_opts"/*.opts; do if [ -f "$opts_file" ]; then @@ -87,15 +96,14 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then opts_content=$(cat "$opts_file") # Expand $DEPS_DIR and $HOME using bash parameter expansion. - # sed-based substitution breaks when these values contain the sed delimiter (|), - # backslashes, ampersands, or newlines — all valid in JAVA_OPTS and paths. - opts_content="${opts_content//\$DEPS_DIR/$DEPS_DIR}" - opts_content="${opts_content//\$HOME/$HOME}" + # In ${var//pattern/repl}, '&' and '\' are special in replacement strings, + # so escape them first to preserve literal path contents. + opts_content="${opts_content//\$DEPS_DIR/$_escaped_deps_dir}" + opts_content="${opts_content//\$HOME/$_escaped_home}" # Shield $JAVA_OPTS from eval: replace with a placeholder first, # then substitute the actual value AFTER eval so that quotes and # backslashes in the user-provided JAVA_OPTS are never exposed to eval. - _user_java_opts_placeholder='__JAVA_OPTS_BUILDPACK_PLACEHOLDER__' opts_content="${opts_content//\$JAVA_OPTS/$_user_java_opts_placeholder}" # Expand any remaining environment variables in opts content via eval. @@ -105,8 +113,6 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then opts_content=$(eval "printf '%%s' \"$opts_content\"") # Now safely substitute JAVA_OPTS after eval (preserves quotes, backslashes, and ampersands) - _escaped_user_java_opts="${USER_JAVA_OPTS//\\/\\\\}" - _escaped_user_java_opts="${_escaped_user_java_opts//&/\\&}" opts_content="${opts_content//$_user_java_opts_placeholder/$_escaped_user_java_opts}" if [ -n "$opts_content" ]; then diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index 86dcb3f5c..5a648d53d 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -77,6 +77,17 @@ var _ = Describe("Java Opts Writer", func() { return string(output), err } + runWithCustomRuntimeEnv := func(scriptPath, javaOpts, bashExpr, runtimeDepsDir, runtimeHome string) (string, error) { + cmd := exec.Command("bash", "-c", "source "+scriptPath+" && "+bashExpr) + cmd.Env = append(os.Environ(), + "JAVA_OPTS="+javaOpts, + "DEPS_DIR="+runtimeDepsDir, + "HOME="+runtimeHome, + ) + output, err := cmd.CombinedOutput() + return string(output), err + } + runScript := func(javaOpts string, optsFileContent string) (string, error) { scriptPath := setupScript(javaOpts, optsFileContent) return runWithEnv(scriptPath, javaOpts, `printf '%s\n' "$JAVA_OPTS"`) @@ -123,6 +134,14 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring("-javaagent:/home/vcap/app/BOOT-INF/lib/agent.jar")) }) + It("preserves ampersand and backslash in $HOME replacement", func() { + scriptPath := setupScript("", "-javaagent:$HOME/BOOT-INF/lib/agent.jar") + customHome := `/tmp/home&dir\sub` + output, err := runWithCustomRuntimeEnv(scriptPath, "", `printf '%s\n' "$JAVA_OPTS"`, depsDir, customHome) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-javaagent:" + customHome + "/BOOT-INF/lib/agent.jar")) + }) + It("expands $DEPS_DIR in opts file content", func() { output, err := runScript("", "-Djava.security.properties=$DEPS_DIR/0/security.properties") Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) @@ -187,6 +206,12 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring(`-Dpath=C:\tmp\app`)) }) + It("preserves double backslashes in JAVA_OPTS values during placeholder substitution", func() { + output, err := runScript(`-Dpath=C:\\double`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dpath=C:\\double`)) + }) + // Full invocation cycle test for issue #1301: // Verifies that the quoted eval "exec ... $JAVA_OPTS" form delivers the correct // argument to java — glob chars in $JAVA_OPTS are not expanded. From edbf42d3027cfd0d5843a93931e5b92f3274cee9 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Fri, 5 Jun 2026 17:41:38 +0000 Subject: [PATCH 07/24] fix(play): quote eval java command for JAVA_OPTS safety (#1301) --- src/java/containers/play.go | 4 ++-- src/java/containers/play_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/java/containers/play.go b/src/java/containers/play.go index 2dc37717a..3001c6d7b 100644 --- a/src/java/containers/play.go +++ b/src/java/containers/play.go @@ -548,8 +548,8 @@ func (p *PlayContainer) Release() (string, error) { libPath = filepath.ToSlash(relPath) } } - // Use eval to properly handle backslash-escaped values in $JAVA_OPTS (Ruby buildpack parity) - cmd = fmt.Sprintf("eval exec java $JAVA_OPTS -cp $HOME/%s/* play.core.server.NettyServer $HOME", libPath) + // Use quoted eval argument to prevent glob/word-splitting of $JAVA_OPTS before eval. + cmd = fmt.Sprintf(`eval "exec java $JAVA_OPTS -cp $HOME/%s/* play.core.server.NettyServer $HOME"`, libPath) } p.context.Log.Debug("Play Framework release command: %s", cmd) diff --git a/src/java/containers/play_test.go b/src/java/containers/play_test.go index d4d225434..6d758afe3 100644 --- a/src/java/containers/play_test.go +++ b/src/java/containers/play_test.go @@ -185,7 +185,7 @@ var _ = Describe("Play Container", func() { It("returns correct java command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - Expect(cmd).To(ContainSubstring("java $JAVA_OPTS -cp")) + Expect(cmd).To(ContainSubstring(`eval "exec java $JAVA_OPTS -cp`)) Expect(cmd).To(ContainSubstring("play.core.server.NettyServer $HOME")) }) }) @@ -201,7 +201,7 @@ var _ = Describe("Play Container", func() { It("returns correct java command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - Expect(cmd).To(ContainSubstring("java $JAVA_OPTS -cp")) + Expect(cmd).To(ContainSubstring(`eval "exec java $JAVA_OPTS -cp`)) Expect(cmd).To(ContainSubstring("play.core.server.NettyServer $HOME")) }) }) From 0d38e925b405fb903730e0506e41f84e73dcf571 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Sat, 6 Jun 2026 09:28:42 +0000 Subject: [PATCH 08/24] Replace eval with pure-bash expander for .opts variable expansion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The profile.d assembly script expanded remaining environment variables in framework .opts file content using eval, which also executes embedded command substitutions ($(...) / backticks) — an unnecessary code-execution surface. Replace eval with a dependency-free pure-bash expander that expands only $VAR / ${VAR} references and never executes command substitutions. envsubst is not used because gettext-base is not present on the cflinuxfs stacks (verified: envsubst not found at runtime). The buildpack itself emits one trusted command substitution into .opts content: -XX:ActiveProcessorCount=$(nproc) (jres/openjdk.go). Resolve this single known token by computing the processor count once in the runtime profile.d script (so nproc reflects the running app container's CPUs, as the old eval did) and substituting it literally, so the JVM still receives a numeric value. Any other $(...) or backtick that survives expansion is buildpack-emitted and would reach the JVM literally, so emit a warning to surface such a regression. Expansion remains single-pass to match the previous eval behavior. Add tests for arbitrary runtime variable expansion, command substitutions being preserved literally rather than executed, the $(nproc) resolution, and the unresolved-substitution warning. --- src/java/frameworks/java_opts_writer.go | 68 +++++++++++++++++--- src/java/frameworks/java_opts_writer_test.go | 40 ++++++++++++ 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index f45a742a8..0d72b57ac 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -80,6 +80,39 @@ USER_JAVA_OPTS=$(printf '%%s' "$JAVA_OPTS" | tr '\n' ' ') # Start building new JAVA_OPTS JAVA_OPTS="" +# Expand $VAR and ${VAR} references in a string using only bash builtins. +# Unlike eval, this NEVER executes command substitutions ($(...) or backticks); +# only environment-variable references are expanded. It is also dependency-free +# (envsubst from gettext-base is not available on the cflinuxfs stacks). +# Expansion is single-pass: substituted values are not re-scanned for further +# references, matching the previous eval-based behavior. +_expand_env_vars() { + local s="$1" out="" name + while [[ "$s" =~ ^([^$]*)\$(\{([A-Za-z_][A-Za-z0-9_]*)\}|([A-Za-z_][A-Za-z0-9_]*))? ]]; do + out+="${BASH_REMATCH[1]}" + name="${BASH_REMATCH[3]}${BASH_REMATCH[4]}" + if [ -n "$name" ]; then + out+="${!name}" + else + out+='$' + fi + s="${s#"${BASH_REMATCH[0]}"}" + done + out+="$s" + printf '%%s' "$out" +} + +# The buildpack itself emits the literal command substitution $(nproc) in its +# base JAVA_OPTS (e.g. -XX:ActiveProcessorCount=$(nproc)). The pure-bash +# expander deliberately does not execute command substitutions, so resolve this +# single known, trusted token here by computing the processor count once and +# substituting it literally below. All other command substitutions remain +# literal (and therefore inert). +_nproc_value="$(nproc 2>/dev/null || echo 1)" +_nproc_token='$(nproc)' +# Backtick char, computed here because this script lives inside a Go raw string. +_backtick="$(printf '\140')" + # Escape replacement-special chars once; these values are loop-invariant. _escaped_deps_dir="${DEPS_DIR//\\/\\\\}" _escaped_deps_dir="${_escaped_deps_dir//&/\\&}" @@ -101,18 +134,33 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then opts_content="${opts_content//\$DEPS_DIR/$_escaped_deps_dir}" opts_content="${opts_content//\$HOME/$_escaped_home}" - # Shield $JAVA_OPTS from eval: replace with a placeholder first, - # then substitute the actual value AFTER eval so that quotes and - # backslashes in the user-provided JAVA_OPTS are never exposed to eval. - opts_content="${opts_content//\$JAVA_OPTS/$_user_java_opts_placeholder}" + # Resolve the trusted $(nproc) token to the computed processor count. + opts_content="${opts_content//$_nproc_token/$_nproc_value}" - # Expand any remaining environment variables in opts content via eval. - # Note: eval executes commands, but .opts files are written by the buildpack - # at staging time and run within the container context. - # This matches how the Ruby buildpack naturally expanded variables via shell. - opts_content=$(eval "printf '%%s' \"$opts_content\"") + # Shield $JAVA_OPTS from expansion: replace with a placeholder first, + # then substitute the actual value AFTER expansion so that quotes and + # backslashes in the user-provided JAVA_OPTS are never reinterpreted. + opts_content="${opts_content//\$JAVA_OPTS/$_user_java_opts_placeholder}" - # Now safely substitute JAVA_OPTS after eval (preserves quotes, backslashes, and ampersands) + # Expand any remaining environment variables in opts content. + # Use a pure-bash expander instead of eval so that command + # substitutions like $(...) or backticks in .opts content are + # NOT executed — only $VAR / ${VAR} references are expanded. + opts_content=$(_expand_env_vars "$opts_content") + + # Defense-in-depth: the buildpack resolves its only known command + # substitution ($(nproc)) above. Any remaining $(...) or backtick at + # this point is an unresolved buildpack-emitted substitution (the + # user's JAVA_OPTS is still a placeholder here) and would reach the + # JVM literally. Warn so such a regression is caught instead of + # silently producing a broken argument. + case "$opts_content" in + *'$('*|*"$_backtick"*) + echo "WARNING: unresolved command substitution in $opts_file; it will be passed to the JVM literally: $opts_content" >&2 + ;; + esac + + # Now safely substitute JAVA_OPTS after expansion (preserves quotes, backslashes, and ampersands) opts_content="${opts_content//$_user_java_opts_placeholder/$_escaped_user_java_opts}" if [ -n "$opts_content" ]; then diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index 5a648d53d..a2f01e927 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -148,6 +148,46 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring("-Djava.security.properties=" + depsDir + "/0/security.properties")) }) + It("expands arbitrary runtime environment variables in opts file content", func() { + os.Setenv("MY_RUNTIME_VAR", "512m") + defer os.Unsetenv("MY_RUNTIME_VAR") + output, err := runScript("", "-Dmem=$MY_RUNTIME_VAR -Dother=${MY_RUNTIME_VAR}") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-Dmem=512m -Dother=512m")) + }) + + It("resolves the trusted $(nproc) token to a processor count", func() { + // The buildpack emits -XX:ActiveProcessorCount=$(nproc); it must be + // resolved to a number even though arbitrary command substitutions + // are not executed. + output, err := runScript("", "-XX:ActiveProcessorCount=$(nproc)") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(MatchRegexp(`-XX:ActiveProcessorCount=[0-9]+`)) + Expect(output).NotTo(ContainSubstring("nproc")) + }) + + It("warns when an unresolved command substitution survives in opts content", func() { + // A buildpack-emitted $(...) or backtick other than $(nproc) is not + // executed and would reach the JVM literally; the script must warn. + output, err := runScript("", "-Dwhen=$(date)") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("WARNING: unresolved command substitution")) + + output, err = runScript("", "-Dwhen=`date`") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("WARNING: unresolved command substitution")) + }) + + It("does not execute command substitutions embedded in opts file content", func() { + marker := filepath.Join(cacheDir, "pwned_marker") + Expect(os.Remove(marker)).To(Or(Succeed(), MatchError(os.ErrNotExist))) + output, err := runScript("", "-Dx=$(touch "+marker+")") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + _, statErr := os.Stat(marker) + Expect(os.IsNotExist(statErr)).To(BeTrue(), "command substitution was executed; marker file created") + Expect(output).To(ContainSubstring("$(touch")) + }) + It("preserves literal -n from opts file content", func() { output, err := runScript("", "-n") Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) From 87c3e45c2041bdec26b0d827e95c790112a926bb Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Sat, 6 Jun 2026 10:19:06 +0000 Subject: [PATCH 09/24] Replace start-command eval with a shell-free javaexec launcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The container start commands invoked the JVM via eval "exec $JAVA_HOME/bin/java $JAVA_OPTS " so the shell re-parsed JAVA_OPTS, executing any embedded command substitutions and treating shell metacharacters as operators — a code-execution surface and a source of quoting bugs (issue #1301). Introduce a small Go launcher (src/java/javaexec) that tokenizes JAVA_OPTS with POSIX word-splitting and quote removal only — never expanding variables, globs, or command substitutions — then syscall.Exec's java in place. The start command becomes exec $DEPS_DIR//bin/javaexec "$JAVA_HOME/bin/java" where JAVA_OPTS is read from the environment by the launcher, so user-provided options can never be re-interpreted by a shell. Trusted, buildpack-built args (classpath, globs, $PWD) remain on the normal command line and expand as before. The finalize step installs the launcher binary into the dependency bin dir; it now hard-fails if the binary is missing so a packaging error is caught at staging rather than at launch. The binary is a build artifact (built by scripts/build.sh) and is gitignored. Covers java_main, spring_boot and play containers. --- .gitignore | 1 + docs/framework-ordering.md | 10 +++ src/java/containers/container.go | 21 +++-- src/java/containers/java_main.go | 6 +- src/java/containers/java_main_test.go | 25 +++--- src/java/containers/play.go | 5 +- src/java/containers/play_test.go | 8 +- src/java/containers/spring_boot.go | 8 +- src/java/containers/spring_boot_test.go | 15 ++-- src/java/finalize/cli/main.go | 1 + src/java/finalize/finalize.go | 47 ++++++++++++ src/java/finalize/finalize_test.go | 16 ++-- src/java/frameworks/java_opts_writer_test.go | 21 +++-- src/java/javaexec/args.go | 15 ++++ src/java/javaexec/args_test.go | 23 ++++++ src/java/javaexec/cli/main.go | 48 ++++++++++++ src/java/javaexec/tokenize.go | 80 ++++++++++++++++++++ src/java/javaexec/tokenize_test.go | 67 ++++++++++++++++ 18 files changed, 369 insertions(+), 48 deletions(-) create mode 100644 src/java/javaexec/args.go create mode 100644 src/java/javaexec/args_test.go create mode 100644 src/java/javaexec/cli/main.go create mode 100644 src/java/javaexec/tokenize.go create mode 100644 src/java/javaexec/tokenize_test.go diff --git a/.gitignore b/.gitignore index 6cc6ac3ff..4c02109d3 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ tmp/ .cache/ bin/detect bin/finalize +bin/javaexec bin/release bin/supply /*.md diff --git a/docs/framework-ordering.md b/docs/framework-ordering.md index 16537a162..d6158fffb 100644 --- a/docs/framework-ordering.md +++ b/docs/framework-ordering.md @@ -94,6 +94,16 @@ This ensures: 2. **Container Security Provider runs BEFORE JRebel** (07 < 20) 3. **User JAVA_OPTS override everything** (99 runs last) +> **Note (safe expansion):** the snippet above is simplified. The real +> `00_java_opts.sh` does **not** use `eval`. It expands only `$VAR` / `${VAR}` +> references in `.opts` content via a pure-bash expander, so embedded command +> substitutions (`$(...)`, backticks) are never executed. The one trusted +> substitution the buildpack emits, `-XX:ActiveProcessorCount=$(nproc)`, is +> resolved explicitly at runtime; any other surviving `$(...)` triggers a +> warning. At launch the JVM is started through the shell-free `javaexec` +> launcher (`$DEPS_DIR//bin/javaexec`), which tokenizes `JAVA_OPTS` +> without re-invoking a shell, rather than `eval "exec java $JAVA_OPTS"`. + ## Critical Ordering Dependencies ### Container Security Provider (Priority 17, Line 51) diff --git a/src/java/containers/container.go b/src/java/containers/container.go index 06a7b84b7..9f40bb800 100644 --- a/src/java/containers/container.go +++ b/src/java/containers/container.go @@ -129,15 +129,20 @@ func (r *Registry) RegisterStandardContainers() { // JavaExecCommand builds a start command of the form: // -// eval "exec $JAVA_HOME/bin/java $JAVA_OPTS " +// exec $DEPS_DIR//bin/javaexec "$JAVA_HOME/bin/java" // -// Wrapping the argument to eval in double quotes prevents bash from -// glob-expanding or word-splitting $JAVA_OPTS before eval sees it. -// eval then re-parses the string, honouring any embedded quotes in $JAVA_OPTS. -// javaArgs may include app/config-derived values (e.g. jar paths, Main-Class, -// configured arguments), so callers must ensure it is shell-safe. -func JavaExecCommand(javaArgs string) string { - return `eval "exec $JAVA_HOME/bin/java $JAVA_OPTS ` + javaArgs + `"` +// The javaexec launcher reads JAVA_OPTS from the environment and tokenizes it +// without a shell, so glob characters, quotes, and shell metacharacters in +// user-provided JAVA_OPTS are never expanded or executed. This replaces the +// previous `eval "exec $JAVA_HOME/bin/java $JAVA_OPTS "` form, which +// let the shell glob-expand $JAVA_OPTS and execute embedded command +// substitutions. +// +// javaArgs are buildpack-generated (jar paths, Main-Class, classpath, and +// shell expansions like ${CLASSPATH} or BOOT-INF/lib/*) and are placed on a +// normal command line, where the shell expands them exactly as before. +func JavaExecCommand(depsIdx, javaArgs string) string { + return `exec $DEPS_DIR/` + depsIdx + `/bin/javaexec "$JAVA_HOME/bin/java" ` + javaArgs } // This script is used to process the CLASSPATH assembled from various framework scripts sourced from profile.d diff --git a/src/java/containers/java_main.go b/src/java/containers/java_main.go index 374c8250a..e4547e688 100644 --- a/src/java/containers/java_main.go +++ b/src/java/containers/java_main.go @@ -292,11 +292,11 @@ func (j *JavaMainContainer) Release() (string, error) { // JBP_CONFIG_JAVA_MAIN java_main_class takes precedence over manifest Main-Class. // Use classpath mode so the configured class is actually invoked (not the manifest's). if cfg.JavaMainClass != "" { - return JavaExecCommand(fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", cfg.JavaMainClass, args)), nil + return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", cfg.JavaMainClass, args)), nil } if j.jarFile != "" { - return JavaExecCommand(fmt.Sprintf("-jar %s%s", j.jarFile, args)), nil + return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-jar %s%s", j.jarFile, args)), nil } // Classpath mode: need an explicit main class @@ -309,5 +309,5 @@ func (j *JavaMainContainer) Release() (string, error) { j.context.Log.Debug("Main Class %s found in JAVA_MAIN_CLASS", mainClass) } - return JavaExecCommand(fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", mainClass, args)), nil + return JavaExecCommand(j.context.Stager.DepsIdx(), fmt.Sprintf("-cp ${CLASSPATH}${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s%s", mainClass, args)), nil } diff --git a/src/java/containers/java_main_test.go b/src/java/containers/java_main_test.go index ebd094507..7c115a204 100644 --- a/src/java/containers/java_main_test.go +++ b/src/java/containers/java_main_test.go @@ -154,8 +154,11 @@ var _ = Describe("Java Main Container", func() { }) Describe("Release", func() { - expectQuotedEval := func(cmd string) { - Expect(cmd).To(MatchRegexp(`eval "exec .*\$JAVA_OPTS`)) + expectSafeLaunch := func(cmd string) { + Expect(cmd).To(ContainSubstring("/bin/javaexec")) + Expect(cmd).To(ContainSubstring(`"$JAVA_HOME/bin/java"`)) + Expect(cmd).NotTo(ContainSubstring("eval")) + Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS")) } Context("with JAR file", func() { @@ -286,7 +289,7 @@ var _ = Describe("Java Main Container", func() { // Regression tests for issue #1301: start command must use eval "exec ... $JAVA_OPTS" // (quoted string) so that glob chars in JAVA_OPTS are not expanded by bash before eval. - Context("with JAR file (eval quoting)", func() { + Context("with JAR file (javaexec launch)", func() { BeforeEach(func() { Expect(createJar( filepath.Join(buildDir, "app.jar"), @@ -295,14 +298,14 @@ var _ = Describe("Java Main Container", func() { container.Detect() }) - It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - expectQuotedEval(cmd) + expectSafeLaunch(cmd) }) }) - Context("with JAVA_MAIN_CLASS env variable (eval quoting)", func() { + Context("with JAVA_MAIN_CLASS env variable (javaexec launch)", func() { BeforeEach(func() { os.Setenv("JAVA_MAIN_CLASS", "com.example.Main") os.WriteFile(filepath.Join(buildDir, "Main.class"), []byte("fake"), 0644) @@ -313,14 +316,14 @@ var _ = Describe("Java Main Container", func() { os.Unsetenv("JAVA_MAIN_CLASS") }) - It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - expectQuotedEval(cmd) + expectSafeLaunch(cmd) }) }) - Context("with JBP_CONFIG_JAVA_MAIN java_main_class (eval quoting)", func() { + Context("with JBP_CONFIG_JAVA_MAIN java_main_class (javaexec launch)", func() { BeforeEach(func() { os.Setenv("JBP_CONFIG_JAVA_MAIN", "{java_main_class: com.example.Main}") Expect(createJar( @@ -334,10 +337,10 @@ var _ = Describe("Java Main Container", func() { os.Unsetenv("JBP_CONFIG_JAVA_MAIN") }) - It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - expectQuotedEval(cmd) + expectSafeLaunch(cmd) }) }) diff --git a/src/java/containers/play.go b/src/java/containers/play.go index 3001c6d7b..c2860158d 100644 --- a/src/java/containers/play.go +++ b/src/java/containers/play.go @@ -548,8 +548,9 @@ func (p *PlayContainer) Release() (string, error) { libPath = filepath.ToSlash(relPath) } } - // Use quoted eval argument to prevent glob/word-splitting of $JAVA_OPTS before eval. - cmd = fmt.Sprintf(`eval "exec java $JAVA_OPTS -cp $HOME/%s/* play.core.server.NettyServer $HOME"`, libPath) + // JAVA_OPTS is applied by the javaexec launcher, which tokenizes it + // without a shell (no glob/word-splitting/command execution). + cmd = fmt.Sprintf(`exec $DEPS_DIR/%s/bin/javaexec "$JAVA_HOME/bin/java" -cp $HOME/%s/* play.core.server.NettyServer $HOME`, p.context.Stager.DepsIdx(), libPath) } p.context.Log.Debug("Play Framework release command: %s", cmd) diff --git a/src/java/containers/play_test.go b/src/java/containers/play_test.go index 6d758afe3..29dc76e5e 100644 --- a/src/java/containers/play_test.go +++ b/src/java/containers/play_test.go @@ -185,7 +185,9 @@ var _ = Describe("Play Container", func() { It("returns correct java command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - Expect(cmd).To(ContainSubstring(`eval "exec java $JAVA_OPTS -cp`)) + Expect(cmd).To(ContainSubstring(`/bin/javaexec "$JAVA_HOME/bin/java" -cp`)) + Expect(cmd).NotTo(ContainSubstring("eval")) + Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS")) Expect(cmd).To(ContainSubstring("play.core.server.NettyServer $HOME")) }) }) @@ -201,7 +203,9 @@ var _ = Describe("Play Container", func() { It("returns correct java command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - Expect(cmd).To(ContainSubstring(`eval "exec java $JAVA_OPTS -cp`)) + Expect(cmd).To(ContainSubstring(`/bin/javaexec "$JAVA_HOME/bin/java" -cp`)) + Expect(cmd).NotTo(ContainSubstring("eval")) + Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS")) Expect(cmd).To(ContainSubstring("play.core.server.NettyServer $HOME")) }) }) diff --git a/src/java/containers/spring_boot.go b/src/java/containers/spring_boot.go index e7a693c61..b584973a7 100644 --- a/src/java/containers/spring_boot.go +++ b/src/java/containers/spring_boot.go @@ -257,7 +257,7 @@ func (s *SpringBootContainer) Release() (string, error) { if s.isSpringBootExplodedJar(buildDir) { launcherClass := s.getLauncherClass(buildDir) - return JavaExecCommand(fmt.Sprintf("-cp $PWD/.${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s", launcherClass)), nil + return JavaExecCommand(s.context.Stager.DepsIdx(), fmt.Sprintf("-cp $PWD/.${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER} %s", launcherClass)), nil } mainClass, err := s.readMainClassFromManifest(buildDir) @@ -265,7 +265,7 @@ func (s *SpringBootContainer) Release() (string, error) { s.context.Log.Debug("Could not read MANIFEST.MF: %s", err.Error()) } if mainClass != "" { - return JavaExecCommand(fmt.Sprintf("-cp $HOME${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER}:$HOME/BOOT-INF/classes:$HOME/BOOT-INF/lib/* %s", mainClass)), nil + return JavaExecCommand(s.context.Stager.DepsIdx(), fmt.Sprintf("-cp $HOME${CONTAINER_SECURITY_PROVIDER:+:$CONTAINER_SECURITY_PROVIDER}:$HOME/BOOT-INF/classes:$HOME/BOOT-INF/lib/* %s", mainClass)), nil } return "", fmt.Errorf("exploded JAR found but no Main-Class in MANIFEST.MF") @@ -287,8 +287,8 @@ func (s *SpringBootContainer) Release() (string, error) { jarFile = jar } - // Use eval with quoted string to prevent glob-expansion of $JAVA_OPTS (#1301) - cmd := JavaExecCommand(fmt.Sprintf("${CONTAINER_SECURITY_PROVIDER:+-Dloader.path=$CONTAINER_SECURITY_PROVIDER} -jar %s", jarFile)) + // JAVA_OPTS is applied by the javaexec launcher (see JavaExecCommand) (#1301) + cmd := JavaExecCommand(s.context.Stager.DepsIdx(), fmt.Sprintf("${CONTAINER_SECURITY_PROVIDER:+-Dloader.path=$CONTAINER_SECURITY_PROVIDER} -jar %s", jarFile)) return cmd, nil } diff --git a/src/java/containers/spring_boot_test.go b/src/java/containers/spring_boot_test.go index f3239a7ce..9b650cd93 100644 --- a/src/java/containers/spring_boot_test.go +++ b/src/java/containers/spring_boot_test.go @@ -122,8 +122,11 @@ var _ = Describe("Spring Boot Container", func() { }) Describe("Release", func() { - expectQuotedEval := func(cmd string) { - Expect(cmd).To(MatchRegexp(`eval "exec .*\$JAVA_OPTS`)) + expectSafeLaunch := func(cmd string) { + Expect(cmd).To(ContainSubstring("/bin/javaexec")) + Expect(cmd).To(ContainSubstring(`"$JAVA_HOME/bin/java"`)) + Expect(cmd).NotTo(ContainSubstring("eval")) + Expect(cmd).NotTo(ContainSubstring("$JAVA_OPTS")) } Context("with exploded JAR (BOOT-INF)", func() { @@ -141,10 +144,10 @@ var _ = Describe("Spring Boot Container", func() { Expect(cmd).To(ContainSubstring("JarLauncher")) }) - It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - expectQuotedEval(cmd) + expectSafeLaunch(cmd) }) }) @@ -162,10 +165,10 @@ var _ = Describe("Spring Boot Container", func() { Expect(cmd).To(ContainSubstring("app-boot.jar")) }) - It("uses quoted eval to protect $JAVA_OPTS from glob expansion", func() { + It("uses javaexec launcher and omits $JAVA_OPTS from the command", func() { cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - expectQuotedEval(cmd) + expectSafeLaunch(cmd) }) }) diff --git a/src/java/finalize/cli/main.go b/src/java/finalize/cli/main.go index 3b26be4ef..770dbd78b 100644 --- a/src/java/finalize/cli/main.go +++ b/src/java/finalize/cli/main.go @@ -52,6 +52,7 @@ func main() { logger.Error("Unable to initialize finalizer from supply config: %s", err.Error()) os.Exit(11) } + f.BuildpackDir = buildpackDir if err = finalize.Run(f); err != nil { os.Exit(12) diff --git a/src/java/finalize/finalize.go b/src/java/finalize/finalize.go index 9cb7d444c..aea61f875 100644 --- a/src/java/finalize/finalize.go +++ b/src/java/finalize/finalize.go @@ -24,6 +24,7 @@ type Finalizer struct { JRE jres.JRE ContainerName string JREName string + BuildpackDir string } // SupplyConfig holds the values written to config.yml by the supply phase. @@ -113,6 +114,12 @@ func Run(f *Finalizer) error { return err } + // Install the javaexec launcher binary used by the start command. + if err := f.installJavaexecLauncher(); err != nil { + f.Log.Error("Failed to install javaexec launcher: %s", err.Error()) + return err + } + // Write release YAML configuration if err := f.writeReleaseYaml(container); err != nil { f.Log.Error("Failed to write release YAML: %s", err.Error()) @@ -191,6 +198,46 @@ func (f *Finalizer) finalizeFrameworks(ctx *common.Context) error { return nil } +// installJavaexecLauncher copies the javaexec launcher binary from the +// buildpack's bin/ directory into the dependency directory so it is present in +// the droplet at runtime. The start command (see containers.JavaExecCommand) +// invokes it as $DEPS_DIR//bin/javaexec to tokenize JAVA_OPTS without a +// shell. The binary is required: if it is missing the generated start command +// would fail at launch, so this returns an error rather than continuing. +func (f *Finalizer) installJavaexecLauncher() error { + buildpackDir := f.BuildpackDir + if buildpackDir == "" { + var err error + buildpackDir, err = libbuildpack.GetBuildpackDir() + if err != nil { + return fmt.Errorf("unable to determine buildpack directory for javaexec launcher: %w", err) + } + } + + src := filepath.Join(buildpackDir, "bin", "javaexec") + data, err := os.ReadFile(src) + if err != nil { + return fmt.Errorf("javaexec launcher binary not found at %s (required by the start command): %w", src, err) + } + + dstDir := filepath.Join(f.Stager.DepDir(), "bin") + if err := os.MkdirAll(dstDir, 0755); err != nil { + return fmt.Errorf("failed to create bin directory: %w", err) + } + + dst := filepath.Join(dstDir, "javaexec") + if err := os.WriteFile(dst, data, 0755); err != nil { + return fmt.Errorf("failed to write javaexec launcher: %w", err) + } + // WriteFile does not change the mode of an existing file; ensure it is executable. + if err := os.Chmod(dst, 0755); err != nil { + return fmt.Errorf("failed to make javaexec launcher executable: %w", err) + } + + f.Log.Debug("Installed javaexec launcher: %s", dst) + return nil +} + // writeReleaseYaml writes the release configuration to a YAML file func (f *Finalizer) writeReleaseYaml(container containers.Container) error { f.Log.BeginStep("Writing release configuration") diff --git a/src/java/finalize/finalize_test.go b/src/java/finalize/finalize_test.go index b13a192fe..781eaa354 100644 --- a/src/java/finalize/finalize_test.go +++ b/src/java/finalize/finalize_test.go @@ -55,6 +55,11 @@ dependencies: [] ` Expect(os.WriteFile(manifestFile, []byte(manifestContent), 0644)).To(Succeed()) + // The finalizer installs the javaexec launcher from /bin. + binDir := filepath.Join(buildpackDir, "bin") + Expect(os.MkdirAll(binDir, 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(binDir, "javaexec"), []byte("#!/bin/sh\n"), 0755)).To(Succeed()) + logger = libbuildpack.NewLogger(GinkgoWriter) mockCtrl = gomock.NewController(GinkgoT()) @@ -67,11 +72,12 @@ dependencies: [] stager = libbuildpack.NewStager([]string{buildDir, cacheDir, depsDir, depsIdx}, logger, manifest) finalizer = &finalize.Finalizer{ - Stager: stager, - Manifest: mockManifest, - Installer: mockInstaller, - Log: logger, - Command: &libbuildpack.Command{}, + Stager: stager, + Manifest: mockManifest, + Installer: mockInstaller, + Log: logger, + Command: &libbuildpack.Command{}, + BuildpackDir: buildpackDir, } }) diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index a2f01e927..16dde281b 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -11,6 +11,7 @@ import ( "github.com/cloudfoundry/java-buildpack/src/java/common" "github.com/cloudfoundry/java-buildpack/src/java/frameworks" + "github.com/cloudfoundry/java-buildpack/src/java/javaexec" "github.com/cloudfoundry/libbuildpack" ) @@ -93,15 +94,21 @@ var _ = Describe("Java Opts Writer", func() { return runWithEnv(scriptPath, javaOpts, `printf '%s\n' "$JAVA_OPTS"`) } - // runStartCommand simulates the actual JVM invocation: - // eval "exec $JAVA_HOME/bin/java $JAVA_OPTS -jar app.jar" - // Returns the argument list java would receive (one arg per line). + // runStartCommand simulates the actual JVM invocation. At launch the + // start command is: + // exec $DEPS_DIR//bin/javaexec "$JAVA_HOME/bin/java" + // where the javaexec launcher tokenizes $JAVA_OPTS without a shell. + // This sources the profile.d assembly script to build $JAVA_OPTS, then + // tokenizes it with the real launcher, returning the argument list java + // would receive (one arg per line). runStartCommand := func(javaOpts string, optsFileContent string) (string, error) { scriptPath := setupScript(javaOpts, optsFileContent) - // Simulate: eval "exec java $JAVA_OPTS" — quoted string prevents bash glob-expansion. - // eval then re-parses the string, honouring embedded quotes in $JAVA_OPTS. - return runWithEnv(scriptPath, javaOpts, - `eval "set -- $JAVA_OPTS"; printf '%s\n' "$@"`) + assembled, err := runWithEnv(scriptPath, javaOpts, `printf '%s' "$JAVA_OPTS"`) + if err != nil { + return assembled, err + } + tokens := javaexec.TokenizeJavaOpts(assembled) + return strings.Join(tokens, "\n") + "\n", nil } It("handles multiline JAVA_OPTS from YAML block scalar without sed error", func() { diff --git a/src/java/javaexec/args.go b/src/java/javaexec/args.go new file mode 100644 index 000000000..1086b92c1 --- /dev/null +++ b/src/java/javaexec/args.go @@ -0,0 +1,15 @@ +package javaexec + +// BuildArgs assembles the full argv for execing the JVM. The resulting slice is +// [java, , ], mirroring the order of +// the previous `exec java $JAVA_OPTS ` start command. trusted args are +// produced by the buildpack (classpath, -jar/-cp, main class) and are passed +// through unchanged; only JAVA_OPTS is tokenized. +func BuildArgs(java, javaOpts string, trusted []string) []string { + opts := TokenizeJavaOpts(javaOpts) + argv := make([]string, 0, 1+len(opts)+len(trusted)) + argv = append(argv, java) + argv = append(argv, opts...) + argv = append(argv, trusted...) + return argv +} diff --git a/src/java/javaexec/args_test.go b/src/java/javaexec/args_test.go new file mode 100644 index 000000000..11891e6e0 --- /dev/null +++ b/src/java/javaexec/args_test.go @@ -0,0 +1,23 @@ +package javaexec + +import ( + "reflect" + "testing" +) + +func TestBuildArgs(t *testing.T) { + got := BuildArgs("/jdk/bin/java", `-Xmx1g -Dfoo="bar baz"`, + []string{"-jar", "app.jar"}) + want := []string{"/jdk/bin/java", "-Xmx1g", "-Dfoo=bar baz", "-jar", "app.jar"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("BuildArgs = %#v, want %#v", got, want) + } +} + +func TestBuildArgsEmptyOptsHasNoEmptyArg(t *testing.T) { + got := BuildArgs("/jdk/bin/java", "", []string{"-jar", "app.jar"}) + want := []string{"/jdk/bin/java", "-jar", "app.jar"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("BuildArgs with empty JAVA_OPTS = %#v, want %#v", got, want) + } +} diff --git a/src/java/javaexec/cli/main.go b/src/java/javaexec/cli/main.go new file mode 100644 index 000000000..428512f22 --- /dev/null +++ b/src/java/javaexec/cli/main.go @@ -0,0 +1,48 @@ +package main + +import ( + "fmt" + "os" + "os/exec" + "strings" + "syscall" + + "github.com/cloudfoundry/java-buildpack/src/java/javaexec" +) + +// The javaexec launcher replaces the runtime `eval "exec java $JAVA_OPTS "` +// start command. It is invoked as: +// +// javaexec [trusted args...] +// +// It reads JAVA_OPTS from the environment, tokenizes it without a shell (no +// expansion, no command substitution, no globbing), and execs the JVM with +// [java, , ]. Because it execs, the JVM +// replaces this process: no extra wrapper process remains. +func main() { + if len(os.Args) < 2 { + fmt.Fprintln(os.Stderr, "javaexec: usage: javaexec [args...]") + os.Exit(2) + } + + java := os.Args[1] + trusted := os.Args[2:] + + // syscall.Exec does not search PATH; resolve a bare command name (e.g. the + // Play container's "java") to an absolute path. + if !strings.ContainsRune(java, '/') { + resolved, err := exec.LookPath(java) + if err != nil { + fmt.Fprintf(os.Stderr, "javaexec: cannot find %q on PATH: %v\n", java, err) + os.Exit(127) + } + java = resolved + } + + argv := javaexec.BuildArgs(java, os.Getenv("JAVA_OPTS"), trusted) + + if err := syscall.Exec(java, argv, os.Environ()); err != nil { + fmt.Fprintf(os.Stderr, "javaexec: failed to exec %s: %v\n", java, err) + os.Exit(126) + } +} diff --git a/src/java/javaexec/tokenize.go b/src/java/javaexec/tokenize.go new file mode 100644 index 000000000..418f1b0ab --- /dev/null +++ b/src/java/javaexec/tokenize.go @@ -0,0 +1,80 @@ +// Package launcher tokenizes JAVA_OPTS and execs the JVM without invoking a +// shell. It exists to remove the runtime `eval "exec java $JAVA_OPTS ..."` +// start command, which lets the shell execute command substitutions and treat +// metacharacters (&, |, ;, newlines) in user-provided JAVA_OPTS as operators. +package javaexec + +import "strings" + +// TokenizeJavaOpts splits a JAVA_OPTS string into individual arguments using +// POSIX shell word-splitting and quote-removal rules, but WITHOUT performing +// any expansion: variable references ($VAR), command substitutions ($(...) and +// backticks), globbing, and operators (&, |, ;, <, >) are all treated as +// ordinary literal text. This yields the same arguments the previous +// `eval`-based start command produced for normal and quoted inputs, while +// never executing embedded commands. +// +// An empty or whitespace-only input yields no arguments (nil), so the JVM is +// never handed a spurious empty argument. +func TokenizeJavaOpts(s string) []string { + var tokens []string + var b strings.Builder + started := false + + flush := func() { + if started { + tokens = append(tokens, b.String()) + b.Reset() + started = false + } + } + + runes := []rune(s) + for i := 0; i < len(runes); i++ { + c := runes[i] + switch c { + case ' ', '\t', '\n', '\r', '\v', '\f': + flush() + case '\'': + started = true + // Single quotes: everything literal until the next single quote. + for i++; i < len(runes) && runes[i] != '\''; i++ { + b.WriteRune(runes[i]) + } + case '"': + started = true + // Double quotes: literal, except backslash escapes a small set. + for i++; i < len(runes) && runes[i] != '"'; i++ { + if runes[i] == '\\' && i+1 < len(runes) { + next := runes[i+1] + switch next { + case '"', '\\', '$', '`': + b.WriteRune(next) + i++ + continue + case '\n': + // Line continuation: drop backslash and newline. + i++ + continue + } + } + b.WriteRune(runes[i]) + } + case '\\': + started = true + // Unquoted backslash escapes the next character. + if i+1 < len(runes) { + i++ + b.WriteRune(runes[i]) + } else { + b.WriteRune('\\') + } + default: + started = true + b.WriteRune(c) + } + } + flush() + + return tokens +} diff --git a/src/java/javaexec/tokenize_test.go b/src/java/javaexec/tokenize_test.go new file mode 100644 index 000000000..d8da295ef --- /dev/null +++ b/src/java/javaexec/tokenize_test.go @@ -0,0 +1,67 @@ +package javaexec + +import ( + "reflect" + "testing" +) + +func TestTokenizeJavaOpts(t *testing.T) { + cases := []struct { + name string + in string + want []string + }{ + {"empty", "", nil}, + {"whitespace only", " \t\n ", nil}, + {"simple", "-Xmx1g", []string{"-Xmx1g"}}, + {"two simple", "-Xmx1g -Xms512m", []string{"-Xmx1g", "-Xms512m"}}, + {"leading -n", "-n -Dfoo=bar", []string{"-n", "-Dfoo=bar"}}, + + // Quoting: #1301 — quoted value with spaces stays a single argument. + {"double-quoted spaces", `-Dfoo="bar baz"`, []string{"-Dfoo=bar baz"}}, + {"single-quoted spaces", `-Dfoo='bar baz'`, []string{"-Dfoo=bar baz"}}, + {"two quoted args", `-Dfoo="bar baz" -Dother="qux quux"`, + []string{"-Dfoo=bar baz", "-Dother=qux quux"}}, + + // #1301 — cron with glob characters: one argument, '*' NOT expanded. + {"cron quoted", `-DcronSched="0 */7 * * * *"`, []string{"-DcronSched=0 */7 * * * *"}}, + + // Newlines (YAML block scalar) act as whitespace separators (issue #1259). + {"newline separated", "-Xmx1g\n-Xms512m", []string{"-Xmx1g", "-Xms512m"}}, + + // Shell metacharacters are treated as ordinary literal characters, + // NOT operators (the launcher is not a shell). This is safe by design. + {"ampersand literal", "-Dfoo=a&b", []string{"-Dfoo=a&b"}}, + {"pipe literal", "-Dx=a|b", []string{"-Dx=a|b"}}, + {"semicolon literal", "-Dx=a;b", []string{"-Dx=a;b"}}, + {"redirect literal", "-Dx=a>b", []string{"-Dx=a>b"}}, + + // Security: command substitution is NOT executed; passed through literally. + // Unquoted, the space inside splits it into two literal tokens (correct + // word-splitting) — neither is executed. + {"command substitution unquoted", "-Dx=$(echo HACK)", []string{"-Dx=$(echo", "HACK)"}}, + // Quoted, it stays a single literal argument and is never executed. + {"command substitution quoted", `-Dx="$(echo danger)"`, []string{"-Dx=$(echo danger)"}}, + {"backtick literal", "-Dx=`id`", []string{"-Dx=`id`"}}, + {"dollar var literal", "-Dx=$HOME", []string{"-Dx=$HOME"}}, + + // Backslash follows POSIX shell rules (parity with previous eval form): + // unquoted backslash escapes the next character. + {"escaped space joins", `a\ b`, []string{"a b"}}, + {"double backslash", `-Dpath=C:\\double`, []string{`-Dpath=C:\double`}}, + {"single-quoted backslashes literal", `-Dpath='C:\tmp\app'`, []string{`-Dpath=C:\tmp\app`}}, + + // Empty quoted string yields an explicit empty argument. + {"empty double quotes", `-Dfoo=""`, []string{"-Dfoo="}}, + {"standalone empty quotes", `'' x`, []string{"", "x"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := TokenizeJavaOpts(tc.in) + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("TokenizeJavaOpts(%q) = %#v, want %#v", tc.in, got, tc.want) + } + }) + } +} From afa8f9d99c181a3a1f799892ca213ce462481684 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Mon, 8 Jun 2026 12:49:48 +0000 Subject: [PATCH 10/24] fix: escape double quotes in JBP_CONFIG_JAVA_MAIN arguments; rename unused test param Arguments from JBP_CONFIG_JAVA_MAIN containing double quotes would break the outer eval "..." string in the start command. Escape them before building the javaArgs string. Add regression test. Rename unused 'javaOpts' param in setupScript test helper to '_' to document that it has no effect on script generation. --- src/java/containers/java_main.go | 3 ++- src/java/containers/java_main_test.go | 14 ++++++++++++++ src/java/frameworks/java_opts_writer_test.go | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/java/containers/java_main.go b/src/java/containers/java_main.go index 374c8250a..08b6ceb55 100644 --- a/src/java/containers/java_main.go +++ b/src/java/containers/java_main.go @@ -286,7 +286,8 @@ func (j *JavaMainContainer) Release() (string, error) { args := "" if cfg.Arguments != "" { - args = " " + cfg.Arguments + // Escape double quotes so they don't terminate the outer eval "..." string. + args = " " + strings.ReplaceAll(cfg.Arguments, `"`, `\"`) } // JBP_CONFIG_JAVA_MAIN java_main_class takes precedence over manifest Main-Class. diff --git a/src/java/containers/java_main_test.go b/src/java/containers/java_main_test.go index ebd094507..a4dec30d5 100644 --- a/src/java/containers/java_main_test.go +++ b/src/java/containers/java_main_test.go @@ -282,6 +282,20 @@ var _ = Describe("Java Main Container", func() { Expect(err).NotTo(HaveOccurred()) Expect(cmd).To(ContainSubstring("--foo=bar")) }) + + It("escapes double quotes in arguments to avoid breaking the eval string", func() { + os.Setenv("JBP_CONFIG_JAVA_MAIN", `{arguments: "--spring.datasource.url=\"jdbc:h2:mem:test\""}`) + Expect(createJar( + filepath.Join(buildDir, "app.jar"), + "Manifest-Version: 1.0\nMain-Class: com.example.Main\n", + )).To(Succeed()) + container.Detect() + cmd, err := container.Release() + Expect(err).NotTo(HaveOccurred()) + // Double quotes in arguments must be backslash-escaped so they + // don't terminate the outer eval "..." string. + Expect(cmd).To(ContainSubstring(`--spring.datasource.url=\"jdbc:h2:mem:test\"`)) + }) }) // Regression tests for issue #1301: start command must use eval "exec ... $JAVA_OPTS" diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index 5a648d53d..d2084c8f6 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -55,7 +55,7 @@ var _ = Describe("Java Opts Writer", func() { }) Describe("CreateJavaOptsAssemblyScript", func() { - setupScript := func(javaOpts string, optsFileContent string) string { + setupScript := func(_ string, optsFileContent string) string { err := frameworks.CreateJavaOptsAssemblyScript(ctx) Expect(err).NotTo(HaveOccurred()) From b277fabdd71362fa386fd177890cebd2dc79215c Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Mon, 8 Jun 2026 13:21:02 +0000 Subject: [PATCH 11/24] fix(java_opts): escape " and \ in opts_content before eval expansion eval "printf '%s' \"\$opts_content\"" mangles .opts content containing literal double quotes (e.g. Datadog writes -Ddd.service="myapp"): the inner " terminates the outer eval string and the value is stripped. Escape \ (first) and " in opts_content into a temporary _eval_safe variable before expanding it into the eval command, so both chars are preserved. Add regression tests for double-quoted and backslash .opts values. --- src/java/frameworks/java_opts_writer.go | 7 ++++++- src/java/frameworks/java_opts_writer_test.go | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index f45a742a8..66db45f8b 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -106,11 +106,16 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then # backslashes in the user-provided JAVA_OPTS are never exposed to eval. opts_content="${opts_content//\$JAVA_OPTS/$_user_java_opts_placeholder}" + # Escape \ and " in opts_content so they do not break the + # eval "..." wrapper below. \ must be escaped first. + _eval_safe="${opts_content//\\/\\\\}" + _eval_safe="${_eval_safe//\"/\\\"}" + # Expand any remaining environment variables in opts content via eval. # Note: eval executes commands, but .opts files are written by the buildpack # at staging time and run within the container context. # This matches how the Ruby buildpack naturally expanded variables via shell. - opts_content=$(eval "printf '%%s' \"$opts_content\"") + opts_content=$(eval "printf '%%s' \"$_eval_safe\"") # Now safely substitute JAVA_OPTS after eval (preserves quotes, backslashes, and ampersands) opts_content="${opts_content//$_user_java_opts_placeholder/$_escaped_user_java_opts}" diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index d2084c8f6..b97f12f57 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -221,5 +221,20 @@ var _ = Describe("Java Opts Writer", func() { // Java receives exactly one arg: -DcronSched=0 */7 * * * * Expect(strings.TrimSpace(output)).To(Equal("-DcronSched=0 */7 * * * *")) }) + + // Regression test: eval mangles .opts content containing literal double quotes. + // e.g. Datadog writes -Ddd.service="myapp" into its .opts file; the inner " + // terminates the outer eval "..." string, stripping the value. + It("preserves double-quoted values from .opts file content", func() { + output, err := runScript("", `-Ddd.service="myapp"`) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Ddd.service="myapp"`)) + }) + + It("preserves backslashes in .opts file content", func() { + output, err := runScript("", `-Dpattern=foo\|bar`) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dpattern=foo\|bar`)) + }) }) }) From 1d97a85f9063a4f4651cfe19ea86d70f186ac6f5 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 06:58:14 +0000 Subject: [PATCH 12/24] fix(tests): correct SapMachine log assertion strings #1288 BaseJRE refactor renamed the install log from "Installing SAP Machine" to "Installing SapMachine"; four assertions in tomcat_test.go and java_main_test.go were missed. --- src/integration/java_main_test.go | 2 +- src/integration/tomcat_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/integration/java_main_test.go b/src/integration/java_main_test.go index 954ddd83f..017393050 100644 --- a/src/integration/java_main_test.go +++ b/src/integration/java_main_test.go @@ -126,7 +126,7 @@ func testJavaMain(platform switchblade.Platform, fixtures string) func(*testing. // Verify SAPMachine JRE was installed from manifest Expect(logs.String()).To(ContainSubstring("Java Buildpack")) - Expect(logs.String()).To(ContainSubstring("Installing SAP Machine")) + Expect(logs.String()).To(ContainSubstring("Installing SapMachine")) Expect(logs.String()).To(ContainSubstring("17.")) }) }) diff --git a/src/integration/tomcat_test.go b/src/integration/tomcat_test.go index 6e0aa362a..db619765b 100644 --- a/src/integration/tomcat_test.go +++ b/src/integration/tomcat_test.go @@ -371,7 +371,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) - Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (17.")) + Expect(logs.String()).To(ContainSubstring("Installing SapMachine (17.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -384,7 +384,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) - Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (21.")) + Expect(logs.String()).To(ContainSubstring("Installing SapMachine (21.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -397,7 +397,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) - Expect(logs.String()).To(ContainSubstring("Installing SAP Machine (25.")) + Expect(logs.String()).To(ContainSubstring("Installing SapMachine (25.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) }) From 96ae5337a7aff4de43d34f54c28eedff157b7e22 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 07:42:21 +0000 Subject: [PATCH 13/24] fix(tests): correct Tomcat version assertion strings Commit 90abbc21 changed the log format from "Installed Tomcat version %s" to "Installed Tomcat (%s)" but did not update the integration test assertions that matched "Tomcat 9" and "Tomcat 10". Those substrings no longer appear in the output, causing all JRE-selection Tomcat tests to fail. --- src/integration/tomcat_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/integration/tomcat_test.go b/src/integration/tomcat_test.go index db619765b..2952e3be1 100644 --- a/src/integration/tomcat_test.go +++ b/src/integration/tomcat_test.go @@ -171,7 +171,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (8.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 9")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -185,7 +185,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (11.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 9")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -198,7 +198,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 9")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (9.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -224,7 +224,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (11.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 10")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) @@ -237,7 +237,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 10")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) }) @@ -258,7 +258,7 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T, Expect(err).NotTo(HaveOccurred(), logs.String) Expect(logs.String()).To(ContainSubstring("Installing OpenJDK (17.")) - Expect(logs.String()).To(ContainSubstring("Tomcat 10.1.")) + Expect(logs.String()).To(ContainSubstring("Installed Tomcat (10.1.")) Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK"))) }) }) From 2bb06e16da5bf415c6cb0993cc9da4623eb89013 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 07:43:29 +0000 Subject: [PATCH 14/24] feat(java_opts): safe runtime expansion and \$VAR escape (#1301) Replace eval-based JAVA_OPTS assembly with a pure-bash _expand_env_vars function that expands $VAR/${VAR} but never executes $(...) or backtick substitutions. Resolves the one trusted $(nproc) token by exact string match instead of execution. User JAVA_OPTS and .opts file content are both run through the expander. A \$VAR prefix is honoured as a literal $ (Ruby buildpack parity) via a placeholder round-trip. CRLF line endings from Windows-edited manifests are stripped before assembly. Renames _nproc_token/_nproc_value -> _nproc_count; inlines the \$(nproc) pattern literal so the variable name no longer shadows the shell command it references. java_main: remove double-quote escaping that was only needed because the old eval consumed one extra parse layer; javaexec sees the command once so quotes pass through unmodified. Closes #1301 --- manifest.yml | 1 + src/java/containers/java_main.go | 6 +- src/java/containers/java_main_test.go | 13 +-- .../azure_application_insights_agent.go | 3 +- src/java/frameworks/java_opts_writer.go | 34 ++++++-- src/java/frameworks/java_opts_writer_test.go | 80 +++++++++++++++++++ 6 files changed, 120 insertions(+), 17 deletions(-) diff --git a/manifest.yml b/manifest.yml index 16e19274c..eb72ec537 100644 --- a/manifest.yml +++ b/manifest.yml @@ -9,6 +9,7 @@ include_files: - bin/compile - bin/detect - bin/finalize +- bin/javaexec - bin/release - bin/supply - manifest.yml diff --git a/src/java/containers/java_main.go b/src/java/containers/java_main.go index 317fc7dff..bd46f4dfc 100644 --- a/src/java/containers/java_main.go +++ b/src/java/containers/java_main.go @@ -286,8 +286,10 @@ func (j *JavaMainContainer) Release() (string, error) { args := "" if cfg.Arguments != "" { - // Escape double quotes so they don't terminate the outer eval "..." string. - args = " " + strings.ReplaceAll(cfg.Arguments, `"`, `\"`) + // Passed through as-is onto the start command line. The command is parsed + // once by the shell at launch (no eval), so any quoting in the arguments is + // applied exactly once, like a normal shell command line. + args = " " + cfg.Arguments } // JBP_CONFIG_JAVA_MAIN java_main_class takes precedence over manifest Main-Class. diff --git a/src/java/containers/java_main_test.go b/src/java/containers/java_main_test.go index 0792417a2..3835ea657 100644 --- a/src/java/containers/java_main_test.go +++ b/src/java/containers/java_main_test.go @@ -286,7 +286,7 @@ var _ = Describe("Java Main Container", func() { Expect(cmd).To(ContainSubstring("--foo=bar")) }) - It("escapes double quotes in arguments to avoid breaking the eval string", func() { + It("passes double quotes in arguments through unescaped", func() { os.Setenv("JBP_CONFIG_JAVA_MAIN", `{arguments: "--spring.datasource.url=\"jdbc:h2:mem:test\""}`) Expect(createJar( filepath.Join(buildDir, "app.jar"), @@ -295,14 +295,15 @@ var _ = Describe("Java Main Container", func() { container.Detect() cmd, err := container.Release() Expect(err).NotTo(HaveOccurred()) - // Double quotes in arguments must be backslash-escaped so they - // don't terminate the outer eval "..." string. - Expect(cmd).To(ContainSubstring(`--spring.datasource.url=\"jdbc:h2:mem:test\"`)) + // No eval: the start command is parsed once by the shell, which + // consumes the quotes, so they must NOT be backslash-escaped. + Expect(cmd).To(ContainSubstring(`--spring.datasource.url="jdbc:h2:mem:test"`)) }) }) - // Regression tests for issue #1301: start command must use eval "exec ... $JAVA_OPTS" - // (quoted string) so that glob chars in JAVA_OPTS are not expanded by bash before eval. + // Regression tests for issue #1301: start command must use the javaexec + // launcher and keep $JAVA_OPTS out of the command, so glob chars in + // JAVA_OPTS are never expanded by the shell. Context("with JAR file (javaexec launch)", func() { BeforeEach(func() { Expect(createJar( diff --git a/src/java/frameworks/azure_application_insights_agent.go b/src/java/frameworks/azure_application_insights_agent.go index 914dc14b6..00f29610f 100644 --- a/src/java/frameworks/azure_application_insights_agent.go +++ b/src/java/frameworks/azure_application_insights_agent.go @@ -168,7 +168,8 @@ func (a *AzureApplicationInsightsAgentFramework) Finalize() error { // Set connection string if available if credentials.ConnectionString != "" { - // Single-quote the value so eval exec java $JAVA_OPTS does not split on ; in the connection string + // Single-quote the value so the javaexec launcher keeps it as one token + // (the connection string contains ; and spaces). opts = append(opts, fmt.Sprintf("-Dapplicationinsights.connection.string='%s'", credentials.ConnectionString)) } else if credentials.InstrumentationKey != "" { // Fallback to instrumentation key diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index 0d72b57ac..2b076bebc 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -73,9 +73,10 @@ func CreateJavaOptsAssemblyScript(ctx *common.Context) error { # Expands runtime variables like $DEPS_DIR, $HOME, $JAVA_OPTS, and all other environment variables # Save original JAVA_OPTS from environment (user-provided) -# Normalize to single line: YAML block scalars (>) may introduce newlines -# Only convert newlines to spaces — do not use xargs which strips quotes and backslashes -USER_JAVA_OPTS=$(printf '%%s' "$JAVA_OPTS" | tr '\n' ' ') +# Normalize to single line: YAML block scalars (>) may introduce newlines; Windows-edited +# manifests may use CRLF (\r\n). Strip \r first, then convert \n to spaces. +# Do not use xargs — it strips quotes and backslashes. +USER_JAVA_OPTS=$(printf '%%s' "$JAVA_OPTS" | tr -d '\r' | tr '\n' ' ') # Start building new JAVA_OPTS JAVA_OPTS="" @@ -108,10 +109,20 @@ _expand_env_vars() { # single known, trusted token here by computing the processor count once and # substituting it literally below. All other command substitutions remain # literal (and therefore inert). -_nproc_value="$(nproc 2>/dev/null || echo 1)" -_nproc_token='$(nproc)' +_nproc_count="$(nproc 2>/dev/null || echo 1)" # Backtick char, computed here because this script lives inside a Go raw string. _backtick="$(printf '\140')" +# Placeholder for \$ escape sequences (Ruby buildpack parity: \$VAR → literal $VAR). +_escaped_dollar_placeholder='__JAVA_OPTS_ESCAPED_DOLLAR__' +_dollar='$' + +# Expand $VAR / ${VAR} references in user JAVA_OPTS (e.g. $PWD, $HOME), matching +# pre-eval behaviour. \$VAR is treated as a literal $ (not expanded), matching the +# Ruby buildpack's eval-based behaviour where \$ suppressed variable expansion. +# Command substitutions are still not executed. +USER_JAVA_OPTS="${USER_JAVA_OPTS//\\\$/$_escaped_dollar_placeholder}" +USER_JAVA_OPTS=$(_expand_env_vars "$USER_JAVA_OPTS") +USER_JAVA_OPTS="${USER_JAVA_OPTS//$_escaped_dollar_placeholder/$_dollar}" # Escape replacement-special chars once; these values are loop-invariant. _escaped_deps_dir="${DEPS_DIR//\\/\\\\}" @@ -126,8 +137,12 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then for opts_file in "$DEPS_DIR/%s/java_opts"/*.opts; do if [ -f "$opts_file" ]; then # Read content and expand runtime variables - opts_content=$(cat "$opts_file") - + opts_content=$(< "$opts_file") + + # Shield \$ from expansion so buildpack authors can write \$VAR to pass + # a literal $VAR to the JVM (Ruby buildpack parity). + opts_content="${opts_content//\\\$/$_escaped_dollar_placeholder}" + # Expand $DEPS_DIR and $HOME using bash parameter expansion. # In ${var//pattern/repl}, '&' and '\' are special in replacement strings, # so escape them first to preserve literal path contents. @@ -135,7 +150,7 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then opts_content="${opts_content//\$HOME/$_escaped_home}" # Resolve the trusted $(nproc) token to the computed processor count. - opts_content="${opts_content//$_nproc_token/$_nproc_value}" + opts_content="${opts_content//\$\(nproc\)/$_nproc_count}" # Shield $JAVA_OPTS from expansion: replace with a placeholder first, # then substitute the actual value AFTER expansion so that quotes and @@ -148,6 +163,9 @@ if [ -d "$DEPS_DIR/%s/java_opts" ]; then # NOT executed — only $VAR / ${VAR} references are expanded. opts_content=$(_expand_env_vars "$opts_content") + # Restore \$ escapes to literal $ now that expansion is done. + opts_content="${opts_content//$_escaped_dollar_placeholder/$_dollar}" + # Defense-in-depth: the buildpack resolves its only known command # substitution ($(nproc)) above. Any remaining $(...) or backtick at # this point is an unresolved buildpack-emitted substitution (the diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index e7380be39..450264ee6 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -122,6 +122,20 @@ var _ = Describe("Java Opts Writer", func() { output, err := runScript(multilineJavaOpts, "-javaagent:somepath.jar $JAVA_OPTS") Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) Expect(output).To(ContainSubstring("-XX:+UseZGC")) + // $HOME must expand even when JAVA_OPTS contains newlines + Expect(output).To(ContainSubstring("-javaagent:/home/vcap/app/BOOT-INF/lib/agent.jar")) + Expect(output).NotTo(ContainSubstring("$HOME")) + }) + + It("strips carriage returns (CRLF) from multiline JAVA_OPTS", func() { + // Windows-edited manifests may deliver JAVA_OPTS with \r\n line endings. + // tr '\n' ' ' strips \n but leaves \r, corrupting JVM args (e.g. -Dfoo=bar\r). + crlfJavaOpts := "-Dfoo=bar\r\n-Dbaz=qux\r\n" + output, err := runScript(crlfJavaOpts, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-Dfoo=bar")) + Expect(output).To(ContainSubstring("-Dbaz=qux")) + Expect(output).NotTo(ContainSubstring("\r")) }) It("handles pipe character in JAVA_OPTS (e.g. javaagent options) without sed error", func() { @@ -201,6 +215,72 @@ var _ = Describe("Java Opts Writer", func() { Expect(strings.TrimSpace(output)).To(Equal("-n")) }) + It("does not execute command substitution in user JAVA_OPTS (#1301, no eval)", func() { + // The #1301 user-facing scenario: a command substitution in the + // user-provided JAVA_OPTS env must reach java as a literal argument, + // never be executed. Proven end-to-end through the real assembly script + // and the real javaexec tokenizer, with a marker file as a second guard. + marker := filepath.Join(cacheDir, "user_pwned_marker") + Expect(os.Remove(marker)).To(Or(Succeed(), MatchError(os.ErrNotExist))) + // Quoted so the value (which contains a space) stays a single argument. + output, err := runStartCommand(`-Dx="$(touch `+marker+`)"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + _, statErr := os.Stat(marker) + Expect(os.IsNotExist(statErr)).To(BeTrue(), "command substitution was executed; marker file created") + // java receives the single literal argument, $(...) intact. + Expect(strings.TrimSpace(output)).To(Equal("-Dx=$(touch " + marker + ")")) + }) + + It(`treats \$VAR as a literal $ in user JAVA_OPTS (Ruby buildpack parity)`, func() { + // Ruby buildpack: eval treated \$VAR as literal $VAR (not expanded). + // Users migrating with \$HOME should get $HOME literally, not the expanded path. + // The test helper sets HOME=/home/vcap/app so we can confirm non-expansion. + output, err := runScript(`-Dfoo=\$HOME`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dfoo=$HOME`)) + Expect(output).NotTo(ContainSubstring("-Dfoo=/home/vcap/app")) + }) + + It(`treats \$VAR as a literal $ in opts file content (Ruby buildpack parity)`, func() { + output, err := runScript("", `-Dexample=\$MY_VAR`) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring(`-Dexample=$MY_VAR`)) + }) + + It("expands $VAR references in user JAVA_OPTS", func() { + // $PWD in user JAVA_OPTS must expand to the working directory, + // matching pre-eval behaviour. Command substitutions must still not execute. + wd, _ := os.Getwd() + output, err := runScript( + `-Dapplicationinsights.configuration.file=$PWD/BOOT-INF/classes/applicationinsights.json`, + "$JAVA_OPTS", + ) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-Dapplicationinsights.configuration.file=" + wd + "/BOOT-INF/classes/applicationinsights.json")) + Expect(output).NotTo(ContainSubstring("$PWD")) + }) + + It("expands $HOME in user JAVA_OPTS (javaagent path use case)", func() { + output, err := runScript( + `-javaagent:$HOME/BOOT-INF/lib/agent.jar`, + "$JAVA_OPTS", + ) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-javaagent:/home/vcap/app/BOOT-INF/lib/agent.jar")) + Expect(output).NotTo(ContainSubstring("$HOME")) + }) + + It("expands ${VAR} braces form in user JAVA_OPTS", func() { + wd, _ := os.Getwd() + output, err := runScript( + `-Dconfig.file=${PWD}/config/app.properties`, + "$JAVA_OPTS", + ) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("-Dconfig.file=" + wd + "/config/app.properties")) + Expect(output).NotTo(ContainSubstring("${PWD}")) + }) + // Regression tests for issue #1301: xargs strips quotes, breaking quoted JVM args It("preserves quoted value with spaces in JAVA_OPTS", func() { // JAVA_OPTS='-Dfoo="bar baz"' — xargs removes the quotes from USER_JAVA_OPTS, From 078340d0b4e7364ff35a8c3d5682330e1bfd8a7e Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 07:43:47 +0000 Subject: [PATCH 15/24] docs(java_opts): document runtime variable expansion and Ruby migration Replace the stale eval-era "Escaping strings" section with a "Runtime variable expansion" section covering $VAR expansion, the $(nproc) exception, \$VAR literal-dollar escape, POSIX quoting, and a lookup table for common escaping needs. Add docs/java_opts-ruby-migration.md comparing Ruby vs Go buildpack escaping rules with a quick migration checklist. --- docs/framework-java_opts.md | 84 +++++++++++++++++++++++------- docs/java_opts-ruby-migration.md | 87 ++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 19 deletions(-) create mode 100644 docs/java_opts-ruby-migration.md diff --git a/docs/framework-java_opts.md b/docs/framework-java_opts.md index a4067582f..1b83d3555 100644 --- a/docs/framework-java_opts.md +++ b/docs/framework-java_opts.md @@ -26,37 +26,83 @@ The framework can be configured by creating or modifying the [`config/java_opts. Any `JAVA_OPTS` from either the config file or environment variables will be specified in the start command after any Java Opts added by other frameworks. -## Escaping strings +## Runtime variable expansion -Java options will have special characters escaped when used in the shell command that starts the Java application but the `$` and `\` characters will not be escaped. This is to allow Java options to include environment variables when the application starts. +Java options are assembled at container start by the buildpack's `profile.d` script +(`00_java_opts.sh`), then passed to the JVM by the shell-free `javaexec` launcher. +Because `javaexec` tokenizes `JAVA_OPTS` without invoking a shell, characters such as +`*`, `&`, `;`, `|`, and `>` are treated as literals — they reach the JVM exactly as +written. -```bash -cf set-env my-application JAVA_OPTS '-Dexample.port=$PORT' -``` - -If an escaped `$` or `\` character is needed in the Java options they will have to be escaped manually. For example, to obtain this output in the start command. +### Environment variable references -```bash --Dexample.other=something.\$dollar.\\slash -``` +`$VARNAME` and `${VARNAME}` references in **both** `JAVA_OPTS` (env) and `java_opts` +(config) are expanded at container start against the runtime environment: -From the command line use; ```bash -cf set-env my-application JAVA_OPTS '-Dexample.other=something.\\\\\$dollar.\\\\\\\slash' +# $PWD, $HOME, $PORT, and any CF-injected variable all work +cf set-env my-application JAVA_OPTS '-Dapp.config=$PWD/config/app.properties' +cf set-env my-application JAVA_OPTS '-Dserver.port=$PORT' ``` -From the [`config/java_opts.yml`][] file use; ```yaml -from_environment: true -java_opts: '-Dexample.other=something.\\$dollar.\\\\slash' +# config/java_opts.yml +java_opts: '-Xloggc:$PWD/beacon_gc.log -verbose:gc' ``` -Finally, from the applications manifest use; -```yaml - env: - JAVA_OPTS: '-Dexample.other=something.\\\\\$dollar.\\\\\\\slash' +### Command substitutions are never executed + +`$(...)` and backtick command substitutions are **not** executed. A value such as +`-Dinject=$(hostname)` reaches the JVM as the literal string `-Dinject=$(hostname)`. +This is intentional: executing arbitrary commands from a user-supplied option string +would be a security vulnerability. + +### Processor count: `$(nproc)` + +The one exception is `-XX:ActiveProcessorCount=$(nproc)`, which the buildpack itself +emits for JRE vendors that need it. The profile.d script resolves this single known +token to the actual CPU count before passing the option to the JVM. Any other +`$(...)` expression passes to the JVM literally. + +### Special characters and quoting + +Characters that were shell-special under the old `eval`-based launcher (`*`, `&`, +`;`, `|`, `>`) are now passed to the JVM as literals — no quoting tricks required. + +POSIX quoting in the assembled `JAVA_OPTS` string is respected by `javaexec`'s +tokenizer: a quoted value such as `"-Dfoo=bar baz"` is delivered as the single +argument `-Dfoo=bar baz`. + +| Want to pass to JVM | Write in `JAVA_OPTS` / `java_opts` | +|---------------------|-------------------------------------| +| Literal `$PORT` (no expansion) | `\$PORT` | +| Literal `\` backslash | `\\` | +| Literal `\\` two backslashes | `\\\\` | +| Value of `$PORT` at runtime | `$PORT` | +| Cron expression `0 */7 * * *` | `0 */7 * * *` (no quoting needed) | +| Space inside one JVM arg | `"-Dfoo=bar baz"` (quote the arg) | + +```bash +# Expand $PORT at runtime +cf set-env my-application JAVA_OPTS '-Dserver.port=$PORT' + +# Literal $PORT — not expanded +cf set-env my-application JAVA_OPTS '-Dexample.literal=\$PORT' + +# Windows-style path — \\ becomes one backslash +cf set-env my-application JAVA_OPTS '-Dapp.data=C:\\data\\app' + +# Cron expression — * is not glob-expanded +cf set-env my-application JAVA_OPTS '-DcronExpr=0 */7 * * *' ``` +> **Note:** `$` followed by a digit or non-identifier character (e.g. `$1`, `$.`) +> is left as-is. Undefined variables expand to an empty string. + +> **Migrating from the Ruby buildpack?** See +> [Migrating JAVA_OPTS escaping from the Ruby buildpack](java_opts-ruby-migration.md) +> for a comparison of the escaping rules. + ## Examples ### Configuration File Example diff --git a/docs/java_opts-ruby-migration.md b/docs/java_opts-ruby-migration.md new file mode 100644 index 000000000..3bb60baf8 --- /dev/null +++ b/docs/java_opts-ruby-migration.md @@ -0,0 +1,87 @@ +# Migrating JAVA_OPTS escaping from the Ruby buildpack + +The Go rewrite of the Java buildpack changed how `JAVA_OPTS` is assembled and +passed to the JVM. If you are migrating configs written for the Ruby buildpack, +the escaping rules are different. + +--- + +## What changed + +| Mechanism | Ruby buildpack | Go buildpack | +|-----------|---------------|-------------| +| Launch | `eval exec java $JAVA_OPTS ...` | `javaexec` (shell-free tokenizer) | +| `$VAR` in opts | expanded by shell at eval | expanded by `profile.d` at container start | +| `$(cmd)` in opts | **executed** by shell | **never executed** (security fix, #1301) | +| `\` handling | eval consumed one level of backslashes | `javaexec` POSIX: `\\`→`\`, `\"` → `"` | +| `*` glob | expanded against filesystem | literal | + +--- + +## Escaping comparison + +### Dollar sign before a variable name + +Both buildpacks expand `$VAR` references at runtime. No escaping needed or supported. + +```bash +# Works the same in both buildpacks +cf set-env my-app JAVA_OPTS '-Dserver.port=$PORT' +``` + +To prevent expansion, `\$` works in both buildpacks: `\$VAR` delivers the +literal text `$VAR` to the JVM without expanding it. + +### Backslash + +```bash +# Ruby buildpack: \\\\ in the manifest/env → \\ after eval → \ to JVM +# Go buildpack: \\ in the manifest/env → \ to JVM (POSIX tokenizer, one level) +``` + +| Want to deliver to JVM | Ruby buildpack (env) | Go buildpack (env) | +|------------------------|----------------------|--------------------| +| one `\` | `\\\\` | `\\` | +| two `\\` | `\\\\\\\\` | `\\\\` | +| literal `\$PORT` | `\\\\\$PORT` | not supported — `$PORT` expands | + +### Cron expressions and glob characters (`*`) + +```bash +# Ruby buildpack: must be quoted carefully to survive eval and glob expansion +# Go buildpack: write literally — * never globs, no eval +cf set-env my-app JAVA_OPTS '-DcronExpr=0 */7 * * *' +``` + +### Command substitution + +```bash +# Ruby buildpack: $(hostname) in JAVA_OPTS was EXECUTED and replaced with output +# Go buildpack: $(hostname) reaches the JVM as the literal string $(hostname) +# This is intentional — executing user-supplied commands is unsafe +``` + +--- + +## Quick migration checklist + +1. **Remove extra backslashes.** Replace `\\\\` with `\\` — the old pattern + survived two shell parse layers (eval) which no longer exist. + +2. **`\$VAR` still works.** Keep any `\$VAR` escapes you have — they are + honoured and pass the literal `$VAR` text to the JVM in both buildpacks. + +3. **Cron / glob expressions.** Remove any protective quoting that was needed + to survive `eval` — write the expression directly. + +4. **Command substitutions.** If you relied on `$(cmd)` being executed in + `JAVA_OPTS` (e.g. `$(hostname)`, `$(cat /etc/myconfig)`), that no longer + works. Compute the value before the app starts and set it as a separate + environment variable, then reference it via `$MYVAR` in `JAVA_OPTS`. + +--- + +## References + +- [Java Options Framework](framework-java_opts.md) +- Issue [#1301](https://github.com/cloudfoundry/java-buildpack/issues/1301) — remove `eval` from start command From 955a37692d4677edf68232f262087613089515d7 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 07:44:01 +0000 Subject: [PATCH 16/24] test(integration): add javaexec regression tests for #1301 Add integration-level smoke tests verifying that command substitutions in user JAVA_OPTS are not executed and that shell metacharacters do not break app launch under the javaexec launcher (Spring Boot, Play). Note in README that JAVA_OPTS character-fidelity is covered at unit level in java_opts_writer_test.go so integration tests focus on end-to-end launch correctness only. --- src/integration/README.md | 27 +++++++++++++++++++++ src/integration/play_test.go | 17 +++++++++++++ src/integration/spring_boot_test.go | 37 +++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/src/integration/README.md b/src/integration/README.md index 9ecfd7d38..2b6589c13 100644 --- a/src/integration/README.md +++ b/src/integration/README.md @@ -124,6 +124,33 @@ The integration tests cover: - Cached buildpack deployment - No internet access scenarios +### Note: JAVA_OPTS character-fidelity is tested at the unit level, not here (#1301) + +The `#1301` work removed `eval` from the start command: `JAVA_OPTS` is now assembled +by a pure-bash expander and tokenized at launch by the shell-free `javaexec` launcher. +The exact-character behaviour this guarantees — shell metacharacters (`; & | > `), +glob/cron stars, quotes-with-spaces, and backslashes all reaching the JVM as literal +text — is verified **deterministically at the unit level** in +[`../java/frameworks/java_opts_writer_test.go`](../java/frameworks/java_opts_writer_test.go), +which runs the **real** generated assembly script and the **real** `javaexec` +tokenizer end-to-end. + +Do **not** re-assert those exact characters through a docker deployment here. The +switchblade docker harness passes env vars into the container in a way that mangles +some metacharacters (e.g. a user `JAVA_OPTS` `&` was observed arriving as `\&` inside +the container) — that is a **harness artifact, not buildpack behaviour** (the unit +test above confirms the buildpack delivers a clean `&`). A docker integration test +asserting the literal received value would fail for reasons unrelated to the product. + +What this directory **does** cover for `#1301`, because it is a genuine end-to-end +property that must hold in a real container launch: + +- **Command substitution is never executed** — a `$(...)` in user `JAVA_OPTS` reaches + the JVM as literal text (`SpringBoot` suite, asserted via the fixture's `/jvm-args` + endpoint). +- **The `javaexec` launcher actually starts the JVM** for the affected containers, + including `Play` (previously `eval exec java $JAVA_OPTS ...`). + ## Writing New Tests To add a new test: diff --git a/src/integration/play_test.go b/src/integration/play_test.go index baeb04acb..54dd7c907 100644 --- a/src/integration/play_test.go +++ b/src/integration/play_test.go @@ -100,6 +100,23 @@ func testPlay(platform switchblade.Platform, fixtures string) func(*testing.T, s Eventually(deployment).Should(matchers.Serve(Not(BeEmpty()))) }) + // Regression test for #1301: the Play start command now launches the JVM via + // the shell-free javaexec launcher (previously `eval exec java $JAVA_OPTS ...`). + // A command substitution and cron/glob characters in user JAVA_OPTS must not be + // executed or expanded, and must not break launch — the app must still boot. + it("starts via the javaexec launcher with unsafe JAVA_OPTS (#1301)", func() { + deployment, logs, err := platform.Deploy. + WithEnv(map[string]string{ + "BP_JAVA_VERSION": "11", + "JAVA_OPTS": `-DcronExpr="0 */7 * * * *" -Dinject=$(hostname)`, + }). + Execute(name, filepath.Join(fixtures, "containers", "play_2.2_staged")) + Expect(err).NotTo(HaveOccurred(), logs.String) + + Expect(logs.String()).To(ContainSubstring("Java Buildpack")) + Eventually(deployment).Should(matchers.Serve(Not(BeEmpty()))) + }) + it("handles Play 2.2 application without bat file", func() { deployment, logs, err := platform.Deploy. WithEnv(map[string]string{ diff --git a/src/integration/spring_boot_test.go b/src/integration/spring_boot_test.go index 7ad542342..fc691a718 100644 --- a/src/integration/spring_boot_test.go +++ b/src/integration/spring_boot_test.go @@ -156,6 +156,43 @@ func testSpringBoot(platform switchblade.Platform, fixtures string) func(*testin Eventually(deployment).Should(matchers.Serve(ContainSubstring("Hello from Spring Boot"))) }) + // Regression tests for #1301: the start command no longer uses `eval`, and + // JAVA_OPTS is assembled by a pure-bash expander and tokenized at launch by + // the shell-free javaexec launcher. A user-supplied JAVA_OPTS value must + // therefore reach the JVM as literal text — command substitutions are never + // executed, and globs/cron stars/shell metacharacters are never expanded or + // interpreted as operators. These drive the value through the user JAVA_OPTS + // env path (from_environment: true; memory comes from the configured opts), + // then read the JVM's actual received value back via the fixture's /jvm-args + // endpoint (System.getProperty("userProperty")). + memoryOpts := `java_opts: ["-Xmx256M", "-Xms128M", "-Xss512k", "-XX:ReservedCodeCacheSize=120M", "-XX:MetaspaceSize=78643K", "-XX:MaxMetaspaceSize=157286K"]` + + it("does not execute command substitution in JAVA_OPTS (#1301, no eval)", func() { + deployment, logs, err := platform.Deploy. + WithEnv(map[string]string{ + "BP_JAVA_VERSION": "17", + "JBP_CONFIG_JAVA_OPTS": `{from_environment: true, ` + memoryOpts + `}`, + // $(hostname) would run under the old eval start command. It must + // instead arrive at the JVM verbatim. + "JAVA_OPTS": `-DuserProperty=$(hostname)`, + }). + Execute(name, filepath.Join(fixtures, "containers", "spring_boot_staged")) + Expect(err).NotTo(HaveOccurred(), logs.String) + + Eventually(deployment).Should(matchers.Serve( + ContainSubstring("userProperty=$(hostname)"), // literal, not the executed hostname + ).WithEndpoint("/jvm-args")) + }) + + // NOTE: glob/cron preservation and shell-metacharacter/ampersand/backslash + // fidelity are covered deterministically at the unit level in + // frameworks/java_opts_writer_test.go, which runs the real assembly script + // and the real javaexec tokenizer. They are intentionally not duplicated as + // docker integration tests (the switchblade docker harness mangles some + // metacharacters when passing env vars, which is a harness artifact, not + // buildpack behaviour). The command-substitution case above stays here + // because non-execution is the security property worth proving end-to-end. + it("applies framework opts without any configured JAVA_OPTS", func() { deployment, logs, err := platform.Deploy. WithEnv(map[string]string{ From 25c599b04c044768a9aa213a69444f57633dbf5b Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 10:01:05 +0000 Subject: [PATCH 17/24] test(java_opts): add end-to-end javaexec tests for issue #1301 scenarios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add runStartCommand tests (full assembly + javaexec tokenization) for the exact reproducer from #1301 and two additional shell-metacharacter cases that were only tested at the profile.d output level: - Combined quoted-value + cron: '-Dfoo="bar baz" -DcronSched="0 */7 * * * *"' verifies both args arrive with correct boundaries (previously xargs stripped quotes → * glob-expanded → ClassNotFoundException) - Pipe character: -Dpattern=foo|bar passes through without shell pipe - Shell metacharacters &, ;, > treated as literals by javaexec Refs #1301 --- src/java/frameworks/java_opts_writer_test.go | 39 ++++++++++++++++++-- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index 450264ee6..7edf28eca 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -339,9 +339,9 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring(`-Dpath=C:\\double`)) }) - // Full invocation cycle test for issue #1301: - // Verifies that the quoted eval "exec ... $JAVA_OPTS" form delivers the correct - // argument to java — glob chars in $JAVA_OPTS are not expanded. + // Full invocation cycle tests for issue #1301: + // Verify that javaexec tokenizes $JAVA_OPTS without a shell, so glob chars, + // pipes, and shell metacharacters are never expanded or executed. It("does not glob-expand * in cron expression when invoking java", func() { output, err := runStartCommand(`-DcronSched="0 */7 * * * *"`, "$JAVA_OPTS") Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) @@ -349,6 +349,39 @@ var _ = Describe("Java Opts Writer", func() { Expect(strings.TrimSpace(output)).To(Equal("-DcronSched=0 */7 * * * *")) }) + It("delivers exact issue-#1301 reproducer: quoted value + cron as separate args", func() { + // Exact reproducer from the bug report: + // JAVA_OPTS='-Dfoo="bar baz" -DcronSched="0 */7 * * * *"' + // Old xargs path: quotes stripped → "bar baz" splits → * globs → ClassNotFoundException + output, err := runStartCommand(`-Dfoo="bar baz" -DcronSched="0 */7 * * * *"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + Expect(lines).To(HaveLen(2)) + Expect(lines[0]).To(Equal("-Dfoo=bar baz")) + Expect(lines[1]).To(Equal("-DcronSched=0 */7 * * * *")) + }) + + It("passes pipe character in JAVA_OPTS through to java without shell interpretation", func() { + // Old sed-based assembly (5.0.2) used | as sed delimiter, so a | in JAVA_OPTS + // caused a sed syntax error and dropped options. javaexec never invokes a shell. + output, err := runStartCommand(`-Dpattern=foo|bar -Dother=baz`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + Expect(lines).To(HaveLen(2)) + Expect(lines[0]).To(Equal("-Dpattern=foo|bar")) + Expect(lines[1]).To(Equal("-Dother=baz")) + }) + + It("passes shell metacharacters (&, ;, >) through to java as literals", func() { + output, err := runStartCommand(`-Da=x&y -Db=a;b -Dc=a>b`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + Expect(lines).To(HaveLen(3)) + Expect(lines[0]).To(Equal("-Da=x&y")) + Expect(lines[1]).To(Equal("-Db=a;b")) + Expect(lines[2]).To(Equal("-Dc=a>b")) + }) + // Regression test: eval mangles .opts content containing literal double quotes. // e.g. Datadog writes -Ddd.service="myapp" into its .opts file; the inner " // terminates the outer eval "..." string, stripping the value. From 7d87e317ef8e62a8c207f19895e97b2c9fb19ccc Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 10:40:43 +0000 Subject: [PATCH 18/24] fix(jres): use \$DEPS_DIR instead of hardcoded /home/vcap/deps Hardcoded path breaks app startup if CF ever mounts deps elsewhere: - memory_calculator: bad path breaks the && chain before exec javaexec, JVM never starts - jvmkill: bad -agentpath causes JVM to abort on agent load before main() Use \$DEPS_DIR//... in both. The platform sets \$DEPS_DIR before the start command runs; the profile.d assembly script expands it in .opts file content before passing to the JVM. --- src/java/jres/jvmkill.go | 15 ++++----------- src/java/jres/memory_calculator.go | 14 ++++---------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/java/jres/jvmkill.go b/src/java/jres/jvmkill.go index 277207bd4..223e7f690 100644 --- a/src/java/jres/jvmkill.go +++ b/src/java/jres/jvmkill.go @@ -176,20 +176,13 @@ func (j *JVMKillAgent) Finalize() error { return nil } -// convertToRuntimePath converts absolute staging path to runtime absolute path -// Example: /tmp/contents.../deps//jre/bin/jvmkill-1.16.0.so -> /home/vcap/deps//jre/bin/jvmkill-1.16.0.so -// Note: We use absolute path instead of $DEPS_DIR because startup scripts run before .profile.d scripts -// are sourced, so $DEPS_DIR is not yet available at runtime. +// convertToRuntimePath converts a staging path to a runtime path using $DEPS_DIR. +// The path ends up in a .opts file that is read by the profile.d assembly script, +// which expands $DEPS_DIR before passing the value to the JVM. func (j *JVMKillAgent) convertToRuntimePath(stagingPath string) string { - // Extract filename and build runtime path - // We know the structure: /deps//jre/bin/jvmkill-VERSION.so - // Runtime path: /home/vcap/deps//jre/bin/jvmkill-VERSION.so - depsIdx := j.ctx.Stager.DepsIdx() filename := filepath.Base(stagingPath) - - // Build absolute runtime path (Cloud Foundry standard location) - return fmt.Sprintf("/home/vcap/deps/%s/jre/bin/%s", depsIdx, filename) + return fmt.Sprintf("$DEPS_DIR/%s/jre/bin/%s", depsIdx, filename) } // getHeapDumpPath checks for volume service with heap-dump tag and returns path diff --git a/src/java/jres/memory_calculator.go b/src/java/jres/memory_calculator.go index 1d7199c52..2f3bf3452 100644 --- a/src/java/jres/memory_calculator.go +++ b/src/java/jres/memory_calculator.go @@ -355,24 +355,18 @@ func (m *MemoryCalculator) GetCalculatorCommand() string { return fmt.Sprintf(`CALCULATED_MEMORY=$(%s) && echo JVM Memory Configuration: $CALCULATED_MEMORY && JAVA_OPTS="$JAVA_OPTS $CALCULATED_MEMORY" && MALLOC_ARENA_MAX=2`, calcCmd) } -// convertToRuntimePath converts a staging path to a runtime path -// Example: /tmp/staging/deps/0/jre/bin/calculator -> /home/vcap/deps/0/jre/bin/calculator +// convertToRuntimePath converts a staging path to a runtime path using $DEPS_DIR. +// Example: /tmp/staging/deps/0/jre/bin/calculator -> $DEPS_DIR/0/jre/bin/calculator func (m *MemoryCalculator) convertToRuntimePath(stagingPath string) string { depsIdx := m.ctx.Stager.DepsIdx() - // Extract the relative path from deps// onwards - // stagingPath: /tmp/.../deps//jre/bin/java-buildpack-memory-calculator-X.X.X - // We want: /home/vcap/deps//jre/bin/java-buildpack-memory-calculator-X.X.X - - // Find "jre/bin/" in the path if idx := strings.Index(stagingPath, "jre/bin/"); idx != -1 { relativePath := stagingPath[idx:] - return fmt.Sprintf("/home/vcap/deps/%s/%s", depsIdx, relativePath) + return fmt.Sprintf("$DEPS_DIR/%s/%s", depsIdx, relativePath) } - // Fallback: just use the filename filename := filepath.Base(stagingPath) - return fmt.Sprintf("/home/vcap/deps/%s/jre/bin/%s", depsIdx, filename) + return fmt.Sprintf("$DEPS_DIR/%s/jre/bin/%s", depsIdx, filename) } // openJDKJREConfig mirrors the memory_calculator section of JBP_CONFIG_OPEN_JDK_JRE. From e787b0aac27aeb6ddb49057b9fdc086acbc42760 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 11:23:14 +0000 Subject: [PATCH 19/24] fix(javaexec): keep \$(...), \${...}, and backtick subs as one token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spaces inside command/parameter substitutions in JAVA_OPTS caused javaexec to split the token. e.g. -Dwhere=\$( hostname | tr '\n' | curl -v 'https://example.com') -Dfoo=\${MY_VAR:-hello world} -Dx=\`hostname -f\` split on the space, and Java received 'hostname' as a class name → ClassNotFoundException, app did not start. Tokenizer now treats \$(...), \${...}, and \`...\` as unbreakable units using depth-tracked delimiters. Nested parens/braces are handled correctly. None are executed; all pass literally to the JVM. profile.d: add \$( / backtick WARNING to the USER_JAVA_OPTS path (was only emitted for .opts file content before). Tests added before fix; full manifest scenario covers all reported edge cases from issue #1301. Refs #1301 --- src/java/frameworks/java_opts_writer.go | 8 ++++ src/java/frameworks/java_opts_writer_test.go | 44 ++++++++++++++++++-- src/java/javaexec/tokenize.go | 42 +++++++++++++++++++ src/java/javaexec/tokenize_test.go | 31 ++++++++++++-- 4 files changed, 117 insertions(+), 8 deletions(-) diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index 2b076bebc..b38a5e1da 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -124,6 +124,14 @@ USER_JAVA_OPTS="${USER_JAVA_OPTS//\\\$/$_escaped_dollar_placeholder}" USER_JAVA_OPTS=$(_expand_env_vars "$USER_JAVA_OPTS") USER_JAVA_OPTS="${USER_JAVA_OPTS//$_escaped_dollar_placeholder/$_dollar}" +# Warn if any command substitution remains in user JAVA_OPTS. It will be +# passed literally to the JVM — command substitutions are never executed. +case "$USER_JAVA_OPTS" in + *'$('*|*"$_backtick"*) + echo "WARNING: JAVA_OPTS contains a command substitution (\$(...) or backtick); it will NOT be executed and will be passed literally to the JVM: $USER_JAVA_OPTS" >&2 + ;; +esac + # Escape replacement-special chars once; these values are loop-invariant. _escaped_deps_dir="${DEPS_DIR//\\/\\\\}" _escaped_deps_dir="${_escaped_deps_dir//&/\\&}" diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index 7edf28eca..c9ad97d8b 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -1,6 +1,7 @@ package frameworks_test import ( + "bytes" "os" "os/exec" "path/filepath" @@ -101,13 +102,24 @@ var _ = Describe("Java Opts Writer", func() { // This sources the profile.d assembly script to build $JAVA_OPTS, then // tokenizes it with the real launcher, returning the argument list java // would receive (one arg per line). + // Stderr is captured separately so that WARNING messages from the + // assembly script do not pollute the JAVA_OPTS value passed to the + // tokenizer (warnings are diagnostic, not part of the value). runStartCommand := func(javaOpts string, optsFileContent string) (string, error) { scriptPath := setupScript(javaOpts, optsFileContent) - assembled, err := runWithEnv(scriptPath, javaOpts, `printf '%s' "$JAVA_OPTS"`) - if err != nil { - return assembled, err + cmd := exec.Command("bash", "-c", "source "+scriptPath+` && printf '%s' "$JAVA_OPTS"`) + cmd.Env = append(os.Environ(), + "JAVA_OPTS="+javaOpts, + "DEPS_DIR="+depsDir, + "HOME=/home/vcap/app", + ) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return stderr.String() + stdout.String(), err } - tokens := javaexec.TokenizeJavaOpts(assembled) + tokens := javaexec.TokenizeJavaOpts(stdout.String()) return strings.Join(tokens, "\n") + "\n", nil } @@ -382,6 +394,30 @@ var _ = Describe("Java Opts Writer", func() { Expect(lines[2]).To(Equal("-Dc=a>b")) }) + It("handles full manifest JAVA_OPTS with all edge cases (issue #1301)", func() { + // Reproduces the exact manifest scenario reported by users: + // JAVA_OPTS: >- + // -Dfoo="bar baz" + // -DcronSched="0 */7 * * * *" + // -Dbar=$HOME + // -Dwhere=$( hostname | tr '\n' | curl -v 'https://testasdkjfhakl.me') + // -Dmyfile=c:\\first\\second\\file.txt;ext + // YAML >- folds newlines to spaces, delivering this as one line. + javaOpts := `-Dfoo="bar baz" -DcronSched="0 */7 * * * *" -Dbar=$HOME -Dwhere=$( hostname | tr '\n' | curl -v 'https://testasdkjfhakl.me') -Dmyfile=c:\\first\\second\\file.txt;ext` + output, err := runStartCommand(javaOpts, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + Expect(lines).To(HaveLen(5)) + Expect(lines[0]).To(Equal("-Dfoo=bar baz")) + Expect(lines[1]).To(Equal("-DcronSched=0 */7 * * * *")) + Expect(lines[2]).To(Equal("-Dbar=/home/vcap/app")) // $HOME expanded by profile.d + // $( hostname | ...) passes literally as one arg — not executed, not split + Expect(lines[3]).To(ContainSubstring("-Dwhere=$(")) + Expect(lines[3]).To(ContainSubstring("hostname")) + // \\ → \ by javaexec; ; is literal + Expect(lines[4]).To(Equal(`-Dmyfile=c:\first\second\file.txt;ext`)) + }) + // Regression test: eval mangles .opts content containing literal double quotes. // e.g. Datadog writes -Ddd.service="myapp" into its .opts file; the inner " // terminates the outer eval "..." string, stripping the value. diff --git a/src/java/javaexec/tokenize.go b/src/java/javaexec/tokenize.go index 418f1b0ab..63d3a9d6b 100644 --- a/src/java/javaexec/tokenize.go +++ b/src/java/javaexec/tokenize.go @@ -60,6 +60,48 @@ func TokenizeJavaOpts(s string) []string { } b.WriteRune(runes[i]) } + case '$': + started = true + b.WriteRune(c) + // Keep $(...) and ${...} as unbreakable units so that spaces inside + // do not split the enclosing token. Neither is executed; both pass + // literally to the JVM. + if i+1 < len(runes) { + next := runes[i+1] + var open, close rune + if next == '(' { + open, close = '(', ')' + } else if next == '{' { + open, close = '{', '}' + } + if open != 0 { + i++ + b.WriteRune(open) + depth := 1 + for i++; i < len(runes) && depth > 0; i++ { + ch := runes[i] + if ch == open { + depth++ + } else if ch == close { + depth-- + } + b.WriteRune(ch) + if depth == 0 { + break + } + } + } + } + case '`': + started = true + b.WriteRune(c) + // Keep `...` as one unbreakable unit (same reasoning as $(...)). + for i++; i < len(runes) && runes[i] != '`'; i++ { + b.WriteRune(runes[i]) + } + if i < len(runes) { + b.WriteRune('`') // closing backtick + } case '\\': started = true // Unquoted backslash escapes the next character. diff --git a/src/java/javaexec/tokenize_test.go b/src/java/javaexec/tokenize_test.go index d8da295ef..a46cc827b 100644 --- a/src/java/javaexec/tokenize_test.go +++ b/src/java/javaexec/tokenize_test.go @@ -36,15 +36,38 @@ func TestTokenizeJavaOpts(t *testing.T) { {"semicolon literal", "-Dx=a;b", []string{"-Dx=a;b"}}, {"redirect literal", "-Dx=a>b", []string{"-Dx=a>b"}}, - // Security: command substitution is NOT executed; passed through literally. - // Unquoted, the space inside splits it into two literal tokens (correct - // word-splitting) — neither is executed. - {"command substitution unquoted", "-Dx=$(echo HACK)", []string{"-Dx=$(echo", "HACK)"}}, + // Security: command substitution is NOT executed; passed through literally + // as a single token so the app does not crash with "class not found". + {"command substitution unquoted", "-Dx=$(echo HACK)", []string{"-Dx=$(echo HACK)"}}, + {"command substitution with pipe", "-Dx=$(hostname | tr a b)", []string{"-Dx=$(hostname | tr a b)"}}, + {"command substitution nested parens", "-Dx=$(foo (bar))", []string{"-Dx=$(foo (bar))"}}, // Quoted, it stays a single literal argument and is never executed. {"command substitution quoted", `-Dx="$(echo danger)"`, []string{"-Dx=$(echo danger)"}}, {"backtick literal", "-Dx=`id`", []string{"-Dx=`id`"}}, + {"backtick with spaces", "-Dx=`hostname -f`", []string{"-Dx=`hostname -f`"}}, + + // ${...} extended bash forms with spaces — pass literally to JVM as one token. + {"brace default with spaces", "-Dx=${MY_VAR:-hello world}", []string{"-Dx=${MY_VAR:-hello world}"}}, + {"brace replacement with spaces", "-Dx=${MY_VAR//foo/bar baz}", []string{"-Dx=${MY_VAR//foo/bar baz}"}}, + {"brace simple no spaces", "-Dx=${MY_VAR}", []string{"-Dx=${MY_VAR}"}}, + {"brace nested braces in pattern", "-Dx=${MY_VAR//foo{bar}/baz qux}", []string{"-Dx=${MY_VAR//foo{bar}/baz qux}"}}, + {"brace assign default with spaces", "-Dx=${MY_VAR:=hello world}", []string{"-Dx=${MY_VAR:=hello world}"}}, {"dollar var literal", "-Dx=$HOME", []string{"-Dx=$HOME"}}, + // Full manifest scenario (issue #1301 reproducer with all edge cases): + // -Dfoo="bar baz" -DcronSched="0 */7 * * * *" -Dbar=$HOME + // -Dwhere=$( hostname | tr '\n' | curl -v 'https://example.me') + // -Dmyfile=c:\\first\\second\\file.txt;ext + // $HOME is already expanded by profile.d before javaexec sees it. + {"full manifest scenario", `-Dfoo="bar baz" -DcronSched="0 */7 * * * *" -Dbar=/home/vcap/app -Dwhere=$( hostname | tr '\n' | curl -v 'https://example.me') -Dmyfile=c:\\first\\second\\file.txt;ext`, + []string{ + "-Dfoo=bar baz", + "-DcronSched=0 */7 * * * *", + "-Dbar=/home/vcap/app", + `-Dwhere=$( hostname | tr '\n' | curl -v 'https://example.me')`, + `-Dmyfile=c:\first\second\file.txt;ext`, + }}, + // Backslash follows POSIX shell rules (parity with previous eval form): // unquoted backslash escapes the next character. {"escaped space joins", `a\ b`, []string{"a b"}}, From 32435748120db5bfadb1cedc45412ab13e12f7d4 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 11:30:32 +0000 Subject: [PATCH 20/24] test(java_opts): document POSIX word-split behavior for runtime env vars with spaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vars whose values contain spaces split into multiple JVM args when unquoted (identical to old eval behavior). Double-quoting the reference — "-Dfoo=$VAR" — passes the expanded value as one token via javaexec's double-quote handler. Three end-to-end tests cover: unquoted split, double-quoted user JAVA_OPTS, double-quoted .opts file content. --- src/java/frameworks/java_opts_writer_test.go | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index c9ad97d8b..a2dcb112a 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -432,5 +432,37 @@ var _ = Describe("Java Opts Writer", func() { Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) Expect(output).To(ContainSubstring(`-Dpattern=foo\|bar`)) }) + + // Expanded variable values with spaces: same POSIX rules as old eval. + // Unquoted $VAR whose value contains spaces → split (javaexec treats spaces as + // word separators). Double-quoted "$VAR" → one token. Quote your references. + It("splits unquoted expanded $VAR with spaces into separate tokens (POSIX word-split)", func() { + os.Setenv("MY_SPACED_VAR", "hello world") + defer os.Unsetenv("MY_SPACED_VAR") + output, err := runStartCommand(`-Dfoo=$MY_SPACED_VAR`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + lines := strings.Split(strings.TrimSpace(output), "\n") + // Unquoted: space splits into two JVM arguments, same as old eval. + Expect(lines).To(HaveLen(2)) + Expect(lines[0]).To(Equal("-Dfoo=hello")) + Expect(lines[1]).To(Equal("world")) + }) + + It(`keeps double-quoted "$VAR" with spaces as one JVM argument`, func() { + os.Setenv("MY_SPACED_VAR", "hello world") + defer os.Unsetenv("MY_SPACED_VAR") + output, err := runStartCommand(`-Dfoo="$MY_SPACED_VAR"`, "$JAVA_OPTS") + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + // Double-quoted: javaexec treats the quoted region as one token. + Expect(strings.TrimSpace(output)).To(Equal("-Dfoo=hello world")) + }) + + It(`keeps double-quoted "$VAR" with spaces in .opts content as one JVM argument`, func() { + os.Setenv("MY_SPACED_VAR", "hello world") + defer os.Unsetenv("MY_SPACED_VAR") + output, err := runStartCommand("", `-Dfoo="$MY_SPACED_VAR"`) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(strings.TrimSpace(output)).To(Equal("-Dfoo=hello world")) + }) }) }) From c44a2c7cc206b4833378e4a5bae665d113cd4328 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 10 Jun 2026 12:19:19 +0000 Subject: [PATCH 21/24] fix(java_opts): warning shows only matching $(...) token, not full JAVA_OPTS Full value in the warning was noisy for long option sets and risked leaking secrets embedded in JAVA_OPTS. Extract just the first $(...) or backtick token so the user can identify the offending substitution without exposing unrelated values. --- src/java/frameworks/java_opts_writer.go | 12 ++++++++++-- src/java/frameworks/java_opts_writer_test.go | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/java/frameworks/java_opts_writer.go b/src/java/frameworks/java_opts_writer.go index b38a5e1da..77b9a0d28 100644 --- a/src/java/frameworks/java_opts_writer.go +++ b/src/java/frameworks/java_opts_writer.go @@ -126,9 +126,17 @@ USER_JAVA_OPTS="${USER_JAVA_OPTS//$_escaped_dollar_placeholder/$_dollar}" # Warn if any command substitution remains in user JAVA_OPTS. It will be # passed literally to the JVM — command substitutions are never executed. +# Show only the offending token, not the full value (which may be long or contain secrets). case "$USER_JAVA_OPTS" in - *'$('*|*"$_backtick"*) - echo "WARNING: JAVA_OPTS contains a command substitution (\$(...) or backtick); it will NOT be executed and will be passed literally to the JVM: $USER_JAVA_OPTS" >&2 + *'$('*) + _warn_match="${USER_JAVA_OPTS#*'$('}" + _warn_match="\$(${_warn_match%%%%')'*})" + echo "WARNING: JAVA_OPTS contains command substitution; it will NOT be executed and will be passed literally to the JVM. Matching: ${_warn_match}" >&2 + ;; + *"$_backtick"*) + _warn_match="${USER_JAVA_OPTS#*"$_backtick"}" + _warn_match="${_backtick}${_warn_match%%%%"$_backtick"*}${_backtick}" + echo "WARNING: JAVA_OPTS contains command substitution (backtick); it will NOT be executed and will be passed literally to the JVM. Matching: ${_warn_match}" >&2 ;; esac diff --git a/src/java/frameworks/java_opts_writer_test.go b/src/java/frameworks/java_opts_writer_test.go index a2dcb112a..ad10bb5fb 100644 --- a/src/java/frameworks/java_opts_writer_test.go +++ b/src/java/frameworks/java_opts_writer_test.go @@ -211,6 +211,25 @@ var _ = Describe("Java Opts Writer", func() { Expect(output).To(ContainSubstring("WARNING: unresolved command substitution")) }) + It("warning for user JAVA_OPTS command substitution shows matching token, not full value", func() { + // Warning must identify the offending $(...) fragment without dumping + // the entire JAVA_OPTS string (which may be long or contain secrets). + // Use `true` as bashExpr so only the WARNING (stderr) appears in combined output. + scriptPath := setupScript("", "$JAVA_OPTS") + output, err := runWithEnv(scriptPath, + `-Dsafe=before $( hostname | wc -l ) -Dsafe=after`, + `true`, + ) + Expect(err).NotTo(HaveOccurred(), "script failed with output: %s", output) + Expect(output).To(ContainSubstring("WARNING")) + // Warning shows the $(...) fragment + Expect(output).To(ContainSubstring("$(")) + Expect(output).To(ContainSubstring("hostname")) + // Warning must NOT dump the full JAVA_OPTS value + Expect(output).NotTo(ContainSubstring("-Dsafe=before")) + Expect(output).NotTo(ContainSubstring("-Dsafe=after")) + }) + It("does not execute command substitutions embedded in opts file content", func() { marker := filepath.Join(cacheDir, "pwned_marker") Expect(os.Remove(marker)).To(Or(Succeed(), MatchError(os.ErrNotExist))) From 534a6580de5e0baf7d7f43373c03884f8ad27ff4 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Thu, 11 Jun 2026 04:46:29 +0000 Subject: [PATCH 22/24] fix(finalize): build javaexec in bin/finalize for source/git buildpack usage bin/finalize only built src/java/finalize/cli into a temp dir; packaged buildpacks got bin/javaexec from scripts/build.sh, but source/git usage (e.g. WithBuildpacks("https://github.com/cloudfoundry/java-buildpack.git")) had no bin/javaexec and finalize aborted before release completed. - bin/finalize now also builds src/java/javaexec/cli into the same temp dir and passes the path via JAVAEXEC_BINARY_PATH. - InstallJavaexecLauncher() (renamed from private) prefers JAVAEXEC_BINARY_PATH and falls back to /bin/javaexec for packaged buildpacks. - Three unit tests cover packaged path, source/git env-var override, and the missing-binary error case. --- bin/finalize | 4 ++- src/java/finalize/finalize.go | 30 +++++++++++++++++------ src/java/finalize/finalize_test.go | 39 ++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/bin/finalize b/bin/finalize index 5061e97b5..3c33ad487 100755 --- a/bin/finalize +++ b/bin/finalize @@ -14,6 +14,8 @@ output_dir=$(mktemp -d -t finalizeXXX) pushd $BUILDPACK_DIR > /dev/null echo "-----> Running go build finalize" GOROOT=$GoInstallDir $GoInstallDir/bin/go build -mod=vendor -o $output_dir/finalize ./src/java/finalize/cli +echo "-----> Running go build javaexec" +GOROOT=$GoInstallDir $GoInstallDir/bin/go build -mod=vendor -o $output_dir/javaexec ./src/java/javaexec/cli popd > /dev/null -$output_dir/finalize "$BUILD_DIR" "$CACHE_DIR" "$DEPS_DIR" "$DEPS_IDX" "$PROFILE_DIR" +JAVAEXEC_BINARY_PATH=$output_dir/javaexec $output_dir/finalize "$BUILD_DIR" "$CACHE_DIR" "$DEPS_DIR" "$DEPS_IDX" "$PROFILE_DIR" diff --git a/src/java/finalize/finalize.go b/src/java/finalize/finalize.go index aea61f875..16b50598f 100644 --- a/src/java/finalize/finalize.go +++ b/src/java/finalize/finalize.go @@ -115,7 +115,7 @@ func Run(f *Finalizer) error { } // Install the javaexec launcher binary used by the start command. - if err := f.installJavaexecLauncher(); err != nil { + if err := f.InstallJavaexecLauncher(); err != nil { f.Log.Error("Failed to install javaexec launcher: %s", err.Error()) return err } @@ -198,13 +198,20 @@ func (f *Finalizer) finalizeFrameworks(ctx *common.Context) error { return nil } -// installJavaexecLauncher copies the javaexec launcher binary from the -// buildpack's bin/ directory into the dependency directory so it is present in -// the droplet at runtime. The start command (see containers.JavaExecCommand) -// invokes it as $DEPS_DIR//bin/javaexec to tokenize JAVA_OPTS without a -// shell. The binary is required: if it is missing the generated start command -// would fail at launch, so this returns an error rather than continuing. -func (f *Finalizer) installJavaexecLauncher() error { +// InstallJavaexecLauncher copies the javaexec launcher binary into the +// dependency directory so it is present in the droplet at runtime. The start +// command (see containers.JavaExecCommand) invokes it as +// $DEPS_DIR//bin/javaexec to tokenize JAVA_OPTS without a shell. +// +// Source path resolution (first match wins): +// 1. JAVAEXEC_BINARY_PATH env var — set by bin/finalize for source/git usage +// where javaexec is built into a temp dir alongside the finalize binary. +// 2. /bin/javaexec — present in packaged buildpacks built by +// scripts/build.sh. +// +// The binary is required: missing it means the generated start command would +// fail at launch, so this returns an error rather than continuing. +func (f *Finalizer) InstallJavaexecLauncher() error { buildpackDir := f.BuildpackDir if buildpackDir == "" { var err error @@ -215,6 +222,13 @@ func (f *Finalizer) installJavaexecLauncher() error { } src := filepath.Join(buildpackDir, "bin", "javaexec") + // For source/git buildpack usage the bin/finalize wrapper builds javaexec + // into a temp directory and passes the path here so the source checkout is + // not mutated. Prefer the override; fall back to /bin/javaexec + // for packaged buildpacks where scripts/build.sh provides the binary. + if override := os.Getenv("JAVAEXEC_BINARY_PATH"); override != "" { + src = override + } data, err := os.ReadFile(src) if err != nil { return fmt.Errorf("javaexec launcher binary not found at %s (required by the start command): %w", src, err) diff --git a/src/java/finalize/finalize_test.go b/src/java/finalize/finalize_test.go index 781eaa354..d756f2c8d 100644 --- a/src/java/finalize/finalize_test.go +++ b/src/java/finalize/finalize_test.go @@ -242,4 +242,43 @@ dependencies: [] Expect(depDir).To(ContainSubstring(depsDir)) }) }) + + Describe("javaexec launcher installation", func() { + It("installs launcher from buildpack bin/ for packaged buildpack usage", func() { + // Default path: /bin/javaexec already written in BeforeEach. + Expect(finalizer.InstallJavaexecLauncher()).To(Succeed()) + installed := filepath.Join(stager.DepDir(), "bin", "javaexec") + Expect(installed).To(BeARegularFile()) + }) + + It("installs launcher from JAVAEXEC_BINARY_PATH for source/git buildpack usage", func() { + // Simulate bin/finalize wrapper: javaexec built into a temp dir, path passed via env var. + // /bin/javaexec is still present from BeforeEach, but the env var must take precedence. + altBinary, err := os.CreateTemp("", "javaexec-alt-*") + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(altBinary.Name()) + _, err = altBinary.WriteString("#!/bin/sh\n# alt\n") + Expect(err).NotTo(HaveOccurred()) + Expect(altBinary.Close()).To(Succeed()) + + os.Setenv("JAVAEXEC_BINARY_PATH", altBinary.Name()) + defer os.Unsetenv("JAVAEXEC_BINARY_PATH") + + Expect(finalizer.InstallJavaexecLauncher()).To(Succeed()) + installed := filepath.Join(stager.DepDir(), "bin", "javaexec") + Expect(installed).To(BeARegularFile()) + content, err := os.ReadFile(installed) + Expect(err).NotTo(HaveOccurred()) + Expect(string(content)).To(ContainSubstring("# alt")) + }) + + It("fails when launcher is missing and JAVAEXEC_BINARY_PATH is not set", func() { + // Point BuildpackDir at an empty dir so bin/javaexec doesn't exist. + emptyDir, err := os.MkdirTemp("", "finalize-no-javaexec") + Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(emptyDir) + finalizer.BuildpackDir = emptyDir + Expect(finalizer.InstallJavaexecLauncher()).NotTo(Succeed()) + }) + }) }) From 453dc44801795767448df396666f57bed8de57e0 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Thu, 11 Jun 2026 05:07:12 +0000 Subject: [PATCH 23/24] docs: document javaexec binary and source/git buildpack path --- docs/DEVELOPING.md | 22 ++++++++++++++++++++ docs/design.md | 6 ++++++ scripts/test-javaexec-source-path.sh | 30 ++++++++++++++++++++++++++++ src/integration/README.md | 6 ++++++ 4 files changed, 64 insertions(+) create mode 100755 scripts/test-javaexec-source-path.sh diff --git a/docs/DEVELOPING.md b/docs/DEVELOPING.md index 9db4c1bbd..e38682770 100644 --- a/docs/DEVELOPING.md +++ b/docs/DEVELOPING.md @@ -97,6 +97,7 @@ Build the buildpack binaries: This creates executables in the `bin/` directory: - `bin/supply` - Staging phase binary (downloads and installs dependencies) - `bin/finalize` - Finalization phase binary (configures runtime) +- `bin/javaexec` - Shell-free JVM launcher (tokenizes `JAVA_OPTS` without `eval`) ## Project Structure @@ -118,6 +119,7 @@ java-buildpack/ │ ├── jres/ # JRE implementations (7 providers) │ ├── supply/cli/ # Supply phase entrypoint │ ├── finalize/cli/ # Finalize phase entrypoint +│ ├── javaexec/cli/ # Shell-free JVM launcher entrypoint │ ├── resources/ # Resource configuration files │ └── integration/ # Integration tests ├── scripts/ # Build and test scripts @@ -156,6 +158,7 @@ Build for the default platform (Linux): ``` -----> Building supply for linux -----> Building finalize for linux +-----> Building javaexec for linux -----> Build complete ``` @@ -193,6 +196,25 @@ go build -mod vendor -o bin/supply src/java/supply/cli/main.go # Build finalize go build -mod vendor -o bin/finalize src/java/finalize/cli/main.go + +# Build javaexec (shell-free JVM launcher, required at runtime) +go build -mod vendor -o bin/javaexec src/java/javaexec/cli/main.go +``` + +### Source/Git Buildpack Usage + +When deploying with a git URL (`cf push -b https://github.com/.../java-buildpack.git`), +Cloud Foundry runs `bin/finalize` directly from the cloned source. In that mode +`bin/javaexec` does not exist (only packaged buildpacks have it). `bin/finalize` +therefore builds `javaexec` into a temp directory alongside the finalize binary +and passes the path via the `JAVAEXEC_BINARY_PATH` environment variable. +`InstallJavaexecLauncher()` prefers this override and falls back to +`bin/javaexec` for packaged buildpacks. + +To verify this path locally (no CF required): + +```bash +bash scripts/test-javaexec-source-path.sh ``` ## Running Tests diff --git a/docs/design.md b/docs/design.md index 47e91e6b9..8b24afe1b 100644 --- a/docs/design.md +++ b/docs/design.md @@ -186,6 +186,12 @@ During the finalize phase (`bin/finalize`), the buildpack: 1. **Finalizes JRE**: Configures JVM options, memory calculator 2. **Finalizes Frameworks**: Adds agent paths, system properties 3. **Finalizes Container**: Generates launch command +4. **Installs `javaexec`**: Copies the shell-free JVM launcher into + `$DEPS_DIR//bin/javaexec`. The generated start command invokes it + instead of `eval "exec java $JAVA_OPTS ..."` so that `JAVA_OPTS` is + tokenized without shell interpretation. For source/git buildpack usage + `bin/finalize` builds `javaexec` on the fly and passes its path via + `JAVAEXEC_BINARY_PATH` (see [DEVELOPING.md](DEVELOPING.md#sourcegit-buildpack-usage)). Components can: - Read installed dependencies from `$DEPS_DIR//` diff --git a/scripts/test-javaexec-source-path.sh b/scripts/test-javaexec-source-path.sh new file mode 100755 index 000000000..ad3c234c1 --- /dev/null +++ b/scripts/test-javaexec-source-path.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# Manual smoke test for the bin/finalize source/git buildpack path. +# Simulates what bin/finalize does: build javaexec into a temp dir and pass +# the path via JAVAEXEC_BINARY_PATH. Verifies the binary builds and that +# InstallJavaexecLauncher picks up the override (unit tests cover the logic; +# this confirms the wiring end-to-end on this machine). +set -euo pipefail + +BUILDPACK_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +cd "$BUILDPACK_DIR" + +tmpdir=$(mktemp -d) +trap 'rm -rf "$tmpdir"' EXIT + +echo "==> [1/3] Build javaexec from source (as bin/finalize now does)" +go build -mod=vendor -o "$tmpdir/javaexec" ./src/java/javaexec/cli +echo " OK: $tmpdir/javaexec ($(wc -c < "$tmpdir/javaexec") bytes)" + +echo "" +echo "==> [2/3] Build finalize from source" +go build -mod=vendor -o "$tmpdir/finalize" ./src/java/finalize/cli +echo " OK: $tmpdir/finalize" + +echo "" +echo "==> [3/3] Unit tests: InstallJavaexecLauncher with JAVAEXEC_BINARY_PATH override" +go test ./src/java/finalize/ -count=1 -v -run "javaexec launcher" 2>&1 | grep -E "PASS|FAIL|RUN|---" + +echo "" +echo "PASS: source/git buildpack path works." +echo " bin/finalize builds javaexec and passes it via JAVAEXEC_BINARY_PATH." diff --git a/src/integration/README.md b/src/integration/README.md index 2b6589c13..e4580ba09 100644 --- a/src/integration/README.md +++ b/src/integration/README.md @@ -151,6 +151,12 @@ property that must hold in a real container launch: - **The `javaexec` launcher actually starts the JVM** for the affected containers, including `Play` (previously `eval exec java $JAVA_OPTS ...`). +**Source/git buildpack path** (`bin/finalize` building `javaexec` on the fly) is +**not covered** by integration tests here: they use a packaged zip where `bin/javaexec` +is pre-built by `scripts/build.sh`. The source/git path is covered by unit tests in +[`../java/finalize/finalize_test.go`](../java/finalize/finalize_test.go) and can be +verified locally with `bash scripts/test-javaexec-source-path.sh`. + ## Writing New Tests To add a new test: From 5dc7b5718dae449dc5d2156a4ae4313cd2fe774f Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Thu, 11 Jun 2026 05:08:53 +0000 Subject: [PATCH 24/24] test(scripts): add JAVA_OPTS tokenization smoke test to source-path script --- scripts/test-javaexec-source-path.sh | 46 ++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/scripts/test-javaexec-source-path.sh b/scripts/test-javaexec-source-path.sh index ad3c234c1..cb261e1c5 100755 --- a/scripts/test-javaexec-source-path.sh +++ b/scripts/test-javaexec-source-path.sh @@ -1,9 +1,9 @@ #!/bin/bash # Manual smoke test for the bin/finalize source/git buildpack path. # Simulates what bin/finalize does: build javaexec into a temp dir and pass -# the path via JAVAEXEC_BINARY_PATH. Verifies the binary builds and that -# InstallJavaexecLauncher picks up the override (unit tests cover the logic; -# this confirms the wiring end-to-end on this machine). +# the path via JAVAEXEC_BINARY_PATH. Verifies the binary builds, that +# InstallJavaexecLauncher picks up the override, and that javaexec tokenizes +# JAVA_OPTS correctly when actually invoked. set -euo pipefail BUILDPACK_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) @@ -12,19 +12,53 @@ cd "$BUILDPACK_DIR" tmpdir=$(mktemp -d) trap 'rm -rf "$tmpdir"' EXIT -echo "==> [1/3] Build javaexec from source (as bin/finalize now does)" +echo "==> [1/4] Build javaexec from source (as bin/finalize now does)" go build -mod=vendor -o "$tmpdir/javaexec" ./src/java/javaexec/cli echo " OK: $tmpdir/javaexec ($(wc -c < "$tmpdir/javaexec") bytes)" echo "" -echo "==> [2/3] Build finalize from source" +echo "==> [2/4] Build finalize from source" go build -mod=vendor -o "$tmpdir/finalize" ./src/java/finalize/cli echo " OK: $tmpdir/finalize" echo "" -echo "==> [3/3] Unit tests: InstallJavaexecLauncher with JAVAEXEC_BINARY_PATH override" +echo "==> [3/4] Unit tests: InstallJavaexecLauncher with JAVAEXEC_BINARY_PATH override" go test ./src/java/finalize/ -count=1 -v -run "javaexec launcher" 2>&1 | grep -E "PASS|FAIL|RUN|---" +echo "" +echo "==> [4/4] Tokenization smoke test: run javaexec with a fake java binary" + +# Fake java: prints each received argument on its own line. +cat > "$tmpdir/fake-java" << 'EOF' +#!/bin/bash +printf '%s\n' "$@" +EOF +chmod +x "$tmpdir/fake-java" + +# Quoted value with spaces → one token; cron expr with * → literal; $(...) → not executed. +JAVA_OPTS='-Dfoo="bar baz" -DcronSched="0 */7 * * * *" -Dwhere=$(hostname)' \ + "$tmpdir/javaexec" "$tmpdir/fake-java" -jar app.jar 2>/dev/null > "$tmpdir/actual.txt" + +expected="-Dfoo=bar baz +-DcronSched=0 */7 * * * * +-Dwhere=\$(hostname) +-jar +app.jar" + +actual=$(cat "$tmpdir/actual.txt") + +if [ "$actual" = "$expected" ]; then + echo " OK: all tokens correct" +else + echo " FAIL: unexpected output" + echo " expected:" + printf '%s\n' "$expected" | sed 's/^/ /' + echo " got:" + printf '%s\n' "$actual" | sed 's/^/ /' + exit 1 +fi + echo "" echo "PASS: source/git buildpack path works." echo " bin/finalize builds javaexec and passes it via JAVAEXEC_BINARY_PATH." +echo " javaexec tokenizes JAVA_OPTS correctly without shell execution."