Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +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))
23 changes: 23 additions & 0 deletions cli/lucli/services/deploy/cli/DeployAppCli.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ 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.
$registerSecretsForRedaction();
var version = arguments.opts.version ?: "";
var versionOptional = arguments.flags.versionOptional ?: false;
if (!versionOptional && !len(version)) {
Expand Down Expand Up @@ -192,6 +198,23 @@ 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.
*/
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
Expand Down
29 changes: 29 additions & 0 deletions cli/lucli/services/deploy/cli/DeployMainCli.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,9 @@ component {
// remote exit codes was the silent-success bug.
var c = arguments.cmd;
var doRaise = !arguments.allowFail;
// Scrub resolved secret values from any RemoteExecutionFailed summary
// (#3159) — env.clear values ride `docker run ... -e KEY=value`.
$registerSecretsForRedaction();
variables.sshPool.onEach(arguments.hosts, function(ssh, host) {
ssh.run(c, {raise: doRaise});
});
Expand Down Expand Up @@ -718,6 +721,28 @@ component {
return isObject(resolver) ? resolver.all() : {};
}

/**
* Hand the resolved secret VALUES to the pool so $raiseRemoteFailure
* scrubs them from command summaries (#3159). env.clear values
* interpolated from ${SECRET} tokens ride `docker run ... -e KEY=value`,
* so a nonzero exit would otherwise leak them into the
* RemoteExecutionFailed message and CI logs. Reads from the
* most-recent-load resolver, so calling it at dispatch time picks up
* whichever verb's config is live; idempotent and guarded for pools that
* predate $setSecretValues.
*/
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):
* 1. dispatch ensure-cmd (mkdir + touch + chmod 600) so the file is
Expand Down Expand Up @@ -785,6 +810,8 @@ component {
}
var c = arguments.cmd;
var doRaise = !arguments.allowFail;
// Scrub resolved secret values from any RemoteExecutionFailed summary (#3159).
$registerSecretsForRedaction();
// Closures can't write outer locals reliably — collect via a shared struct.
var ctx = {lines: []};
variables.sshPool.sequential(arguments.hosts, function(ssh, host) {
Expand Down Expand Up @@ -826,6 +853,8 @@ component {
if (arrayLen(arguments.hosts) == 0) return;
var c = arguments.cmd;
var doRaise = !arguments.allowFail;
// Scrub resolved secret values from any RemoteExecutionFailed summary (#3159).
$registerSecretsForRedaction();
// Prefer onAny when available (both real SshPool and FakeSshPool
// expose it). Fall back to onEach with a single host otherwise.
if (structKeyExists(variables.sshPool, "onAny")) {
Expand Down
47 changes: 42 additions & 5 deletions cli/lucli/services/deploy/lib/FakeSshPool.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ component {
variables.calls = [];
variables.expectations = {};
variables.connectFailures = {};
// Resolved-secret values to scrub from RemoteExecutionFailed command
// summaries (#3159). Mirrors SshClient — seeded empty, populated via
// $setSecretValues.
variables.$secretValues = [];
return this;
}

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

/**
* Register the set of resolved secret values to redact from
* RemoteExecutionFailed command summaries (#3159).
*
* MIRROR: keep byte-identical with SshClient.$setSecretValues.
*/
public void function $setSecretValues(required array values) {
variables.$secretValues = arguments.values;
}

/**
* Replace every occurrence of each registered secret value with
* [REDACTED]. Empty and trivially short values are skipped so they can't
* mangle unrelated text. A value may appear multiple times (#3159).
*
* MIRROR: keep byte-identical with SshClient.$redactSecrets.
*/
public string function $redactSecrets(required string text) {
var out = arguments.text;
var values = variables.$secretValues ?: [];
for (var v in values) {
if (isSimpleValue(v) && len(v) >= 4) {
out = replace(out, v, "[REDACTED]", "all");
}
}
return out;
}

private any function $makeFakeSsh(required string host) {
var pool = this;
return {
Expand Down
50 changes: 47 additions & 3 deletions cli/lucli/services/deploy/lib/SshClient.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ component {
*/
public SshClient function init(string host = "", struct opts = {}) {
variables.$loader = new modules.wheels.services.deploy.lib.JarLoader();
// Resolved-secret values to scrub from RemoteExecutionFailed command
// summaries (#3159). Seeded empty; deploy verbs register the config's
// resolved secrets via $setSecretValues after loading. Set before the
// deferred-open early return so an unconnected client still redacts.
variables.$secretValues = [];
// Deferred-open pattern: `new SshClient()` with no args is a no-op so
// Lucee's implicit init-on-new doesn't try to connect. Callers use
// `new SshClient().init(host, opts)` to actually open a connection.
Expand Down Expand Up @@ -179,8 +184,15 @@ component {
* output stays scannable when long shell pipelines or noisy stderr would
* otherwise dominate the surfaced error.
*
* MIRROR: FakeSshPool.$raiseRemoteFailure must stay byte-identical to this
* method. If you change the trim limits, throw type, or message template
* Resolved secret values registered via $setSecretValues are scrubbed from
* the command summary BEFORE the trim, so a value sitting on the 200-char
* boundary can't leak a partial fragment (#3159). env.clear values
* interpolated from ${SECRET} tokens ride as `docker run ... -e KEY=value`,
* which would otherwise surface in this message and in CI logs.
*
* MIRROR: FakeSshPool.$raiseRemoteFailure (and $setSecretValues /
* $redactSecrets) must stay byte-identical to this method. If you change
* the trim limits, throw type, message template, or redaction behavior
* here, update the test double in lockstep — tests assert against this
* exact shape regardless of which pool the deploy layer is talking to.
*/
Expand All @@ -193,7 +205,7 @@ component {
if (len(stderr) > 500) {
stderr = left(stderr, 500) & "…";
}
var cmdSummary = arguments.cmd;
var cmdSummary = $redactSecrets(arguments.cmd);
if (len(cmdSummary) > 200) {
cmdSummary = left(cmdSummary, 200) & "…";
}
Expand All @@ -205,6 +217,38 @@ component {
);
}

/**
* Register the set of resolved secret values to redact from
* RemoteExecutionFailed command summaries (#3159). Deploy verbs call this
* with SecretResolver.all()'s values after loading config so env.clear
* values interpolated from ${SECRET} tokens never reach a thrown message.
*
* MIRROR: keep byte-identical with FakeSshPool.$setSecretValues.
*/
public void function $setSecretValues(required array values) {
variables.$secretValues = arguments.values;
}

/**
* Replace every occurrence of each registered secret value with
* [REDACTED]. Empty and trivially short values are skipped so they can't
* mangle unrelated text — a 1-char or 2-char "secret" would otherwise
* shred ordinary command bytes. A value may appear multiple times (several
* -e flags), so every occurrence is replaced (#3159).
*
* MIRROR: keep byte-identical with FakeSshPool.$redactSecrets.
*/
public string function $redactSecrets(required string text) {
var out = arguments.text;
var values = variables.$secretValues ?: [];
for (var v in values) {
if (isSimpleValue(v) && len(v) >= 4) {
out = replace(out, v, "[REDACTED]", "all");
}
}
return out;
}

/**
* SFTP upload a local file to `remotePath`.
*
Expand Down
61 changes: 61 additions & 0 deletions cli/lucli/tests/specs/deploy/cli/DeployAppCliSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,70 @@ 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`. A nonzero exit must surface a redacted summary so
// the secret never lands in CI logs. End-to-end through the loader:
// DeployAppCli registers the resolver's values on the pool.
it("redacts a ${SECRET}-interpolated env.clear value from a failed docker run (issue 3159)", () => {
var proj = $makeClearSecretProject();
try {
// Capture the exact docker run command via dry-run first.
var probe = new cli.lucli.services.deploy.cli.DeployAppCli(
new cli.lucli.services.deploy.lib.FakeSshPool()
);
probe.boot({configPath: proj.config, version: "v1", 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-db-pw-9000");

var fake = new cli.lucli.services.deploy.lib.FakeSshPool();
fake.expect("1.2.3.4", runCmd, {exitCode: 125, stdout: "", stderr: "boom"});
var cli = new cli.lucli.services.deploy.cli.DeployAppCli(fake);
try {
cli.boot({configPath: proj.config, version: "v1"});
fail("expected RemoteExecutionFailed");
} catch (any e) {
expect(e.type).toBe("Wheels.Deploy.RemoteExecutionFailed");
expect(e.message).notToInclude("super-secret-db-pw-9000");
expect(e.message).toInclude("[REDACTED]");
}
} finally {
directoryDelete(proj.root, true);
}
});
});
}

/**
* Temp project: config/deploy.yml with an env.clear value interpolated
* from a ${DB_PASSWORD} token, resolved by .kamal/secrets. No env.secret,
* so the value rides `docker run ... -e DB_PASSWORD=value` (#3159).
*/
private struct function $makeClearSecretProject() {
var root = getTempDirectory() & "/wheels-3159-app-" & 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)#"
& "env: {clear: {DB_PASSWORD: '$#chr(123)#DB_PASSWORD#chr(125)#'}}"
);
fileWrite(
root & "/.kamal/secrets",
"DB_PASSWORD=super-secret-db-pw-9000#chr(10)#REGISTRY_PASSWORD=regpw"
);
return {root: root, config: root & "/config/deploy.yml"};
}

/**
* Temp project: config/deploy.yml declaring env.secret [APP_SECRET],
* resolved by .kamal/secrets.
Expand Down
Loading
Loading