diff --git a/cli/lucli/services/deploy/cli/DeployAccessoryCli.cfc b/cli/lucli/services/deploy/cli/DeployAccessoryCli.cfc index c055060d4f..782cea2ce0 100644 --- a/cli/lucli/services/deploy/cli/DeployAccessoryCli.cfc +++ b/cli/lucli/services/deploy/cli/DeployAccessoryCli.cfc @@ -55,6 +55,13 @@ component { arguments.opts.configPath, {destination: arguments.opts.destination ?: ""} ); + // Register resolved secret values so a nonzero `docker run ... -e + // KEY=value` exit can't leak an env.clear value interpolated from a + // ${SECRET} token into the RemoteExecutionFailed message / CI logs + // (#3159). The env.secret path already avoids -e via --env-file, but + // env.clear values still ride argv — AccessoryCommands.$envArgs emits + // them as `-e 'KEY=value'`. + $registerSecretsForRedaction(); var accCmds = new modules.wheels.services.deploy.commands.AccessoryCommands(cfg); var dryRun = arguments.opts.dryRun ?: false; var targets = (arguments.opts.name == "all") @@ -121,6 +128,25 @@ component { return isObject(resolver) ? resolver.all() : {}; } + /** + * Hand the resolved secret VALUES to the pool so $raiseRemoteFailure + * scrubs them from command summaries (#3159). Guarded for pools that + * predate $setSecretValues. Values only — keys are harmless in argv. + * Mirrors DeployAppCli/DeployMainCli (each Cli keeps its own dispatch + * plumbing by design). + */ + private void function $registerSecretsForRedaction() { + if (!isObject(variables.sshPool) || !structKeyExists(variables.sshPool, "$setSecretValues")) { + return; + } + var resolved = $resolvedSecrets(); + var values = []; + for (var k in resolved) { + arrayAppend(values, toString(resolved[k])); + } + variables.sshPool.$setSecretValues(values); + } + /** * Deliver env.secret content to `remotePath` on each host (#2957): * ensure-cmd (mkdir + touch + chmod 600) first so the file is diff --git a/cli/lucli/tests/specs/deploy/cli/DeployAccessoryCliSpec.cfc b/cli/lucli/tests/specs/deploy/cli/DeployAccessoryCliSpec.cfc index b459374ec1..411536fef9 100644 --- a/cli/lucli/tests/specs/deploy/cli/DeployAccessoryCliSpec.cfc +++ b/cli/lucli/tests/specs/deploy/cli/DeployAccessoryCliSpec.cfc @@ -143,9 +143,74 @@ component extends="wheels.wheelstest.system.BaseSpec" { directoryDelete(proj.root, true); } }); + + // Secret redaction in RemoteExecutionFailed (#3159): an env.clear + // value interpolated from a ${SECRET} token rides `docker run ... + // -e KEY=value` (env.clear, NOT env.secret — so it's argv, not the + // --env-file path). A nonzero accessory boot exit must surface a + // redacted summary so the secret never lands in CI logs. + // DeployAccessoryCli registers the resolver's values on the pool + // after load(), same as DeployAppCli/DeployMainCli. + it("redacts a ${SECRET}-interpolated env.clear value from a failed accessory docker run (issue 3159)", () => { + var proj = $makeClearSecretAccessoryProject(); + try { + // Capture the exact docker run command via dry-run first. + var probe = new cli.lucli.services.deploy.cli.DeployAccessoryCli( + new cli.lucli.services.deploy.lib.FakeSshPool() + ); + probe.boot({configPath: proj.config, name: "db", dryRun: true}); + var runCmd = ""; + for (var line in probe.dryRunOutput()) { + if (findNoCase("docker run", line)) { + // Strip the "[host] " prefix the dry-run buffer adds. + runCmd = reReplace(line, "^\[[^\]]*\] ", ""); + break; + } + } + expect(runCmd).toInclude("super-secret-acc-pw-9000"); + + var fake = new cli.lucli.services.deploy.lib.FakeSshPool(); + fake.expect("1.2.3.5", runCmd, {exitCode: 125, stdout: "", stderr: "boom"}); + var cli = new cli.lucli.services.deploy.cli.DeployAccessoryCli(fake); + try { + cli.boot({configPath: proj.config, name: "db"}); + fail("expected RemoteExecutionFailed"); + } catch (any e) { + expect(e.type).toBe("Wheels.Deploy.RemoteExecutionFailed"); + expect(e.message).notToInclude("super-secret-acc-pw-9000"); + expect(e.message).toInclude("[REDACTED]"); + } + } finally { + directoryDelete(proj.root, true); + } + }); }); } + /** + * Temp project: a postgres accessory with an env.clear value interpolated + * from a ${DB_ROOT_PW} token, resolved by .kamal/secrets. No accessory + * env.secret, so the value rides `docker run ... -e DB_ROOT_PW=value` in + * argv — the exact leak #3159 closes, on the accessory verb. + */ + private struct function $makeClearSecretAccessoryProject() { + var root = getTempDirectory() & "/wheels-3159-acc-" & createUUID(); + directoryCreate(root & "/config", true, true); + directoryCreate(root & "/.kamal", true, true); + fileWrite( + root & "/config/deploy.yml", + "service: demo#chr(10)#image: acme/demo#chr(10)#servers: [1.2.3.4]#chr(10)#" + & "registry: {username: u, password: [REGISTRY_PASSWORD]}#chr(10)#" + & "accessories: {db: {image: 'postgres:16', host: 1.2.3.5, " + & "env: {clear: {DB_ROOT_PW: '$#chr(123)#DB_ROOT_PW#chr(125)#'}}}}" + ); + fileWrite( + root & "/.kamal/secrets", + "DB_ROOT_PW=super-secret-acc-pw-9000#chr(10)#REGISTRY_PASSWORD=regpw" + ); + return {root: root, config: root & "/config/deploy.yml"}; + } + /** * Temp project: config/deploy.yml with a postgres accessory declaring * env.secret [POSTGRES_PASSWORD], resolved by .kamal/secrets.