Redact live-streamed subprocess output in System.run()
- Dominant language
- TypeScript
- Stars
- 16
- Forks
- 6
- Avg merge
- 1h 51m
- Merged PRs (30d)
- 167
Description
### Context
game-ci/cli#246 added the first secret redaction in `src/`. Before it, nothing anywhere redacted anything.
That PR covers the paths where secrets are *formatted for logging*:
- the logger's `formatArgs` (so whole-object logs like `cli.ts`'s `parsed:` dump and `loadConfig`'s `cliOptions` are scrubbed)
- the `docker run` command string logged at `-vv`
- `System.run`'s captured-output debug dump, redacted before truncation
It deliberately does **not** cover live-streamed subprocess output.
### The gap
`System.run()` defaults to `silent: false`, so `Docker.run()` writes the child's stdout/stderr straight through to CI logs as it arrives. Those chunks are never redacted.
This is not a regression — that output was equally unredacted before #246 — but it is the one remaining place a registered secret can reach a log.
### Why it wasn't fixed in #246
Doing it correctly is more than wrapping each chunk in `SecretRedaction.redact()`:
1. **Chunk boundaries.** A secret can be split across two `data` events, so a naive per-chunk redact misses it. It needs a retained tail of at least `maxSecretLength - 1` bytes carried between chunks.
2. **Throughput.** This stream carries whole Unity build logs (multi-MB). A regex pass per registered secret per chunk is a real cost on the hot path, unlike the `-vv`-gated paths #246 touched.
3. **Fidelity.** `runResult.output` should stay unredacted so callers such as `UnityBuildValidation.validateBuild` keep matching on the true text. Only what is written out should change.
### Suggested approach
- A small streaming redactor holding a carry-over tail sized from the longest registered secret.
- Apply it only on the write-out path, leaving `runResult.output` untouched.
- Skip the work entirely when no secrets are registered, which is the common case and already how `SecretRedaction.redact` behaves.
- Test: a secret deliberately split across two chunk boundaries.
### References
- Review thread: https://github.com/game-ci/cli/pull/246#discussion_r3939465647
- Relevant code: `src/model/system/system.ts`, `src/model/secret-redaction.ts`
Contributor guide
Research direction
Start with src/model/system/system.ts to trace the live stdout/stderr write path, then read src/model/secret-redaction.ts and the review thread for existing redaction behavior. Test a secret split across two stream chunks and verify the streamed output is redacted while runResult.output remains unchanged; also cover the no-secrets path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- cli, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100