feat(daemon): redact secrets in ACP command approval displays by lsm · Pull Request #2778 · lsm/HyperNeo

lsm · 2026-08-23T04:12:43Z

Extracted from #2711 (ACP split 8/10, first of five slices). Reference ref: space/acp-split-8-10-feat-daemon-wire-acp-runtime-query-runner.

What this adds

parseAcpCommandWithSpans (shared acp-command.ts): the existing tokenizer now also records each token's raw span in the original line, so callers can rewrite a token in place while preserving the author's quoting. parseAcpCommand is now a thin wrapper over it — no behavior change.
redactCommandSecrets + shellQuote (daemon acp-command.ts): redacts secret-shaped argument names (--token, -p, --api-key, …), header values (Authorization: …, curl -H/--header), curl -u/--user, leading and inline env assignments, URL userinfo (https://user:pass@host), and nested shell scripts (recursively, depth-bounded, span-anchored so quoting survives).
getAcpCommandIdentityDigest now hashes the redacted identity, so rotating a credential value no longer starts a fresh ACP conversation — only a real command change does.
New acp-command.test.ts (26 tests) pinning the redactor and the redaction-aware digest.

What this does NOT add (later slices)

No call sites in the query runner yet — the terminal-approval display that consumes redactCommandSecrets/shellQuote lands in the host-callbacks slice. All exports are exercised by tests, so knip stays green.

Scope freeze

Redaction coverage is exactly what #2711 converged on after review. Further secret-vector hardening belongs in follow-up issues, not this stack.

Verification

bun test packages/daemon/tests/unit/lib/acp/acp-command.test.ts — 26/26 pass
knip, tsc --build --noEmit, biome format — clean
Full local ACP suites match the clean-dev baseline exactly (28 pre-existing sandbox-only failures, none new)

Add parseAcpCommandWithSpans to the shared ACP command parser so callers can map parsed tokens back to their raw quoting spans, and a redactCommandSecrets engine in the daemon that redacts secret-shaped flags, env assignments, header values, URL userinfo, and nested shell scripts before a command is shown in approval prompts. The command-identity digest now hashes the redacted identity so rotating a credential does not start a fresh ACP conversation. Extracted from #2711 (ACP split 8/10).

lsm · 2026-08-23T04:12:49Z

@codex review

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d1fb066d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-23T04:21:23Z

+  let inLeadingAssignments =
+    options.shellScript === true ? command.indexOf('=') > 0 || isEnv : true;
+  let currentCommand = command;
+  let awaitingCommandWord = options.shellScript === true || isEnv;


Keep curl as the current command after positional arguments

When redacting a nested shell script such as sh -c 'curl https://api -H "Authorization: Bearer topsecret"', awaitingCommandWord starts true even though parsed.command is already curl, so the URL replaces currentCommand. The later -H or -u is consequently treated as belonging to the URL and the credential is returned unchanged for an approval display. Only wait for a command word when the parsed command is an assignment or an actual wrapper such as env.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-23T04:21:23Z

+  } catch {
+    return script;


Redact sensitive scripts when nested parsing fails

When an outer ACP command is valid but its shell script is malformed or unsupported by this tokenizer—for example an inner script containing curl --token topsecret '—the catch returns the complete script unchanged. Agent-generated commands can therefore expose the exact credential in the redactor output precisely when parsing fails; apply the conservative sensitivity fallback here instead of returning raw text.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-23T04:21:23Z

+  if (depth <= 0) {
+    return SCRIPT_SENSITIVITY_PATTERN.test(script) ? '[redacted script]' : script;


Detect env and URL credentials at the recursion cutoff

Once nested sh -c processing reaches the default recursion limit, the fallback pattern recognizes only secret-shaped flags and -h/-u. A deeper leaf such as API_TOKEN=topsecret curl /a or curl https://user:topsecret@host/a therefore passes through verbatim, even though shallower versions are redacted. Include the supported environment-assignment and URL-userinfo forms in the cutoff check, or conservatively redact the remaining script.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-23T04:21:23Z

+function commandBaseName(command: string): string {
+  return (command.split('/').pop() ?? command).toLowerCase();


Normalize Windows executable names before tool-specific redaction

On Windows, commands such as C:\Windows\System32\curl.exe -H "Authorization: Bearer topsecret" https://api never match CURL_COMMAND_NAMES because this helper only splits on / and retains the .exe suffix. The authorization value is therefore returned unchanged; the same issue prevents bash.exe -c ... from receiving nested-script redaction. Normalize Windows path separators and executable suffixes before comparing command names.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-23T04:21:23Z

+            awaitingCommandWord = false;
+          }
+        }
+        redacted.push(redactUrlUserinfo(arg));


Redact URL userinfo inside environment assignments

For a valid wrapper command such as env DATABASE_URL=postgresql://alice:topsecret@db/app agent, the assignment name is not secret-shaped and this call applies the anchored URL matcher to the entire NAME=value token rather than to its value. It consequently returns the database password unchanged even though URL userinfo is otherwise explicitly redacted. Split assignment-shaped positional tokens and apply redactUrlUserinfo to their right-hand side.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-23T04:21:23Z

+    return SCRIPT_SENSITIVITY_PATTERN.test(script) ? '[redacted script]' : script;
+  }
+  try {
+    const parsed = parseAcpCommandWithSpans(script);


Preserve newline command boundaries while redacting shell scripts

For a multiline script such as echo ok\ncurl -H 'Authorization: Bearer topsecret' https://api, this tokenizer discards the newline as ordinary whitespace, so the redaction state never recognizes curl as a new command and returns the authorization header unchanged. Literal newlines are command separators in these supported shells; preserve them as control operators or use shell-aware tokenization before applying command-specific redaction.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-23T04:21:23Z

+    if (tokenCurl && /^-[a-z]*[hu]/i.test(arg)) {
+      const lowered = arg.toLowerCase();
+      let carrierIndex = -1;
+      for (let scan = 1; scan < lowered.length; scan++) {
+        const ch = lowered[scan];
+        if (ch === 'h' || ch === 'u') {
+          carrierIndex = scan;
+          break;
+        }
+        if (!/[a-z]/i.test(ch)) break;
+      }
+      if (carrierIndex > 0) {
+        const value = arg.slice(carrierIndex + 1);
+        const secret = lowered[carrierIndex] === 'u' || isSecretHeaderValue(value);
+        if (secret && value) {
+          redacted.push(`${arg.slice(0, carrierIndex + 1)}[redacted]`);
+          continue;


Stop interpreting curl data values as clustered user flags

The scan treats the first u or h anywhere in an attached alphabetic curl argument as a credential-carrying option, without accounting for an earlier option that consumes the remainder as its value. For example, the curl manual defines -d as request data, but curl -duser=delete-all /a is rewritten to -du[redacted]; different request bodies then collapse to the same digest and the approval display hides a material command effect. Parse curl's value-taking short options before looking for clustered -u or -H.

Useful? React with 👍 / 👎.

lsm · 2026-08-23T04:23:54Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d1fb066d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-23T04:29:18Z

+      index++;
+      continue;
+    }
+    if (tokenCurl && isUserArgName(arg) && next !== undefined && !next.startsWith('--')) {


Redact curl proxy-user credentials

For curl --proxy-user alice:topsecret https://api, this exact-name check rejects --proxy-user, so the proxy password remains in the redacted approval output and changes the identity digest. The installed curl --help all identifies -U, --proxy-user <user:password> as carrying the proxy username and password; treat this long option as a curl credential carrier just like --user.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-23T04:29:18Z

+      if (
+        (valueFlag === 'h' || valueFlag === 'u') &&
+        next !== undefined &&
+        !next.startsWith('--')


Consume dash-prefixed curl user values

When curl credentials themselves begin with two hyphens, as in curl -u --topsecret: https://api, this guard leaves them unredacted and treats them as another option in the identity. Curl actually consumes such an option-looking token as the value of -u—running the installed curl -u --version prompts for the password of user --version rather than printing the version—so known curl value-taking flags should redact their next token regardless of its prefix.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-23T04:29:18Z

+  return shellQuote(token);
+}
+
+const URL_USERINFO_PATTERN = /^([A-Za-z][A-Za-z0-9+.-]*:\/\/[^:/@\s]+:)([^@/\s]+)(@.+)$/;


Redact URL passwords when the username is empty

For a valid userinfo URL such as curl https://:topsecret@example.test/a, the username subpattern requires at least one character, so redactUrlUserinfo returns the entire URL unchanged and the password remains visible in the approval output and identity. Empty usernames are commonly used when an API token occupies the password field; allow the username capture to be empty while still requiring the separating colon.

Useful? React with 👍 / 👎.

lsm · 2026-08-23T04:57:44Z

Closing in favor of a finer split: this slice was too large for effective bot review (round 1 found 10 findings, mostly independent redaction gaps). Reopening as two focused PRs — flat redaction engine, then shell-script nesting — which together address all 10 findings. Links to follow. (Part of the #2711 split stack.)

devin-ai-integration Bot reviewed Aug 23, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 23, 2026

View reviewed changes

lsm closed this Aug 23, 2026

lsm deleted the space/acp-split-8a-redaction branch August 23, 2026 04:57

This was referenced Aug 23, 2026

feat(daemon): redact secrets in ACP command approval displays #2781

Closed

ACP security hardening (approval gating + command redaction) — built, intentionally not shipping yet #2782

Open

		if (depth <= 0) {
		return SCRIPT_SENSITIVITY_PATTERN.test(script) ? '[redacted script]' : script;

		function commandBaseName(command: string): string {
		return (command.split('/').pop() ?? command).toLowerCase();

Conversation

lsm commented Aug 23, 2026 • edited by devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this adds

What this does NOT add (later slices)

Scope freeze

Verification

Uh oh!

lsm commented Aug 23, 2026

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

lsm commented Aug 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

lsm commented Aug 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lsm commented Aug 23, 2026 •

edited by devin-ai-integration Bot

Loading