cloudflare / cloudflare/ci

Step credentials can reach failure output and persisted step state unredacted

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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.