Skip to content

Commit 59d6207

Browse files
bpamiriPeter Amiri
andauthored
fix(cli): redact interpolated secret values on the deploy accessory verb (#3206)
The #3159 fix wired $registerSecretsForRedaction() into DeployAppCli and DeployMainCli, but the standalone `wheels deploy accessory boot|reboot <name>` verbs route through DeployAccessoryCli, which shares no base class and never registered the resolver's secret values on its SshPool. AccessoryCommands embeds env.clear values — including ${SECRET}-interpolated ones — as `docker run ... -e 'KEY=value'` in argv, so a nonzero accessory exit surfaced the raw secret in the Wheels.Deploy.RemoteExecutionFailed message and CI logs: the exact leak #3159 closes, just on the accessory verb. DeployAccessoryCli now calls $registerSecretsForRedaction() in $forEach right after load() (ahead of any $dispatch), reading loader.secretResolver().all() the same way the other two CLIs do. A regression spec drives the leak path end-to-end: an env.clear value interpolated from a ${DB_ROOT_PW} token, a failed accessory docker run, and asserts the value is scrubbed to [REDACTED]. Refs #3159 Signed-off-by: Peter Amiri <petera@pai.com> Co-authored-by: Peter Amiri <petera@pai.com>
1 parent 68fadcf commit 59d6207

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

cli/lucli/services/deploy/cli/DeployAccessoryCli.cfc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ component {
5555
arguments.opts.configPath,
5656
{destination: arguments.opts.destination ?: ""}
5757
);
58+
// Register resolved secret values so a nonzero `docker run ... -e
59+
// KEY=value` exit can't leak an env.clear value interpolated from a
60+
// ${SECRET} token into the RemoteExecutionFailed message / CI logs
61+
// (#3159). The env.secret path already avoids -e via --env-file, but
62+
// env.clear values still ride argv — AccessoryCommands.$envArgs emits
63+
// them as `-e 'KEY=value'`.
64+
$registerSecretsForRedaction();
5865
var accCmds = new modules.wheels.services.deploy.commands.AccessoryCommands(cfg);
5966
var dryRun = arguments.opts.dryRun ?: false;
6067
var targets = (arguments.opts.name == "all")
@@ -121,6 +128,25 @@ component {
121128
return isObject(resolver) ? resolver.all() : {};
122129
}
123130

131+
/**
132+
* Hand the resolved secret VALUES to the pool so $raiseRemoteFailure
133+
* scrubs them from command summaries (#3159). Guarded for pools that
134+
* predate $setSecretValues. Values only — keys are harmless in argv.
135+
* Mirrors DeployAppCli/DeployMainCli (each Cli keeps its own dispatch
136+
* plumbing by design).
137+
*/
138+
private void function $registerSecretsForRedaction() {
139+
if (!isObject(variables.sshPool) || !structKeyExists(variables.sshPool, "$setSecretValues")) {
140+
return;
141+
}
142+
var resolved = $resolvedSecrets();
143+
var values = [];
144+
for (var k in resolved) {
145+
arrayAppend(values, toString(resolved[k]));
146+
}
147+
variables.sshPool.$setSecretValues(values);
148+
}
149+
124150
/**
125151
* Deliver env.secret content to `remotePath` on each host (#2957):
126152
* ensure-cmd (mkdir + touch + chmod 600) first so the file is

cli/lucli/tests/specs/deploy/cli/DeployAccessoryCliSpec.cfc

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,74 @@ component extends="wheels.wheelstest.system.BaseSpec" {
143143
directoryDelete(proj.root, true);
144144
}
145145
});
146+
147+
// Secret redaction in RemoteExecutionFailed (#3159): an env.clear
148+
// value interpolated from a ${SECRET} token rides `docker run ...
149+
// -e KEY=value` (env.clear, NOT env.secret — so it's argv, not the
150+
// --env-file path). A nonzero accessory boot exit must surface a
151+
// redacted summary so the secret never lands in CI logs.
152+
// DeployAccessoryCli registers the resolver's values on the pool
153+
// after load(), same as DeployAppCli/DeployMainCli.
154+
it("redacts a ${SECRET}-interpolated env.clear value from a failed accessory docker run (issue 3159)", () => {
155+
var proj = $makeClearSecretAccessoryProject();
156+
try {
157+
// Capture the exact docker run command via dry-run first.
158+
var probe = new cli.lucli.services.deploy.cli.DeployAccessoryCli(
159+
new cli.lucli.services.deploy.lib.FakeSshPool()
160+
);
161+
probe.boot({configPath: proj.config, name: "db", dryRun: true});
162+
var runCmd = "";
163+
for (var line in probe.dryRunOutput()) {
164+
if (findNoCase("docker run", line)) {
165+
// Strip the "[host] " prefix the dry-run buffer adds.
166+
runCmd = reReplace(line, "^\[[^\]]*\] ", "");
167+
break;
168+
}
169+
}
170+
expect(runCmd).toInclude("super-secret-acc-pw-9000");
171+
172+
var fake = new cli.lucli.services.deploy.lib.FakeSshPool();
173+
fake.expect("1.2.3.5", runCmd, {exitCode: 125, stdout: "", stderr: "boom"});
174+
var cli = new cli.lucli.services.deploy.cli.DeployAccessoryCli(fake);
175+
try {
176+
cli.boot({configPath: proj.config, name: "db"});
177+
fail("expected RemoteExecutionFailed");
178+
} catch (any e) {
179+
expect(e.type).toBe("Wheels.Deploy.RemoteExecutionFailed");
180+
expect(e.message).notToInclude("super-secret-acc-pw-9000");
181+
expect(e.message).toInclude("[REDACTED]");
182+
}
183+
} finally {
184+
directoryDelete(proj.root, true);
185+
}
186+
});
146187
});
147188
}
148189

190+
/**
191+
* Temp project: a postgres accessory with an env.clear value interpolated
192+
* from a ${DB_ROOT_PW} token, resolved by .kamal/secrets. No accessory
193+
* env.secret, so the value rides `docker run ... -e DB_ROOT_PW=value` in
194+
* argv — the exact leak #3159 closes, on the accessory verb.
195+
*/
196+
private struct function $makeClearSecretAccessoryProject() {
197+
var root = getTempDirectory() & "/wheels-3159-acc-" & createUUID();
198+
directoryCreate(root & "/config", true, true);
199+
directoryCreate(root & "/.kamal", true, true);
200+
fileWrite(
201+
root & "/config/deploy.yml",
202+
"service: demo#chr(10)#image: acme/demo#chr(10)#servers: [1.2.3.4]#chr(10)#"
203+
& "registry: {username: u, password: [REGISTRY_PASSWORD]}#chr(10)#"
204+
& "accessories: {db: {image: 'postgres:16', host: 1.2.3.5, "
205+
& "env: {clear: {DB_ROOT_PW: '$#chr(123)#DB_ROOT_PW#chr(125)#'}}}}"
206+
);
207+
fileWrite(
208+
root & "/.kamal/secrets",
209+
"DB_ROOT_PW=super-secret-acc-pw-9000#chr(10)#REGISTRY_PASSWORD=regpw"
210+
);
211+
return {root: root, config: root & "/config/deploy.yml"};
212+
}
213+
149214
/**
150215
* Temp project: config/deploy.yml with a postgres accessory declaring
151216
* env.secret [POSTGRES_PASSWORD], resolved by .kamal/secrets.

0 commit comments

Comments
 (0)