game-ci / game-ci/cli

Redact live-streamed subprocess output in System.run()

Open
#247 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.