diff --git a/cli/lucli/services/deploy/cli/DeploySecretsCli.cfc b/cli/lucli/services/deploy/cli/DeploySecretsCli.cfc index e1aa38ac47..fd780bd54f 100644 --- a/cli/lucli/services/deploy/cli/DeploySecretsCli.cfc +++ b/cli/lucli/services/deploy/cli/DeploySecretsCli.cfc @@ -65,7 +65,9 @@ component { if (!len(key)) return ""; for (var line in listToArray(text, chr(10), false)) { var eq = find("=", line); - if (eq > 0 && left(line, eq - 1) == key) { + // eq > 1, not > 0: a line starting with '=' has no key, and + // left(line, 0) crashes Lucee 7 (Cross-Engine Invariant 8). + if (eq > 1 && left(line, eq - 1) == key) { return mid(line, eq + 1, 99999); } } diff --git a/cli/lucli/services/deploy/lib/SecretResolver.cfc b/cli/lucli/services/deploy/lib/SecretResolver.cfc index 95272fc81b..f0a08e3d2c 100644 --- a/cli/lucli/services/deploy/lib/SecretResolver.cfc +++ b/cli/lucli/services/deploy/lib/SecretResolver.cfc @@ -7,17 +7,37 @@ * in .kamal/secrets and on load the `op` CLI actually runs. We do the * same — no embedded vault, no network adapter, just `bash -c`. * + * Resolution sources the file in bash (`set -a` exports every assignment + * and bash expands $(cmd) substitutions), then prints each key DECLARED + * IN THE FILE back as a KEYVALUE record (US = chr(31), RS = + * chr(30)). Reading the declared keys individually — instead of diffing + * `env` output against a baseline capture — means: + * - keys that also exist in the parent environment (user exports, CI + * vars) still resolve to the file's value instead of being dropped, + * - multi-line values (TLS certs, SSH keys) survive intact, because + * records are RS-separated rather than newline-separated. + * * Destination overlay: .kamal/secrets loads first, then * .kamal/secrets. (if destination is set and the file * exists) overrides keys. * * Missing file is a no-op — ALL calls to .get() return empty string. + * A present file on a machine where bash can't run throws + * SecretResolver.BashUnavailable instead of silently resolving zero + * secrets (which would let callers proceed with empty credentials). + * A failing command inside the file (e.g. `$(op read …)` when not signed + * in) throws SecretResolver.ResolutionFailed instead of silently + * exporting the key with an empty value. */ component { public SecretResolver function init(struct opts = {}) { variables.projectRoot = arguments.opts.projectRoot ?: expandPath("./"); variables.destination = arguments.opts.destination ?: ""; + // Override when bash isn't reachable as plain `bash` on PATH + // (e.g. a non-standard Git Bash install on Windows). Also the + // seam the BashUnavailable spec uses. + variables.bashCmd = arguments.opts.bashCmd ?: "bash"; variables.resolved = $loadAll(); return this; } @@ -52,67 +72,132 @@ component { } /** - * Run the given secrets file through `bash -c 'set -a; source FILE; env'` - * and parse the resulting env block. The difference between our new env - * and a baseline `env` capture gives us just the keys introduced by the - * file (including $() expansions, since bash resolves those during source). + * Source the secrets file through bash and read back the value of each + * key the file declares, as RS-terminated KEYVALUE records. */ private struct function $resolveFile(required string path) { if (!fileExists(arguments.path)) return {}; - // Step 1: capture baseline env so we can subtract it later. - var baseline = $runBash("env"); - var baselineKeys = $parseEnvKeys(baseline); - - // Step 2: source the file, then emit env. `set -a` exports all vars. - var cmd = "set -a; source " & $shellEscape(arguments.path) & "; env"; - var enriched = $runBash(cmd); + var candidates = $candidateKeys(fileRead(arguments.path, "UTF-8")); + if (!arrayLen(candidates)) return {}; - var out = {}; - for (var line in listToArray(enriched, chr(10), false)) { - var eq = find("=", line); - if (eq < 1) continue; - var key = left(line, eq - 1); - var val = mid(line, eq + 1, 99999); - // Only keep keys introduced by the file (not baseline). - if (!arrayContainsNoCase(baselineKeys, key)) { - out[key] = val; - } + // `set -a` exports every assignment made while sourcing; the file's + // own stdout is discarded so it can't corrupt the record stream. + // `set -e` makes a failing command inside the file — most importantly + // an assignment whose $(cmd) substitution fails, like `$(op read …)` + // when not signed in — abort sourcing with a non-zero exit, so it + // surfaces as ResolutionFailed below instead of silently exporting + // the key with an empty value. (Without -e, bash only reports the + // status of the file's LAST statement, and even that is ignored + // because the script continues into the for-loop.) + // `${!k+x}` (set-check on the indirected name) filters out candidate + // keys bash never actually set — e.g. base64 continuation lines of a + // quoted multi-line value that merely look like assignments. + // \037 = US (key/value separator), \036 = RS (record terminator). + // NUL would be the only byte guaranteed absent from env values, but + // Lucee's chr(0) yields an empty string, so it can't be used as a + // CFML-side delimiter; RS never appears in realistic secret values. + var script = "set -ae; source " & $shellEscape(arguments.path) & " >/dev/null; " + & "for __wheels_key in " & arrayToList(candidates, " ") & "; do " + & "if [ -n ""${!__wheels_key+x}"" ]; then " + & "printf '%s\037%s\036' ""$__wheels_key"" ""${!__wheels_key}""; " + & "fi; done"; + var result = $runBash(script); + if (result.exitCode != 0) { + throw( + type = "SecretResolver.ResolutionFailed", + message = "Resolving secrets from [" & arguments.path & "] failed (bash exit code " & result.exitCode & ").", + detail = result.err + ); } - return out; + return $parseRecords(result.out); } - private array function $parseEnvKeys(required string envBlock) { + /** + * Scan raw file content for the env keys it declares: lines shaped + * `KEY=...` or `export KEY=...`. Lines inside quoted multi-line values + * can produce false candidates; those are filtered by the set-check in + * the resolution script because bash never defines them as variables. + */ + private array function $candidateKeys(required string content) { var keys = []; - for (var line in listToArray(arguments.envBlock, chr(10), false)) { - var eq = find("=", line); - if (eq > 0) arrayAppend(keys, left(line, eq - 1)); + for (var line in listToArray(arguments.content, chr(10), false)) { + var m = reFind("^[ \t]*(export[ \t]+)?([A-Za-z_][A-Za-z0-9_]*)=", line, 1, true); + if (arrayLen(m.pos) >= 3 && m.pos[3] > 0) { + var key = mid(line, m.pos[3], m.len[3]); + // Exact-match dedupe (arrayFind is case-sensitive): FOO and + // foo are distinct bash variables, so both stay candidates. + if (!arrayFind(keys, key)) arrayAppend(keys, key); + } } return keys; } - private string function $runBash(required string cmd) { + /** + * Parse the KEYVALUE records emitted by the resolution script. + * The RS (chr(30)) terminator doesn't appear in realistic secret values, + * so multi-line values pass through intact. The `sep > 1` guard skips + * malformed records and avoids Left(str, 0), which crashes Lucee 7 + * (Cross-Engine Invariant 8). + */ + private struct function $parseRecords(required string blob) { + var out = {}; + for (var rec in listToArray(arguments.blob, chr(30), false)) { + var sep = find(chr(31), rec); + if (sep <= 1) continue; + out[left(rec, sep - 1)] = mid(rec, sep + 1, len(rec)); + } + return out; + } + + /** + * Run a command through local bash, capturing stdout and stderr + * separately. Returns {exitCode, out, err}. Throws + * SecretResolver.BashUnavailable when bash can't be started (e.g. + * Windows without WSL/Git Bash) — surfacing the failure beats silently + * yielding zero secrets. + */ + private struct function $runBash(required string cmd) { + // stderr is redirected to a temp file rather than read from a pipe: + // draining stdout to EOF before touching a piped stderr deadlocks + // when the subprocess fills the OS stderr pipe buffer (~64 KB) — + // e.g. a verbose secret-manager CLI error — because bash blocks on + // the stderr write while we block on the stdout read. A file sink + // never fills, so bash always runs to completion. Secret values + // travel on stdout (read in-memory); only diagnostics touch disk, + // and the file is deleted in the finally block even when waitFor() + // or the throw paths interrupt the happy path. + var errPath = getTempFile(getTempDirectory(), "wheels-secret-err"); try { - var pb = createObject("java", "java.lang.ProcessBuilder").init(["bash", "-c", arguments.cmd]); - pb.redirectErrorStream(true); - var proc = pb.start(); - var reader = createObject("java", "java.io.BufferedReader").init( - createObject("java", "java.io.InputStreamReader").init(proc.getInputStream(), "UTF-8") - ); - var sb = createObject("java", "java.lang.StringBuilder").init(); - var line = reader.readLine(); - while (!isNull(line)) { - sb.append(line); - sb.append(chr(10)); - line = reader.readLine(); + var proc = ""; + try { + var pb = createObject("java", "java.lang.ProcessBuilder").init([variables.bashCmd, "-c", arguments.cmd]); + pb.redirectError(createObject("java", "java.io.File").init(errPath)); + proc = pb.start(); + } catch (any e) { + throw( + type = "SecretResolver.BashUnavailable", + message = "Unable to launch bash to resolve .kamal/secrets: " & e.message, + detail = "Secret resolution requires a local bash for $(cmd) expansion. On Windows, run inside WSL or Git Bash." + ); } - proc.waitFor(); - return sb.toString(); - } catch (any e) { - return ""; + var out = $readStream(proc.getInputStream()); + var exitCode = proc.waitFor(); + var err = fileExists(errPath) ? fileRead(errPath, "UTF-8") : ""; + return {exitCode: exitCode, out: out, err: err}; + } finally { + if (fileExists(errPath)) fileDelete(errPath); } } + private string function $readStream(required any inputStream) { + var scanner = createObject("java", "java.util.Scanner").init(arguments.inputStream, "UTF-8"); + scanner.useDelimiter("\A"); + var content = scanner.hasNext() ? scanner.next() : ""; + scanner.close(); + return content; + } + private string function $shellEscape(required string path) { return "'" & replace(arguments.path, "'", "'\''", "all") & "'"; } diff --git a/cli/lucli/tests/specs/deploy/cli/DeploySecretsCliSpec.cfc b/cli/lucli/tests/specs/deploy/cli/DeploySecretsCliSpec.cfc index ee6e18d659..d0183f05cb 100644 --- a/cli/lucli/tests/specs/deploy/cli/DeploySecretsCliSpec.cfc +++ b/cli/lucli/tests/specs/deploy/cli/DeploySecretsCliSpec.cfc @@ -56,6 +56,14 @@ component extends="wheels.wheelstest.system.BaseSpec" { expect(cli.extract({key: "", from: "FOO=bar"})).toBe(""); }); + it("extract skips malformed lines that start with '='", () => { + // left(line, 0) on a keyless '=...' line crashes Lucee 7 + // (Cross-Engine Invariant 8); such lines must be skipped. + var cli = new cli.lucli.services.deploy.cli.DeploySecretsCli(); + var block = "=stray-value" & chr(10) & "FOO=bar"; + expect(cli.extract({key: "FOO", from: block})).toBe("bar"); + }); + it("resolves 1password and op as the same adapter", () => { var cli = new cli.lucli.services.deploy.cli.DeploySecretsCli(); var stub = new cli.lucli.tests.specs.deploy.secrets._stubs.StubOnePasswordAdapter(); diff --git a/cli/lucli/tests/specs/deploy/lib/SecretResolverSpec.cfc b/cli/lucli/tests/specs/deploy/lib/SecretResolverSpec.cfc index 5cb59a2436..48d36f6a94 100644 --- a/cli/lucli/tests/specs/deploy/lib/SecretResolverSpec.cfc +++ b/cli/lucli/tests/specs/deploy/lib/SecretResolverSpec.cfc @@ -67,6 +67,83 @@ component extends="wheels.wheelstest.system.BaseSpec" { expect(r.has("PRESENT")).toBeTrue(); expect(r.has("MISSING")).toBeFalse(); }); + + it("resolves keys that also exist in the parent environment", () => { + // HOME is always set in the parent env; the old baseline + // subtraction dropped such keys, yielding "" for them. + fileWrite(variables.tempRoot & "/.kamal/secrets", "HOME=/tmp/wheels-secret-override"); + var r = new cli.lucli.services.deploy.lib.SecretResolver({ + projectRoot: variables.tempRoot + }); + expect(r.get("HOME")).toBe("/tmp/wheels-secret-override"); + }); + + it("preserves multi-line quoted values like certificates", () => { + var cert = "-----BEGIN CERTIFICATE-----" & chr(10) + & "dGVzdA==" & chr(10) + & "-----END CERTIFICATE-----"; + fileWrite(variables.tempRoot & "/.kamal/secrets", 'CERT="' & cert & '"'); + var r = new cli.lucli.services.deploy.lib.SecretResolver({ + projectRoot: variables.tempRoot + }); + expect(r.get("CERT")).toBe(cert); + // base64 continuation lines must not be misparsed as keys + expect(r.has("dGVzdA")).toBeFalse(); + }); + + it("preserves values whose continuation lines begin with '='", () => { + // A continuation line starting with '=' previously reached + // left(line, 0), which crashes Lucee 7 (Cross-Engine Invariant 8). + fileWrite(variables.tempRoot & "/.kamal/secrets", + 'WEIRD="line1' & chr(10) & '=line2"'); + var r = new cli.lucli.services.deploy.lib.SecretResolver({ + projectRoot: variables.tempRoot + }); + expect(r.get("WEIRD")).toBe("line1" & chr(10) & "=line2"); + }); + + it("supports export-prefixed declarations", () => { + fileWrite(variables.tempRoot & "/.kamal/secrets", "export TOKEN=abc123"); + var r = new cli.lucli.services.deploy.lib.SecretResolver({ + projectRoot: variables.tempRoot + }); + expect(r.get("TOKEN")).toBe("abc123"); + }); + + it("throws ResolutionFailed when a $(cmd) substitution fails", () => { + // A failing credential-manager command (op not signed in, + // bw locked, …) must abort resolution, not silently export + // an empty value for the key. + fileWrite(variables.tempRoot & "/.kamal/secrets", "BROKEN=$(exit 1)"); + expect(() => { + new cli.lucli.services.deploy.lib.SecretResolver({ + projectRoot: variables.tempRoot + }); + }).toThrow(type="SecretResolver.ResolutionFailed"); + }); + + it("throws ResolutionFailed when the failing command is mid-file", () => { + // Without errexit, bash only reports the LAST statement's + // status, so a mid-file failure followed by a good line + // would slip through. + fileWrite(variables.tempRoot & "/.kamal/secrets", + "BROKEN=$(exit 1)" & chr(10) & "GOOD=ok"); + expect(() => { + new cli.lucli.services.deploy.lib.SecretResolver({ + projectRoot: variables.tempRoot + }); + }).toThrow(type="SecretResolver.ResolutionFailed"); + }); + + it("throws BashUnavailable when bash cannot be launched", () => { + fileWrite(variables.tempRoot & "/.kamal/secrets", "FOO=bar"); + expect(() => { + new cli.lucli.services.deploy.lib.SecretResolver({ + projectRoot: variables.tempRoot, + bashCmd: "/nonexistent/wheels-no-bash-" & createUUID() + }); + }).toThrow(type="SecretResolver.BashUnavailable"); + }); }); } }