Skip to content

Commit 68fadcf

Browse files
bpamiriPeter Amiri
andauthored
fix(deploy): redact interpolated secret values in RemoteExecutionFailed summaries (#3205)
env.clear values interpolated from ${SECRET} tokens in .kamal/secrets ride `docker run ... -e KEY=value`, so a nonzero remote exit surfaced the raw secret in the Wheels.Deploy.RemoteExecutionFailed message and CI logs (deferred from #3008). SshClient.$raiseRemoteFailure (the byte-identical FakeSshPool mirror too) now scrubs every occurrence of each registered secret value to [REDACTED] BEFORE the 200-char trim, so a value on the boundary can't leak a partial fragment. Empty and trivially short (<4 char) values are skipped so unrelated command text is never mangled. $setSecretValues registers the resolver's values; DeployAppCli/DeployMainCli wire it from ConfigLoader.secretResolver().all() after each load, ahead of any dispatch. Covered via FakeSshPool (redaction, multi-occurrence, no-secret, empty/short, boundary), a SshClient mirror-parity spec (runs without sshd), and an end-to-end DeployAppCli integration test (config -> resolver -> failed run). Fixes #3159 Signed-off-by: Peter Amiri <petera@pai.com> Co-authored-by: Peter Amiri <petera@pai.com>
1 parent 45e0fc5 commit 68fadcf

8 files changed

Lines changed: 366 additions & 8 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- `wheels deploy`: resolved secret values are now redacted (`[REDACTED]`) from `Wheels.Deploy.RemoteExecutionFailed` command summaries before the message is thrown. `env.clear` values interpolated from `${SECRET}` tokens in `.kamal/secrets` ride `docker run ... -e KEY=value`, so a nonzero remote exit previously surfaced the raw secret in the exception message and CI logs. Every occurrence is scrubbed (a value may appear in multiple `-e` flags), redaction happens before the 200-char trim so a value on the boundary can't leak a partial fragment, and empty/trivially short values are skipped so unrelated command text is never mangled (deferred from #3008; [#3159](https://github.com/wheels-dev/wheels/issues/3159))

cli/lucli/services/deploy/cli/DeployAppCli.cfc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ component {
110110
arguments.opts.configPath,
111111
{destination: arguments.opts.destination ?: ""}
112112
);
113+
// Register resolved secret values so a nonzero `docker run ... -e
114+
// KEY=value` exit can't leak an env.clear value interpolated from a
115+
// ${SECRET} token into the RemoteExecutionFailed message / CI logs
116+
// (#3159). The env.secret path already avoids -e via --env-file, but
117+
// env.clear values still ride argv.
118+
$registerSecretsForRedaction();
113119
var version = arguments.opts.version ?: "";
114120
var versionOptional = arguments.flags.versionOptional ?: false;
115121
if (!versionOptional && !len(version)) {
@@ -192,6 +198,23 @@ component {
192198
return isObject(resolver) ? resolver.all() : {};
193199
}
194200

201+
/**
202+
* Hand the resolved secret VALUES to the pool so $raiseRemoteFailure
203+
* scrubs them from command summaries (#3159). Guarded for pools that
204+
* predate $setSecretValues. Values only — keys are harmless in argv.
205+
*/
206+
private void function $registerSecretsForRedaction() {
207+
if (!isObject(variables.sshPool) || !structKeyExists(variables.sshPool, "$setSecretValues")) {
208+
return;
209+
}
210+
var resolved = $resolvedSecrets();
211+
var values = [];
212+
for (var k in resolved) {
213+
arrayAppend(values, toString(resolved[k]));
214+
}
215+
variables.sshPool.$setSecretValues(values);
216+
}
217+
195218
/**
196219
* Deliver env.secret content to `remotePath` on each host (#2957):
197220
* ensure-cmd (mkdir + touch + chmod 600) first so the file is

cli/lucli/services/deploy/cli/DeployMainCli.cfc

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,9 @@ component {
566566
// remote exit codes was the silent-success bug.
567567
var c = arguments.cmd;
568568
var doRaise = !arguments.allowFail;
569+
// Scrub resolved secret values from any RemoteExecutionFailed summary
570+
// (#3159) — env.clear values ride `docker run ... -e KEY=value`.
571+
$registerSecretsForRedaction();
569572
variables.sshPool.onEach(arguments.hosts, function(ssh, host) {
570573
ssh.run(c, {raise: doRaise});
571574
});
@@ -718,6 +721,28 @@ component {
718721
return isObject(resolver) ? resolver.all() : {};
719722
}
720723

724+
/**
725+
* Hand the resolved secret VALUES to the pool so $raiseRemoteFailure
726+
* scrubs them from command summaries (#3159). env.clear values
727+
* interpolated from ${SECRET} tokens ride `docker run ... -e KEY=value`,
728+
* so a nonzero exit would otherwise leak them into the
729+
* RemoteExecutionFailed message and CI logs. Reads from the
730+
* most-recent-load resolver, so calling it at dispatch time picks up
731+
* whichever verb's config is live; idempotent and guarded for pools that
732+
* predate $setSecretValues.
733+
*/
734+
private void function $registerSecretsForRedaction() {
735+
if (!isObject(variables.sshPool) || !structKeyExists(variables.sshPool, "$setSecretValues")) {
736+
return;
737+
}
738+
var resolved = $resolvedSecrets();
739+
var values = [];
740+
for (var k in resolved) {
741+
arrayAppend(values, toString(resolved[k]));
742+
}
743+
variables.sshPool.$setSecretValues(values);
744+
}
745+
721746
/**
722747
* Deliver env.secret content to `remotePath` on each host (#2957):
723748
* 1. dispatch ensure-cmd (mkdir + touch + chmod 600) so the file is
@@ -785,6 +810,8 @@ component {
785810
}
786811
var c = arguments.cmd;
787812
var doRaise = !arguments.allowFail;
813+
// Scrub resolved secret values from any RemoteExecutionFailed summary (#3159).
814+
$registerSecretsForRedaction();
788815
// Closures can't write outer locals reliably — collect via a shared struct.
789816
var ctx = {lines: []};
790817
variables.sshPool.sequential(arguments.hosts, function(ssh, host) {
@@ -826,6 +853,8 @@ component {
826853
if (arrayLen(arguments.hosts) == 0) return;
827854
var c = arguments.cmd;
828855
var doRaise = !arguments.allowFail;
856+
// Scrub resolved secret values from any RemoteExecutionFailed summary (#3159).
857+
$registerSecretsForRedaction();
829858
// Prefer onAny when available (both real SshPool and FakeSshPool
830859
// expose it). Fall back to onEach with a single host otherwise.
831860
if (structKeyExists(variables.sshPool, "onAny")) {

cli/lucli/services/deploy/lib/FakeSshPool.cfc

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ component {
1515
variables.calls = [];
1616
variables.expectations = {};
1717
variables.connectFailures = {};
18+
// Resolved-secret values to scrub from RemoteExecutionFailed command
19+
// summaries (#3159). Mirrors SshClient — seeded empty, populated via
20+
// $setSecretValues.
21+
variables.$secretValues = [];
1822
return this;
1923
}
2024

@@ -113,10 +117,15 @@ component {
113117
* detail-trim behavior so tests can assert one contract regardless of
114118
* which pool the deploy layer is talking to. Regression #2696.
115119
*
116-
* MIRROR: SshClient.$raiseRemoteFailure is the source of truth. If you
117-
* change the trim limits, throw type, or message template there, update
118-
* this method in lockstep. We don't share the helper because FakeSshPool
119-
* is a test double that should not import the real SSH client.
120+
* MIRROR: SshClient.$raiseRemoteFailure (and $setSecretValues /
121+
* $redactSecrets) is the source of truth. If you change the trim limits,
122+
* throw type, message template, or redaction behavior there, update these
123+
* methods in lockstep. We don't share the helper because FakeSshPool is a
124+
* test double that should not import the real SSH client.
125+
*
126+
* Resolved secret values registered via $setSecretValues are scrubbed from
127+
* the command summary BEFORE the trim so a boundary value can't partially
128+
* leak (#3159).
120129
*/
121130
public void function $raiseRemoteFailure(
122131
required string host,
@@ -127,7 +136,7 @@ component {
127136
if (len(stderr) > 500) {
128137
stderr = left(stderr, 500) & "";
129138
}
130-
var cmdSummary = arguments.cmd;
139+
var cmdSummary = $redactSecrets(arguments.cmd);
131140
if (len(cmdSummary) > 200) {
132141
cmdSummary = left(cmdSummary, 200) & "";
133142
}
@@ -139,6 +148,34 @@ component {
139148
);
140149
}
141150

151+
/**
152+
* Register the set of resolved secret values to redact from
153+
* RemoteExecutionFailed command summaries (#3159).
154+
*
155+
* MIRROR: keep byte-identical with SshClient.$setSecretValues.
156+
*/
157+
public void function $setSecretValues(required array values) {
158+
variables.$secretValues = arguments.values;
159+
}
160+
161+
/**
162+
* Replace every occurrence of each registered secret value with
163+
* [REDACTED]. Empty and trivially short values are skipped so they can't
164+
* mangle unrelated text. A value may appear multiple times (#3159).
165+
*
166+
* MIRROR: keep byte-identical with SshClient.$redactSecrets.
167+
*/
168+
public string function $redactSecrets(required string text) {
169+
var out = arguments.text;
170+
var values = variables.$secretValues ?: [];
171+
for (var v in values) {
172+
if (isSimpleValue(v) && len(v) >= 4) {
173+
out = replace(out, v, "[REDACTED]", "all");
174+
}
175+
}
176+
return out;
177+
}
178+
142179
private any function $makeFakeSsh(required string host) {
143180
var pool = this;
144181
return {

cli/lucli/services/deploy/lib/SshClient.cfc

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ component {
3636
*/
3737
public SshClient function init(string host = "", struct opts = {}) {
3838
variables.$loader = new modules.wheels.services.deploy.lib.JarLoader();
39+
// Resolved-secret values to scrub from RemoteExecutionFailed command
40+
// summaries (#3159). Seeded empty; deploy verbs register the config's
41+
// resolved secrets via $setSecretValues after loading. Set before the
42+
// deferred-open early return so an unconnected client still redacts.
43+
variables.$secretValues = [];
3944
// Deferred-open pattern: `new SshClient()` with no args is a no-op so
4045
// Lucee's implicit init-on-new doesn't try to connect. Callers use
4146
// `new SshClient().init(host, opts)` to actually open a connection.
@@ -179,8 +184,15 @@ component {
179184
* output stays scannable when long shell pipelines or noisy stderr would
180185
* otherwise dominate the surfaced error.
181186
*
182-
* MIRROR: FakeSshPool.$raiseRemoteFailure must stay byte-identical to this
183-
* method. If you change the trim limits, throw type, or message template
187+
* Resolved secret values registered via $setSecretValues are scrubbed from
188+
* the command summary BEFORE the trim, so a value sitting on the 200-char
189+
* boundary can't leak a partial fragment (#3159). env.clear values
190+
* interpolated from ${SECRET} tokens ride as `docker run ... -e KEY=value`,
191+
* which would otherwise surface in this message and in CI logs.
192+
*
193+
* MIRROR: FakeSshPool.$raiseRemoteFailure (and $setSecretValues /
194+
* $redactSecrets) must stay byte-identical to this method. If you change
195+
* the trim limits, throw type, message template, or redaction behavior
184196
* here, update the test double in lockstep — tests assert against this
185197
* exact shape regardless of which pool the deploy layer is talking to.
186198
*/
@@ -193,7 +205,7 @@ component {
193205
if (len(stderr) > 500) {
194206
stderr = left(stderr, 500) & "";
195207
}
196-
var cmdSummary = arguments.cmd;
208+
var cmdSummary = $redactSecrets(arguments.cmd);
197209
if (len(cmdSummary) > 200) {
198210
cmdSummary = left(cmdSummary, 200) & "";
199211
}
@@ -205,6 +217,38 @@ component {
205217
);
206218
}
207219

220+
/**
221+
* Register the set of resolved secret values to redact from
222+
* RemoteExecutionFailed command summaries (#3159). Deploy verbs call this
223+
* with SecretResolver.all()'s values after loading config so env.clear
224+
* values interpolated from ${SECRET} tokens never reach a thrown message.
225+
*
226+
* MIRROR: keep byte-identical with FakeSshPool.$setSecretValues.
227+
*/
228+
public void function $setSecretValues(required array values) {
229+
variables.$secretValues = arguments.values;
230+
}
231+
232+
/**
233+
* Replace every occurrence of each registered secret value with
234+
* [REDACTED]. Empty and trivially short values are skipped so they can't
235+
* mangle unrelated text — a 1-char or 2-char "secret" would otherwise
236+
* shred ordinary command bytes. A value may appear multiple times (several
237+
* -e flags), so every occurrence is replaced (#3159).
238+
*
239+
* MIRROR: keep byte-identical with FakeSshPool.$redactSecrets.
240+
*/
241+
public string function $redactSecrets(required string text) {
242+
var out = arguments.text;
243+
var values = variables.$secretValues ?: [];
244+
for (var v in values) {
245+
if (isSimpleValue(v) && len(v) >= 4) {
246+
out = replace(out, v, "[REDACTED]", "all");
247+
}
248+
}
249+
return out;
250+
}
251+
208252
/**
209253
* SFTP upload a local file to `remotePath`.
210254
*

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,70 @@ component extends="wheels.wheelstest.system.BaseSpec" {
182182
directoryDelete(proj.root, true);
183183
}
184184
});
185+
186+
// Secret redaction in RemoteExecutionFailed (#3159): an env.clear
187+
// value interpolated from a ${SECRET} token rides `docker run ...
188+
// -e KEY=value`. A nonzero exit must surface a redacted summary so
189+
// the secret never lands in CI logs. End-to-end through the loader:
190+
// DeployAppCli registers the resolver's values on the pool.
191+
it("redacts a ${SECRET}-interpolated env.clear value from a failed docker run (issue 3159)", () => {
192+
var proj = $makeClearSecretProject();
193+
try {
194+
// Capture the exact docker run command via dry-run first.
195+
var probe = new cli.lucli.services.deploy.cli.DeployAppCli(
196+
new cli.lucli.services.deploy.lib.FakeSshPool()
197+
);
198+
probe.boot({configPath: proj.config, version: "v1", dryRun: true});
199+
var runCmd = "";
200+
for (var line in probe.dryRunOutput()) {
201+
if (findNoCase("docker run", line)) {
202+
// Strip the "[host] " prefix the dry-run buffer adds.
203+
runCmd = reReplace(line, "^\[[^\]]*\] ", "");
204+
break;
205+
}
206+
}
207+
expect(runCmd).toInclude("super-secret-db-pw-9000");
208+
209+
var fake = new cli.lucli.services.deploy.lib.FakeSshPool();
210+
fake.expect("1.2.3.4", runCmd, {exitCode: 125, stdout: "", stderr: "boom"});
211+
var cli = new cli.lucli.services.deploy.cli.DeployAppCli(fake);
212+
try {
213+
cli.boot({configPath: proj.config, version: "v1"});
214+
fail("expected RemoteExecutionFailed");
215+
} catch (any e) {
216+
expect(e.type).toBe("Wheels.Deploy.RemoteExecutionFailed");
217+
expect(e.message).notToInclude("super-secret-db-pw-9000");
218+
expect(e.message).toInclude("[REDACTED]");
219+
}
220+
} finally {
221+
directoryDelete(proj.root, true);
222+
}
223+
});
185224
});
186225
}
187226

227+
/**
228+
* Temp project: config/deploy.yml with an env.clear value interpolated
229+
* from a ${DB_PASSWORD} token, resolved by .kamal/secrets. No env.secret,
230+
* so the value rides `docker run ... -e DB_PASSWORD=value` (#3159).
231+
*/
232+
private struct function $makeClearSecretProject() {
233+
var root = getTempDirectory() & "/wheels-3159-app-" & createUUID();
234+
directoryCreate(root & "/config", true, true);
235+
directoryCreate(root & "/.kamal", true, true);
236+
fileWrite(
237+
root & "/config/deploy.yml",
238+
"service: demo#chr(10)#image: acme/demo#chr(10)#servers: [1.2.3.4]#chr(10)#"
239+
& "registry: {username: u, password: [REGISTRY_PASSWORD]}#chr(10)#"
240+
& "env: {clear: {DB_PASSWORD: '$#chr(123)#DB_PASSWORD#chr(125)#'}}"
241+
);
242+
fileWrite(
243+
root & "/.kamal/secrets",
244+
"DB_PASSWORD=super-secret-db-pw-9000#chr(10)#REGISTRY_PASSWORD=regpw"
245+
);
246+
return {root: root, config: root & "/config/deploy.yml"};
247+
}
248+
188249
/**
189250
* Temp project: config/deploy.yml declaring env.secret [APP_SECRET],
190251
* resolved by .kamal/secrets.

0 commit comments

Comments
 (0)