Merge pull request #1316 from stokpop/issue-1301-replace-eval-opts-expansion

fix: replace eval-based start command with shell-free javaexec launcher (#1301)

Author: ramonskie

.gitignore

@@ -15,6 +15,7 @@ tmp/
 .cache/
 bin/detect
 bin/finalize
+bin/javaexec
 bin/release
 bin/supply
 /*.md

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"

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

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/<idx>/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/<idx>/`

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

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/<idx>/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)

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

manifest.yml

@@ -9,6 +9,7 @@ include_files:
 - bin/compile
 - bin/detect
 - bin/finalize
+- bin/javaexec
 - bin/release
 - bin/supply
 - manifest.yml

scripts/test-javaexec-source-path.sh

@@ -0,0 +1,64 @@
+#!/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, 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)
+cd "$BUILDPACK_DIR"
+
+tmpdir=$(mktemp -d)
+trap 'rm -rf "$tmpdir"' EXIT
+
+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/4] Build finalize from source"
+go build -mod=vendor -o "$tmpdir/finalize" ./src/java/finalize/cli
+echo "    OK: $tmpdir/finalize"
+
+echo ""
+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."