fix(telemetry): redact a credential written as the word after a flag

[paoValle]

What this changes

scrubCommand is what keeps a credential out of the two copies of a Bot's shell command that outlive the run: the Action Recording audit trail (server/src/admin/controls.ts) and the copy exported to a telemetry collector (server/src/telemetry/otel.ts). Its rules cover a flag whose name says what it carries (--password, API_KEY=…, Authorization:) and mysql's attached -psecret. A credential passed as the word after a flag has no name to match on, so three shapes were kept in clear:

• docker login -p secret, sshpass -p secret and redis-cli -a secret / --pass secret.
• redis-cli -ahunter2, which was worse than a miss: the attached form reads as a flag called -asecret, so the rule for "a flag named after a secret" redacted the word after it and left the password. Measured on main, it produced redis-cli -asecret [REDACTED] k v — the credential still there and the command mangled.
• redis://:secret@host, a URL whose userinfo has no user, which the userinfo rule could not match because it required a character before the colon.
• curl --proxy-user bob:secret, where -u and --user were already covered.

Measured by importing the exported scrubCommand, before (main) and after:

before (main)
  redis-cli -ahunter2 SET k v      => redis-cli -asecret [REDACTED] k v
  redis-cli -a hunter2 SET k v     => redis-cli -a hunter2 SET k v
  redis-cli --pass hunter2 SET k v => redis-cli --pass hunter2 SET k v
  sshpass -p hunter2 ssh user@host => sshpass -p hunter2 ssh user@host
  docker login -u bob -p hunter2   => docker login -u bob -p hunter2
  curl --proxy-user bob:hunter2 …  => curl --proxy-user bob:hunter2 …
  redis://:hunter2@cache:6379      => redis://:hunter2@cache:6379

after
  redis-cli -ahunter2 SET k v      => redis-cli -a[REDACTED] SET k v
  redis-cli -a hunter2 SET k v     => redis-cli -a [REDACTED] SET k v
  redis-cli --pass hunter2 SET k v => redis-cli --pass [REDACTED] SET k v
  sshpass -p hunter2 ssh user@host => sshpass -p [REDACTED] ssh user@host
  docker login -u bob -p hunter2   => docker login -u bob -p [REDACTED]
  curl --proxy-user bob:hunter2 …  => curl --proxy-user bob:[REDACTED] …
  redis://:hunter2@cache:6379      => redis://:[REDACTED]@cache:6379

Each rule keeps the flag and replaces only the value, so the recorded command still reads as what ran, and refuses a value that begins with -, so a bare flag does not swallow the flag after it: redis-cli -a --tls -h host SET k v is recorded unchanged.

Two ordering details, both of which the diff depends on:

• The tool-anchored rules now sit before the name-based ones. That is the fix for -ahunter2: with the name-based rules first, -asecret is consumed as a flag name and the wrong word is redacted. Ordering is the only thing that decides it, since the two rules overlap by construction.
• The mysql rule is left exactly as it was, apart from its comment. mysql -p secret is not a password form — a space there means "ask at the prompt" and secret is the database — so the comment's claim that it was covered was wrong twice: it was never covered, and it should not be. mysql -p mydb and mysql -p secret db are recorded unchanged, with a test for each.

Deliberately not in this change:

• -U, curl's short form of --proxy-user. The same letter is --user-agent to wget and to ssh, so wget -U Mozilla:5 would have had its user agent redacted. --proxy-user is unambiguous and is covered.
• A Cookie: header, openssl … -passin pass:… and --password-stdin (which carries no value). Same class, addable the same way; I kept the diff to the flag-and-value and userinfo shapes, where what is a credential is not a judgement call.
• mysqladmin / mysqlpump, for the same reason as mysql itself: their -p takes the password attached or asks for it, so a space-following word is not one.

Where it runs

All four answers are "none". scrubCommand is a pure function of the string it is given: no new state, nothing serialised or fanned out, no listener, port or schedule, and a second replica scrubs the same bytes as the first.

Boundary and audit

• This changes only the text of the command that is recorded after the fact, not whether the call is allowed, decided or audited. Every acting call still goes through the gateway unchanged.
• The direction is one way: the audit row and the exported copy redact more, and never less.
• No new input path, and nothing new trusted from the client; the same string goes through the same rules.

Changelog

CHANGELOG.md under Unreleased: "A credential written as the word after a flag is redacted", naming each shape that used to survive and stating that a password separated from -p by a space is left alone.

Proof

$ bun test server/tests/enterprise-controls.test.ts server/tests/otel-export-scrubbing.test.ts
 43 pass
 0 fail
 63 expect() calls
Ran 43 tests across 2 files. [394.00ms]

$ cd server && bunx tsc --noEmit      # exit 0
$ bun run format:check                # Checked 1253 files. No fixes applied.
$ bun run lint                        # Checked 1268 files. No fixes applied.

The tests were checked for whether they bite, by running them with server/src/telemetry/scrub.ts reverted to main and the test files kept:

 35 pass
 8 fail

The eight failures are exactly the new cases — seven in enterprise-controls.test.ts, including redis-cli -ahunter2, and one in otel-export-scrubbing.test.ts, which now carries a positional credential through the real export path to a local OTLP collector so the bytes on the wire are what is asserted rather than the function's return value.

Not run locally: bun run test:ci, which needs Postgres. The two suites above are the ones that touch this path.

AI disclosure: I used an AI assistant to audit scrub.ts for shapes the rules read past and to draft this diff, and a second review pass caught that mysql -p secret is not a MySQL password form before this was pushed. Every command and output above was run by me on this branch.

[paoValle]

Rebased onto main (a4dce8c, #765). Force-pushed as c69db55.

The only conflict was CHANGELOG.md: #765 added a newer section at the top of Unreleased. I kept upstream's section first and moved this entry below it. The scrub.ts and test diffs are byte-identical to the original commit — no code was re-resolved.

Verification on the rebased tree:

$ bun test server/tests/enterprise-controls.test.ts server/tests/otel-export-scrubbing.test.ts
 43 pass
 0 fail
 63 expect() calls
Ran 43 tests across 2 files. [536.00ms]

# bite check: same tests against main's scrub.ts
 35 pass
 8 fail

$ cd server && bunx tsc --noEmit          # clean, exit 0
$ bun run format:check                    # Checked 1316 files. No fixes applied.
$ bun run lint                            # Checked 1331 files. No fixes applied.

test:ci was not run: it needs a Postgres holding this repo's schema, configured through DATABASE_URL/.env. This machine has no .env for the repo and no openbot database, so the full suite cannot be run here.

PR is MERGEABLE again; mergeStateStatus is BLOCKED while checks run.

[paoValle]

Rebased onto main (6d86ba4). One conflict, in CHANGELOG.md — main added newer Unreleased sections above ours, so both are kept, ours below theirs. scrub.ts and the two test files applied cleanly; the diff is unchanged (+67/−9 across 4 files).

Verification on the rebased head 611bcf3:

• bun test server/tests/enterprise-controls.test.ts server/tests/otel-export-scrubbing.test.ts — 43 pass / 0 fail
• the same two files with main's scrub.ts swapped back in — 35 pass / 8 fail, so the tests still bite
• cd server && bunx tsc --noEmit — clean
• bun run format:check — 1375 files, no fixes applied; bun run lint — 1390 files, no fixes applied
• bun run test:ci not run: it needs a Postgres with this repo's schema

[phant0um]

I ran this PR's tests and a probe of 37 commands against main and the head. The new rules redact every shape they target, and the 9 non-credential commands I tried stay unchanged. The probe also found 7 shapes that leak, but they leak the same way on main, so they are not regressions from this PR.

Environment

• main 6d86ba4 (this PR's base) and head 611bcf3.
• Bun 1.4.3. bun install --frozen-lockfile --ignore-scripts.
• Each command was run twice, with the same output both times.

Tests

bun test tests/enterprise-controls.test.ts tests/otel-export-scrubbing.test.ts from server/:

Tree	Result
main with only this PR's test files	35 pass, 8 fail: 7 of the 9 new scrubbing commands cases (the other 2 are the bare mysql -p cases, which pass on main as they should) and a command exported to the collector is scrubbed of the secrets in it
Head	43 pass, 0 fail
Head with server/src/telemetry/scrub.ts reverted to main	35 pass, 8 fail, the same 8 tests

On the head, bun run typecheck in server/ exits 0. bunx biome check on the three touched files reports Checked 3 files in 11ms. No fixes applied.

Probe

The probe calls scrubCommand on each command and checks if hunter is still in the output.

Redacted on the head and leaked on main, besides the cases in the test file:

• docker login registry.example.com -u bob -p hunter2
• sudo docker login -u bob -p hunter2
• docker login -u bob -p 'hunter 2'
• sshpass -phunter2 ssh user@host
• sshpass -p hunter2 ssh -p 2222 user@host (the ssh -p 2222 stays)
• redis-cli -h cache -p 6379 -a hunter2 PING (the -p 6379 stays)
• redis-cli --pass=hunter2 PING
• docker login -u bob -p hunter2 && docker push img
• echo ok; redis-cli -a hunter2 PING

One case changed for the better as a side effect. mysql -u root -p'hunter 2' mydb gives -p[REDACTED] 2' mydb on main, which keeps part of the password. On the head it gives -p[REDACTED] mydb.

Unchanged on the head, as expected: docker run -p 8080:80 nginx, docker login -u bob --password-stdin, ssh -p 2222 user@host, redis-cli -p 6379 PING, redis-cli --askpass PING, mysql -h db -p, mysql -u root -p mydb, curl https://example.com:8443/path, git clone https://github.com/org/repo.git.

Still leaked on the head, and also on main:

• docker -H tcp://10.0.0.1:2375 login -u bob -p hunter2 and docker --context prod login -u bob -p hunter2. The rule needs login directly after docker, so a global option between them stops the match.
• mysqladmin, mysqlimport and mysqlcheck with -phunter2. The anchor \bmysql(?:dump)?\b does not match these names.
• mariadb -u root -phunter2 mydb and mariadb-dump -u root -phunter2 mydb, for the same reason.

These gaps are outside what this PR changes. I can open a separate issue for them if that helps.

Not tested

• Only the two test files above, not the full suite.
• I did not send anything to a real OpenTelemetry collector or check the Action Recording rows in a database.