security: dual-write publish state file to safe location#7886
Merged
Conversation
Craft's .craft-publish-<version>.json state file previously lived at __repo__/<path>/ (the project's cwd inside the container). That path is inside the repository being published, so committed repo contents could pre-populate the "already published" set and trick Craft into silently skipping targets. Write the state file to BOTH: - the legacy cwd location (for currently-deployed getsentry/craft), AND - the new $XDG_STATE_HOME/craft/ location (for Craft builds with getsentry/craft#795 applied, which move the state file out of the repo). Pin XDG_STATE_HOME to /github/workspace/.craft-state inside the container. That path is mounted from the runner, is outside __repo__/, and repo contents cannot reach it. The new filename matches Craft's scheme: publish-state-<owner>-<repo>-<sha1(cwd)[:12]>-<version>.json After Craft ships the new-location support and we've verified a few publish runs succeed, remove the legacy-location write block.
BYK
added a commit
to getsentry/craft
that referenced
this pull request
Apr 21, 2026
## Summary Moves `.craft-publish-<version>.json` out of the repo checkout into `$XDG_STATE_HOME/craft/`. Today the state file lives inside `cwd` (the publish target's checkout); a committed file at `.craft-publish-<X.Y.Z>.json` — or any earlier CI step writing to `cwd` — can pre-populate the "already published" set and trick Craft into silently skipping targets. That's a pipeline-manipulation primitive. **Ships with a companion PR on `getsentry/publish`: getsentry/publish#7886 (dual-writes to both the legacy and new location for the transition). ## Changes ### `src/utils/publishState.ts` (new) - `getCraftStateDir()` → `$XDG_STATE_HOME/craft/` or `$HOME/.local/state/craft/`. - `getPublishStateFilename(version, githubConfig, cwd)` → `publish-state-<owner>-<repo>-<sha1(cwd)[:12]>-<version>.json`. - The sha1(cwd) fragment disambiguates monorepo subpaths (e.g. `packages/foo` vs `packages/bar` at the same version). - When `githubConfig` is null (offline / non-GitHub), falls back to `publish-state-<sha256(cwd)[:16]>-<version>.json`. Still outside cwd. - `getPublishStatePath(...)` joins the two. ### `src/commands/publish.ts` - Resolve the GitHub config up front (already cached) and pass it to `getPublishStatePath`. - `mkdirSync(dirname(publishStateFile), { recursive: true })` before the first write. - Warn (but don't read) when a legacy `.craft-publish-<version>.json` is detected in cwd, so users running outside `getsentry/publish` know to migrate. ### Tests - `src/utils/__tests__/publishState.test.ts` — 12 tests: XDG handling, filename schema, monorepo disambiguation, sanitisation, security invariant that the state file is never inside `cwd`. - Existing `publish.test.ts` tests unchanged and still pass. ## Rollout plan This is one half of a two-PR coordinated change. Merge order: 1. Merge the `getsentry/publish` PR first. It dual-writes to both the legacy and new locations. Current Craft keeps reading the legacy location → works. 2. Merge this PR and ship a new Craft release (+ bump the `getsentry/craft:latest` Docker tag). Craft now reads only the new location. `publish.yml` is still writing both, so the new run finds the new file. Works. 3. After a few publish runs succeed on the new Craft, merge a follow-up on `getsentry/publish` that drops the legacy-location write. Between steps 1–2 and 2–3, nothing regresses. ## Verification - `pnpm test` — 12 new tests pass; existing publish.test.ts (18 tests) untouched. - Manual cross-check: the bash filename computation in the publish workflow produces the **same string** as `getPublishStateFilename()` for both root and monorepo paths: ``` container_cwd: /github/workspace/__repo__/packages/browser Craft: publish-state-getsentry-sentry-javascript-dbd7df897997-1.2.3.json Bash: publish-state-getsentry-sentry-javascript-dbd7df897997-1.2.3.json ``` ## Security property under test `getPublishStatePath(..., cwd)` must never return a path that is a descendant of `cwd`. Both `publishState.test.ts` tests assert this invariant for both the GitHub-config branch and the fallback branch.
BYK
added a commit
that referenced
this pull request
Apr 21, 2026
## Summary Drops the legacy-location write that #7886 added as a rollout aid for [getsentry/craft#797](getsentry/craft#797). Craft 2.26.0 ships the new-location read/write and is now live on `getsentry/craft:latest`, so the dual-write is no longer needed. ## Context - **#7886** (merged): started dual-writing the publish-state file to BOTH the legacy cwd location AND the safe `$XDG_STATE_HOME/craft/` location, so both old and new Craft images could find it during the rollout. - **getsentry/craft#797** (merged, shipped in 2.26.0 at 2026-04-21T20:10Z): Craft reads/writes only the safe location. - **This PR**: drop the legacy write. The new-write logic is unchanged from what #7886 already exercises in production. ## Verification - Pulled the released `craft 2.26.0` binary, seeded a state file at the new location only, ran `craft publish 9.9.9` against a synthetic repo, and confirmed Craft logged `Found publish state file, resuming from there...` and honoured the `published` field to skip the target. End-to-end confirmation of the new-location read path with the 2.26.0 binary. - Bash filename generation and Craft's Node filename generation produce byte-identical strings — verified earlier (in #7886 description) and unchanged here. - YAML still parses (`python3 -c "import yaml; yaml.safe_load(...)"`). ## Diff 8 insertions, 17 deletions: the legacy block is removed, comment updated to name the actual Craft PR (#797) that shipped the reader, and the remaining variables lose their `new_` prefix since there's only one location now. ## Rollback If something regresses, revert this PR; #7886's dual-write code returns and both locations are written again. Old Craft images would still work off the legacy location.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Writes
.craft-publish-<version>.jsonto BOTH the legacy cwd location (for currently-deployed Craft images) and a new safe location at$XDG_STATE_HOME/craft/publish-state-<owner>-<repo>-<sha1(cwd)[:12]>-<version>.json. Ships alongside getsentry/craft#797, which moves Craft's read/write to the new location.Motivation
Craft's state file previously lived inside the project's cwd — which is inside the repository being published. Any committed file at that path (or any earlier CI step) could pre-populate the "already published" set and silently skip targets on publish, a pipeline-manipulation primitive. See the
security(publish): move publish-state file out of repo cwdPR on getsentry/craft.Changes
.github/workflows/publish.yml→Set targetsstep$GITHUB_WORKSPACE/.craft-state/craft/publish-state-<owner>-<repo>-<sha1(container_cwd)[:12]>-<version>.json. This is mounted into the Craft Docker container at/github/workspace/.craft-state/craft/— outside__repo__/, unreachable from repo contents.Publish using CraftstepXDG_STATE_HOME=/github/workspace/.craft-stateso Craft resolves the same new-location path.Filename determinism
The bash filename computation is designed to match
getPublishStateFilename()in Craft exactly. Verified locally:Both root-path (
.) and monorepo (./packages/foo) inputs produce the same filename in bash and Node.Rollout
:latestbump.Verification
python3 -c "import yaml; yaml.safe_load(...)")..and./packages/foopaths; matches Craft's output byte-for-byte.fromJSON(steps.inputs.outputs.result).Why not just move to the new location immediately?
Older Craft images (deployed via
docker://getsentry/craft:latesttoday) expect the file at the legacy cwd path. Dropping the legacy write first would break publishes until the new Craft release rolls out to:latest, and would revert to breakage if anyone ever pinned to an older Craft tag. Dual-writes are zero-cost (a single extraprintf) and make the transition safe.