Step credentials can reach failure output and persisted step state unredacted
- Dominant language
- TypeScript
- Stars
- 145
- Forks
- 9
- Avg merge
- 20m
- Merged PRs (30d)
- 1
Description
Two paths let a step's credentials leave the engine without going through redaction.
### 1. The checkout credential is outside the redaction set
`runCiStep` builds the set from the step's command environment:
```ts
sensitiveValues: Object.values(prepared.extraEnv), // src/ci/capabilities.ts:80
```
The checkout carries its own credential and never enters `extraEnv`. With the Cloudflare Artifacts provider the two are not even the same string: `getRepoAccess` mints a token per call, so `getSourceCheckout` (src/artifacts/source-control.ts:74) and `getStepCredentialEnv` (:131) hand back different tokens, and only the second one is ever redacted.
The reachable case is an archive checkout, because `checkoutSourceScript` interpolates the URL straight into the command (src/shared/source-checkout.ts:25). A presigned URL is therefore in the text of the command that failed, and `sanitizeError`/`redactPreview` have nothing to match it against.
For a git checkout the token travels in `SOURCE_CONTROL_TOKEN` and stays out of the script text, so I could not find a path today that puts that one in the clear. It is unprotected rather than currently leaking.
### 2. Logs a step returns are not redacted, and Workflows persists them
`withStepNotification` redacts the notification preview and sanitizes the thrown error, then returns `completed.logs` untouched (src/ci/step-notification.ts:77). That return value is what Workflows stores as the step's state, which lives for 3 days on the Free plan and 30 on Paid. A step that opts into `secrets:` and runs a command echoing its environment leaves that secret at rest well after the run.
The README documents these logs as raw, so this is a stated choice rather than an oversight. The persistence is what makes it worth reconsidering, and the string case is a two-line fix at a point where the engine already holds the values. A streamed log cannot be rewritten without buffering it, so that branch has to stay as it is either way.
I have both fixed locally with regression tests and will open a PR.
Contributor guide
Research direction
Start with runCiStep in src/ci/capabilities.ts, then trace checkoutSourceScript in src/shared/source-checkout.ts and withStepNotification in src/ci/step-notification.ts. Review the regression tests mentioned in the issue and confirm both archive-checkout credentials and persisted string logs are covered without changing streamed-log behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, typescript
- Domain
- backend, ci-cd, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 30/100