Repository navigation
security(publish): move publish-state file out of repo cwd #797
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import { describe, test, expect, beforeEach, afterEach } from 'vitest'; | ||
| import { homedir } from 'os'; | ||
| import { join } from 'path'; | ||
|
|
||
| import { | ||
| getCraftStateDir, | ||
| getPublishStateFilename, | ||
| getPublishStatePath, | ||
| } from '../publishState'; | ||
|
|
||
| describe('publishState', () => { | ||
| const savedEnv = { ...process.env }; | ||
|
|
||
| beforeEach(() => { | ||
| delete process.env.XDG_STATE_HOME; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| process.env = { ...savedEnv }; | ||
| }); | ||
|
|
||
| describe('getCraftStateDir', () => { | ||
| test('defaults to $HOME/.local/state/craft when XDG_STATE_HOME is unset', () => { | ||
| expect(getCraftStateDir()).toBe( | ||
| join(homedir(), '.local', 'state', 'craft'), | ||
| ); | ||
| }); | ||
|
|
||
| test('honours XDG_STATE_HOME when set', () => { | ||
| process.env.XDG_STATE_HOME = '/var/lib/ci-state'; | ||
| expect(getCraftStateDir()).toBe('/var/lib/ci-state/craft'); | ||
| }); | ||
|
|
||
| test('falls back to HOME when XDG_STATE_HOME is empty string', () => { | ||
| process.env.XDG_STATE_HOME = ''; | ||
| expect(getCraftStateDir()).toBe( | ||
| join(homedir(), '.local', 'state', 'craft'), | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getPublishStateFilename', () => { | ||
| const cwd = '/workspace/repo'; | ||
|
|
||
| test('includes owner, repo, short cwd hash, and version', () => { | ||
| const name = getPublishStateFilename( | ||
| '1.2.3', | ||
| { owner: 'getsentry', repo: 'craft' }, | ||
| cwd, | ||
| ); | ||
| expect(name).toMatch( | ||
| /^publish-state-getsentry-craft-[0-9a-f]{12}-1\.2\.3\.json$/, | ||
| ); | ||
| }); | ||
|
|
||
| test('disambiguates monorepo subpaths via cwd hash', () => { | ||
| const a = getPublishStateFilename( | ||
| '1.2.3', | ||
| { owner: 'o', repo: 'r' }, | ||
| '/workspace/repo/packages/foo', | ||
| ); | ||
| const b = getPublishStateFilename( | ||
| '1.2.3', | ||
| { owner: 'o', repo: 'r' }, | ||
| '/workspace/repo/packages/bar', | ||
| ); | ||
| expect(a).not.toBe(b); | ||
| }); | ||
|
|
||
| test('sanitises owner/repo/version characters', () => { | ||
| const name = getPublishStateFilename( | ||
| '1.2.3+build/hack$', | ||
| { owner: 'Weird Owner', repo: 'Re po!' }, | ||
| cwd, | ||
| ); | ||
| // No slashes, no plus, no dollar — all collapsed to underscores. | ||
| expect(name).not.toMatch(/[/$+!]/); | ||
| expect(name).toMatch(/^publish-state-weird_owner-re_po-[0-9a-f]{12}-/); | ||
| }); | ||
|
|
||
| test('falls back to sha256(cwd)-only filename when github config is null', () => { | ||
| const name = getPublishStateFilename('1.2.3', null, cwd); | ||
| expect(name).toMatch(/^publish-state-[0-9a-f]{16}-1\.2\.3\.json$/); | ||
| }); | ||
|
|
||
| test('fallback filename also disambiguates by cwd', () => { | ||
| const a = getPublishStateFilename('1.0.0', null, '/a'); | ||
| const b = getPublishStateFilename('1.0.0', null, '/b'); | ||
| expect(a).not.toBe(b); | ||
| }); | ||
|
|
||
| test('same inputs produce stable filenames', () => { | ||
| const a = getPublishStateFilename( | ||
| '1.2.3', | ||
| { owner: 'o', repo: 'r' }, | ||
| cwd, | ||
| ); | ||
| const b = getPublishStateFilename( | ||
| '1.2.3', | ||
| { owner: 'o', repo: 'r' }, | ||
| cwd, | ||
| ); | ||
| expect(a).toBe(b); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getPublishStatePath', () => { | ||
| test('joins state dir + filename', () => { | ||
| process.env.XDG_STATE_HOME = '/var/state'; | ||
| const path = getPublishStatePath( | ||
| '1.2.3', | ||
| { owner: 'o', repo: 'r' }, | ||
| '/workspace/repo', | ||
| ); | ||
| expect(path.startsWith('/var/state/craft/')).toBe(true); | ||
| expect(path.endsWith('-1.2.3.json')).toBe(true); | ||
| }); | ||
|
|
||
| test('does not place the state file inside cwd', () => { | ||
| // This is the crucial security property: the file must NEVER | ||
| // live inside the repository being published, because repo | ||
| // contents are attacker-influenceable via PRs. | ||
| const cwd = '/workspace/untrusted-repo'; | ||
| process.env.XDG_STATE_HOME = '/var/state'; | ||
| const path = getPublishStatePath('1.2.3', { owner: 'o', repo: 'r' }, cwd); | ||
| expect(path.startsWith(cwd)).toBe(false); | ||
| }); | ||
|
|
||
| test('does not place fallback state file inside cwd', () => { | ||
| // Same property must hold when github config is unavailable. | ||
| const cwd = '/workspace/untrusted-repo'; | ||
| process.env.XDG_STATE_HOME = '/var/state'; | ||
| const path = getPublishStatePath('1.2.3', null, cwd); | ||
| expect(path.startsWith(cwd)).toBe(false); | ||
| }); | ||
| }); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| /** | ||
| * Helpers for locating Craft's publish-state file in a path that is NOT | ||
| * writable by the repository being published. | ||
| * | ||
| * Background: `craft publish` writes a small JSON file listing targets | ||
| * that have completed so a resumed run can skip them. Before this module | ||
| * existed, the file lived at `.craft-publish-<version>.json` in the | ||
| * project's cwd. That path is inside the repository checkout, so any | ||
| * committed file at the same path (or any earlier CI step) could | ||
| * pre-populate the "published" set and trick Craft into silently | ||
| * skipping targets. | ||
| * | ||
| * The file now lives under `$XDG_STATE_HOME/craft/` (falling back to | ||
| * `$HOME/.local/state/craft/`). The filename is keyed on | ||
| * owner, repo, a hash of cwd (to disambiguate monorepo subpaths), and | ||
| * the version being published. `getsentry/publish` runs inside a Docker | ||
| * image with `HOME=/root`, so the XDG state dir is a clean, | ||
| * workflow-writable location that committed repo contents cannot reach. | ||
| */ | ||
|
|
||
| import { createHash } from 'crypto'; | ||
| import { homedir } from 'os'; | ||
| import { join } from 'path'; | ||
|
|
||
| import type { GitHubGlobalConfig } from '../schemas/project_config'; | ||
|
|
||
| const STATE_DIR_NAME = 'craft'; | ||
|
|
||
| /** | ||
| * Resolves `$XDG_STATE_HOME/craft/` with the standard fallback to | ||
| * `$HOME/.local/state/craft/` when `XDG_STATE_HOME` is unset. | ||
| * | ||
| * Exported for tests and for the publish workflow helper (see | ||
| * `scripts/print-publish-state-path.ts` if present) that needs to | ||
| * compute the same path. | ||
| */ | ||
| export function getCraftStateDir(): string { | ||
| const xdgStateHome = process.env.XDG_STATE_HOME; | ||
| if (xdgStateHome && xdgStateHome.length > 0) { | ||
| return join(xdgStateHome, STATE_DIR_NAME); | ||
| } | ||
| return join(homedir(), '.local', 'state', STATE_DIR_NAME); | ||
| } | ||
|
|
||
| /** | ||
| * Sanitises a string for inclusion in a filename: lowercases, replaces | ||
| * any character outside `[a-z0-9._-]` with `_`, and collapses runs. | ||
| * Owner/repo names are restricted by GitHub to `[A-Za-z0-9._-]` so this | ||
| * is mostly belt-and-braces. | ||
| */ | ||
| function sanitiseForFilename(raw: string): string { | ||
| return raw | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9._-]+/g, '_') | ||
| .replace(/^_+|_+$/g, ''); | ||
| } | ||
|
|
||
| /** | ||
| * Short (12-char) hex digest of the absolute cwd path. Used to | ||
| * disambiguate monorepo subpaths so `packages/foo` and `packages/bar` | ||
| * get separate state files even at the same version. | ||
| */ | ||
| function shortCwdHash(cwd: string): string { | ||
| return createHash('sha1').update(cwd).digest('hex').slice(0, 12); | ||
| } | ||
|
|
||
| /** | ||
| * Builds the filename for the publish-state file. | ||
| * | ||
| * With a resolvable GitHub config: | ||
| * `publish-state-<owner>-<repo>-<sha1(cwd)[:12]>-<version>.json` | ||
| * | ||
| * Without GitHub config (offline / non-GitHub test harnesses) the | ||
| * filename falls back to a cwd-hash-only form so Craft still refuses | ||
| * to write into the repo itself: | ||
| * `publish-state-<sha256(cwd)[:16]>-<version>.json` | ||
| */ | ||
| export function getPublishStateFilename( | ||
| version: string, | ||
| githubConfig: GitHubGlobalConfig | null, | ||
| cwd: string = process.cwd(), | ||
| ): string { | ||
| const safeVersion = sanitiseForFilename(version); | ||
| if (githubConfig) { | ||
| const owner = sanitiseForFilename(githubConfig.owner); | ||
| const repo = sanitiseForFilename(githubConfig.repo); | ||
| return `publish-state-${owner}-${repo}-${shortCwdHash(cwd)}-${safeVersion}.json`; | ||
| } | ||
| const cwdDigest = createHash('sha256').update(cwd).digest('hex').slice(0, 16); | ||
| return `publish-state-${cwdDigest}-${safeVersion}.json`; | ||
| } | ||
|
|
||
| /** | ||
| * Full absolute path to the publish-state file for the given version. | ||
| * | ||
| * @param version The version being published. | ||
| * @param githubConfig Resolved GitHub owner/repo (may be null when | ||
| * Craft is running outside a GitHub context). | ||
| * @param cwd Override cwd; defaults to `process.cwd()`. Used by tests. | ||
| */ | ||
| export function getPublishStatePath( | ||
| version: string, | ||
| githubConfig: GitHubGlobalConfig | null, | ||
| cwd: string = process.cwd(), | ||
| ): string { | ||
| return join( | ||
| getCraftStateDir(), | ||
| getPublishStateFilename(version, githubConfig, cwd), | ||
| ); | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.