diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4e6c11ecb..323e8ba8d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,6 +12,9 @@ on: force: description: Force a release even when there are release-blockers (optional) required: false + workspace: + description: Named Craft release workspace to prepare and publish + required: false # For external repos to call this workflow workflow_call: @@ -51,6 +54,10 @@ on: type: string required: false default: '.' + workspace: + description: Named Craft release workspace to prepare and publish + type: string + required: false craft_config_from_merge_target: description: Use the craft config from the merge target branch type: string @@ -122,6 +129,7 @@ jobs: with: version: ${{ github.event.inputs.version }} force: ${{ github.event.inputs.force }} + workspace: ${{ github.event.inputs.workspace }} # For external repos: use published action - name: Prepare release @@ -139,4 +147,5 @@ jobs: git_user_name: ${{ inputs.git_user_name }} git_user_email: ${{ inputs.git_user_email }} path: ${{ inputs.path }} + workspace: ${{ inputs.workspace }} craft_config_from_merge_target: ${{ inputs.craft_config_from_merge_target }} diff --git a/action.yml b/action.yml index 8fa93907e..d16050903 100644 --- a/action.yml +++ b/action.yml @@ -31,6 +31,9 @@ inputs: description: The path that Craft will run inside required: false default: '.' + workspace: + description: Named Craft release workspace to prepare and publish + required: false craft_config_from_merge_target: description: Use the craft config from the merge target branch required: false @@ -68,6 +71,26 @@ outputs: runs: using: 'composite' steps: + - name: Validate workspace + shell: bash + env: + LC_ALL: C + PATH_INPUT: ${{ inputs.path }} + WORKSPACE: ${{ inputs.workspace }} + run: | + if [[ -n "$WORKSPACE" && "$PATH_INPUT" != '.' ]]; then + echo "::error::The path and workspace inputs cannot be used together." + exit 1 + fi + if ! node -e 'const path = process.env.PATH_INPUT; const segment = /^[A-Za-z0-9_.-]+$/; process.exit(path === "." || (typeof path === "string" && path.split("/").every(name => segment.test(name) && ![".", "..", "__proto__"].includes(name) && !name.startsWith("-"))) ? 0 : 1)'; then + echo "::error::The path input must use ASCII path segments containing only letters, digits, periods, underscores, and hyphens." + exit 1 + fi + if [[ -n "$WORKSPACE" ]] && ! node -e 'const workspace = process.env.WORKSPACE; const segment = /^[A-Za-z0-9_.-]+$/; process.exit(workspace.split("/").every(name => segment.test(name) && ![".", "..", "__proto__"].includes(name) && !name.startsWith("-")) ? 0 : 1)'; then + echo "::error::Workspace names must use ASCII path segments containing only letters, digits, periods, underscores, and hyphens." + exit 1 + fi + - id: killswitch name: Check release blockers shell: bash @@ -162,8 +185,10 @@ runs: CRAFT_CONFIG_FROM_MERGE_TARGET: ${{ inputs.craft_config_from_merge_target }} MERGE_TARGET: ${{ inputs.merge_target }} VERSION: ${{ inputs.version }} + WORKSPACE: ${{ inputs.workspace }} working-directory: ${{ inputs.path }} run: | + unset CRAFT_WORKSPACE # Ensure we have origin/HEAD set git remote set-head origin --auto @@ -172,6 +197,9 @@ runs: if [[ "$CRAFT_CONFIG_FROM_MERGE_TARGET" == 'true' && -n "$MERGE_TARGET" ]]; then CRAFT_ARGS=(--config-from "$MERGE_TARGET") fi + if [[ -n "$WORKSPACE" ]]; then + CRAFT_ARGS+=("--workspace=$WORKSPACE") + fi # Version is optional - if not provided, Craft uses versioning.policy from config VERSION_ARG=() @@ -187,8 +215,14 @@ runs: working-directory: ${{ inputs.path }} env: CRAFT_LOG_LEVEL: Warn + WORKSPACE: ${{ inputs.workspace }} run: | - targets=$(craft targets | jq -r '.[]|" - [ ] \(.)"') + unset CRAFT_WORKSPACE + CRAFT_ARGS=() + if [[ -n "$WORKSPACE" ]]; then + CRAFT_ARGS=("--workspace=$WORKSPACE") + fi + targets=$(craft targets "${CRAFT_ARGS[@]}" | jq -r '.[]|" - [ ] \(.)"') # https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings echo "targets<> "$GITHUB_OUTPUT" @@ -211,6 +245,7 @@ runs: SUBDIRECTORY: ${{ inputs.path != '.' && format('/{0}', inputs.path) || '' }} MERGE_TARGET: ${{ inputs.merge_target || '(default)' }} PUBLISH_REPO: ${{ inputs.publish_repo || format('{0}/publish', github.repository_owner) }} + WORKSPACE: ${{ inputs.workspace }} run: | # Resolve "self" to the current repository if [[ "$PUBLISH_REPO" == "self" ]]; then @@ -221,7 +256,6 @@ runs: echo "::error::Craft did not output a version. This is unexpected." exit 1 fi - # Read changelog from file to avoid E2BIG. # Produced by craft >= 2.22.0; older versions don't produce the file # and the publish issue will be created without a changelog section. @@ -238,7 +272,7 @@ runs: CHANGELOG="${CHANGELOG:0:$MAX_CHANGELOG_CHARS}"$'\n\n---\n*Changelog truncated for issue body.*' fi - title="publish: ${GITHUB_REPOSITORY}${SUBDIRECTORY}@${RESOLVED_VERSION}" + title="publish: ${GITHUB_REPOSITORY}${SUBDIRECTORY}${WORKSPACE:+/$WORKSPACE}@${RESOLVED_VERSION}" # Check if issue already exists by listing all open issues and filtering by exact title match. # We avoid GitHub search API to bypass indexing delays and query syntax edge cases. diff --git a/docs/src/content/docs/targets/github.md b/docs/src/content/docs/targets/github.md index 9f522f2b8..5d983a36f 100644 --- a/docs/src/content/docs/targets/github.md +++ b/docs/src/content/docs/targets/github.md @@ -73,12 +73,46 @@ targets: Releasing `1.2.3` for each product then produces the tags `cli@1.2.3` / `mcp@1.2.3` on release branches `release/cli/1.2.3` / `release/mcp/1.2.3` — no collisions. -:::note[Coming soon: first-class workspaces] -A single, target-agnostic top-level `workspaces:` model (with an explicit `--workspace` selector) is planned so that all of a repo's products can be managed from one `.craft.yml`. It will supersede the per-file convention above. Track progress in [getsentry/craft#842](https://github.com/getsentry/craft/issues/842). -::: +## Release Workspaces + +Use top-level `workspaces:` to define independently versioned release units in +one repository. Keys may be literal paths or glob patterns, and Craft expands +patterns to concrete directories relative to `.craft.yml`. Select one concrete +path explicitly with `--workspace ` or +`CRAFT_WORKSPACE`: + +```yaml +minVersion: 2.29.0 +github: + owner: getsentry + repo: toolkit +workspaces: + packages/*: + releaseBranchPrefix: release/cli + targets: + - name: github + tagPrefix: "cli@" + tools/mcp: + releaseBranchPrefix: release/mcp + targets: + - name: github + tagPrefix: "mcp@" +``` + +Selected workspace paths use nonempty ASCII path segments containing letters, +digits, periods, underscores, and hyphens. Segments cannot be `.`, `..`, +`__proto__`, or start with `-`. They are release units, not npm package +workspaces. Craft ignores files matched by a configured glob and rejects +overlapping workspace keys. A workspace cannot use +`github.projectPath`, and a workflow cannot provide both a `path` and a +workspace. This keeps each publish request unambiguous. + +The `craft workspace list` command prints the concrete workspace paths as a +JSON array for automation. Action publish titles always carry the full concrete +path. :::caution -Declaring **multiple** `github` targets with **different** `tagPrefix` values in a *single* config is currently ambiguous: Craft uses the first prefix for read-path operations and logs a warning. Until workspaces land, use a separate `.craft.yml` per product. +Declaring **multiple** `github` targets with **different** `tagPrefix` values in a single release unit is ambiguous: Craft uses the first prefix for read-path operations and logs a warning. Use separate release workspaces for independent products. ::: :::note[Known limitation: the GitHub "Latest" badge is repo-wide] diff --git a/src/__tests__/action.test.ts b/src/__tests__/action.test.ts new file mode 100644 index 000000000..1605ee3c3 --- /dev/null +++ b/src/__tests__/action.test.ts @@ -0,0 +1,313 @@ +import { + chmodSync, + mkdtempSync, + mkdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { spawnSync } from 'child_process'; + +import { load } from 'js-yaml'; +import { afterEach, expect, test } from 'vitest'; + +interface ActionStep { + env?: Record; + name?: string; + run?: string; +} + +function getActionSteps(): ActionStep[] { + const action = load( + readFileSync(join(__dirname, '../../action.yml'), 'utf8'), + ) as { + runs?: { steps?: ActionStep[] }; + }; + return action.runs?.steps || []; +} + +function getActionStep(name: string): ActionStep { + const step = getActionSteps().find(step => step.name === name); + if (!step?.run) { + throw new Error(`Missing ${name} action step`); + } + return step; +} + +const tempDirectories: string[] = []; + +function createActionEnvironment() { + const directory = mkdtempSync(join(tmpdir(), 'craft-action-test-')); + tempDirectories.push(directory); + const binDirectory = join(directory, 'bin'); + const craftCalls = join(directory, 'craft-calls'); + const ghTitles = join(directory, 'gh-titles'); + const gitCalls = join(directory, 'git-calls'); + const output = join(directory, 'github-output'); + mkdirSync(binDirectory); + writeFileSync(craftCalls, ''); + writeFileSync(ghTitles, ''); + writeFileSync(gitCalls, ''); + writeFileSync(output, ''); + writeFileSync( + join(binDirectory, 'craft'), + '#!/usr/bin/env bash\nif [[ -n "${CRAFT_WORKSPACE:-}" ]]; then\n exit 1\nfi\nprintf "%s\\n" "$*" >> "$CRAFT_CALLS"\nif [[ "$1" == "targets" ]]; then\n printf \'["github"]\'\nfi\n', + ); + writeFileSync( + join(binDirectory, 'git'), + '#!/usr/bin/env bash\nprintf "%s\\n" "$*" >> "$GIT_CALLS"\n', + ); + writeFileSync( + join(binDirectory, 'gh'), + `#!/usr/bin/env bash +if [[ "$*" == *"issue list"* ]]; then + printf '[]' + exit 0 +fi +if [[ "$*" == *"issue create"* ]]; then + while [[ $# -gt 0 ]]; do + if [[ "$1" == '--title' ]]; then + printf '%s\\n' "$2" >> "$GH_TITLES" + break + fi + shift + done + printf 'https://github.com/getsentry/publish/issues/1\\n' +fi +`, + ); + chmodSync(join(binDirectory, 'craft'), 0o755); + chmodSync(join(binDirectory, 'gh'), 0o755); + chmodSync(join(binDirectory, 'git'), 0o755); + + return { binDirectory, craftCalls, directory, ghTitles, gitCalls, output }; +} + +function runRequestPublish( + workspace: string, + environment: ReturnType, +) { + return spawnSync( + 'bash', + ['-e', '-c', getActionStep('Request publish').run!], + { + cwd: environment.directory, + env: { + ...process.env, + CHANGELOG_FILE: '', + GITHUB_ACTOR: 'byk', + GITHUB_OUTPUT: environment.output, + GITHUB_REPOSITORY: 'getsentry/toolkit', + GH_TITLES: environment.ghTitles, + MERGE_TARGET: '(default)', + PATH: `${environment.binDirectory}:${process.env.PATH}`, + PUBLISH_REPO: 'getsentry/publish', + RELEASE_BRANCH: 'release/1.2.3', + RELEASE_PREVIOUS_TAG: '1.2.2', + RELEASE_SHA: 'abc123', + RESOLVED_VERSION: '1.2.3', + SUBDIRECTORY: '', + TARGETS: ' - [ ] github', + WORKSPACE: workspace, + }, + }, + ); +} + +function runActionStep( + stepName: string, + workspace: string, + environment: ReturnType, + pathInput = '.', + locale = 'C', +) { + return spawnSync('bash', ['-e', '-c', getActionStep(stepName).run!], { + cwd: environment.directory, + env: { + ...process.env, + CRAFT_CALLS: environment.craftCalls, + CRAFT_CONFIG_FROM_MERGE_TARGET: '', + GITHUB_OUTPUT: environment.output, + GIT_CALLS: environment.gitCalls, + LC_ALL: locale, + MERGE_TARGET: '', + PATH: `${environment.binDirectory}:${process.env.PATH}`, + PATH_INPUT: pathInput, + VERSION: '', + WORKSPACE: workspace, + }, + }); +} + +afterEach(() => { + for (const directory of tempDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +test('forwards workspace input to every Craft command', () => { + expect(getActionStep('Validate workspace').env?.PATH_INPUT).toBe( + '${{ inputs.path }}', + ); + expect(getActionStep('Craft Prepare').env?.WORKSPACE).toBe( + '${{ inputs.workspace }}', + ); + expect(getActionStep('Read Craft Targets').env?.WORKSPACE).toBe( + '${{ inputs.workspace }}', + ); +}); + +test.each([ + ['control', 'cli\tnext'], + ['format', 'cli\u202enext'], + ['line separator', 'cli\u2028next'], + ['paragraph separator', 'cli\u2029next'], + ['non-ASCII', 'cli-é'], +])( + 'rejects %s characters before every action side effect', + (_name, workspace) => { + const environment = createActionEnvironment(); + + expect(getActionSteps()[0]?.name).toBe('Validate workspace'); + expect( + runActionStep('Validate workspace', workspace, environment).status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); + }, +); + +test('rejects non-ASCII workspace input in a UTF-8 locale', () => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', 'cli-é', environment, '.', 'en_US.utf8') + .status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); +}); + +test.each(['', 'cli-v2', 'packages/cli', 'packages/CLI'])( + 'accepts safe workspace input %j', + workspace => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', workspace, environment).status, + ).toBe(0); + }, +); + +test.each([ + '../outside', + '/tmp', + './packages/cli', + 'packages//cli', + 'packages/../cli', +])( + 'rejects unsafe checkout path %j before every action side effect', + pathInput => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', '', environment, pathInput).status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); + }, +); + +test('rejects a path and workspace together before every action side effect', () => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', 'cli', environment, 'packages/cli') + .status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); +}); + +test.each(['cli\nnext', 'packages/*'])( + 'rejects workspace names outside the path workspace grammar', + workspace => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', workspace, environment).status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); + }, +); + +test.each(['.', '..', '__proto__', '-foo', '--config'])( + 'rejects unsafe workspace name %j', + workspace => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', workspace, environment).status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); + }, +); + +test.each([ + './packages/cli', + 'packages//cli', + 'packages/./cli', + 'packages/../cli', + 'packages/__proto__/cli', + 'packages/-cli', +])('rejects unsafe workspace path %j', workspace => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', workspace, environment).status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); +}); + +test('uses the full workspace path in publish request titles', () => { + const rootEnvironment = createActionEnvironment(); + const workspaceEnvironment = createActionEnvironment(); + + expect(runRequestPublish('', rootEnvironment).status).toBe(0); + expect(runRequestPublish('packages/cli', workspaceEnvironment).status).toBe( + 0, + ); + + expect(readFileSync(rootEnvironment.ghTitles, 'utf8')).toBe( + 'publish: getsentry/toolkit@1.2.3\n', + ); + expect(readFileSync(workspaceEnvironment.ghTitles, 'utf8')).toBe( + 'publish: getsentry/toolkit/packages/cli@1.2.3\n', + ); +}); + +test('clears an inherited workspace before root Craft commands', () => { + const environment = createActionEnvironment(); + const previousWorkspace = process.env.CRAFT_WORKSPACE; + process.env.CRAFT_WORKSPACE = 'packages/cli'; + + try { + expect(runActionStep('Craft Prepare', '', environment).status).toBe(0); + expect(runActionStep('Read Craft Targets', '', environment).status).toBe(0); + } finally { + if (previousWorkspace === undefined) { + delete process.env.CRAFT_WORKSPACE; + } else { + process.env.CRAFT_WORKSPACE = previousWorkspace; + } + } + + expect(readFileSync(environment.craftCalls, 'utf8')).toBe( + 'prepare\ntargets\n', + ); +}); diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index 90e145bfe..58c0f1ad6 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -1,4 +1,8 @@ -import { describe, test, expect, vi, afterEach } from 'vitest'; +import { describe, test, expect, vi, afterEach, beforeEach } from 'vitest'; +import { mkdtempSync, mkdirSync, rmSync, symlinkSync, writeFileSync } from 'fs'; +import { globSync } from 'glob'; +import { tmpdir } from 'os'; +import { join } from 'path'; /** * Tests of our ability to read craft config files. (This is NOT general test * configuration). @@ -10,7 +14,9 @@ import { validateConfiguration, setActiveWorkspace, getActiveWorkspace, + getConfiguration, getVersioningPolicy, + getWorkspaceNames, WORKSPACES_MIN_VERSION, } from '../config'; import { CraftProjectConfigSchema } from '../schemas/project_config'; @@ -134,7 +140,6 @@ describe('noMerge config', () => { workspaces: { cli: { releaseBranchPrefix: 'release/cli', - github: { projectPath: 'cli' }, targets: [{ name: 'github', tagPrefix: 'cli@' }], }, mcp: { @@ -146,16 +151,74 @@ describe('noMerge config', () => { expect(validateConfiguration(data)).toEqual(data); }); - test('allows a workspace github override without owner/repo', () => { + test('allows a workspace github owner/repo override', () => { const data = { workspaces: { - cli: { github: { projectPath: 'cli' } }, + cli: { github: { owner: 'getsentry', repo: 'toolkit' } }, }, }; - // Workspace github is partial; owner/repo are inherited, not required here. + // Workspace github is partial; owner/repo are not required together here. expect(() => validateConfiguration(data)).not.toThrow(); }); + + test('allows multi-segment workspace paths', () => { + expect(() => + validateConfiguration({ workspaces: { 'cli/v2': {} } }), + ).not.toThrow(); + }); + + test.each(['.', '..'])('rejects traversal workspace name %j', name => { + expect(() => validateConfiguration({ workspaces: { [name]: {} } })).toThrow( + 'Workspace names cannot be "." or "..".', + ); + }); + + test('rejects the __proto__ workspace key', () => { + expect(() => + loadConfigurationFromString( + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ' __proto__: {}', + ].join('\n'), + ), + ).toThrow('Workspace name "__proto__" is not supported.'); + }); + + test('rejects workspace github.projectPath', () => { + expect(() => + validateConfiguration({ + workspaces: { cli: { github: { projectPath: 'cli' } } }, + }), + ).toThrow('Workspace github.projectPath is not supported.'); + }); + + test('rejects a base github.projectPath when workspaces are configured', () => { + expect(() => + validateConfiguration({ + github: { + owner: 'getsentry', + repo: 'toolkit', + projectPath: 'packages/cli', + }, + workspaces: { cli: {} }, + }), + ).toThrow('Workspace configurations cannot use github.projectPath.'); + }); + + test('allows github.projectPath with an empty workspace map', () => { + expect(() => + validateConfiguration({ + github: { + owner: 'getsentry', + repo: 'toolkit', + projectPath: 'packages/cli', + }, + workspaces: {}, + }), + ).not.toThrow(); + }); }); describe('getGitTagPrefix', () => { @@ -226,7 +289,18 @@ describe('getGitTagPrefix', () => { }); describe('workspaces', () => { + let originalCwd: string; + const temporaryDirectories: string[] = []; + + beforeEach(() => { + originalCwd = process.cwd(); + }); + afterEach(() => { + process.chdir(originalCwd); + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } setActiveWorkspace(undefined); vi.restoreAllMocks(); }); @@ -240,8 +314,6 @@ describe('workspaces', () => { 'workspaces:', ' cli:', ' releaseBranchPrefix: release/cli', - ' github:', - ' projectPath: cli', ' targets:', ' - name: github', ' tagPrefix: "cli@"', @@ -269,12 +341,7 @@ describe('workspaces', () => { // Overridden by the workspace. expect(config.releaseBranchPrefix).toBe('release/cli'); expect(getGitTagPrefix()).toBe('cli@'); - // github is shallow-merged: owner/repo inherited, projectPath overridden. - expect(config.github).toEqual({ - owner: 'getsentry', - repo: 'toolkit', - projectPath: 'cli', - }); + expect(config.github).toEqual({ owner: 'getsentry', repo: 'toolkit' }); // Inherited from the top level. expect(config.changelog).toBe('CHANGELOG.md'); // `workspaces` is stripped from the resolved config. @@ -287,7 +354,6 @@ describe('workspaces', () => { expect(config.releaseBranchPrefix).toBe('release/mcp'); expect(getGitTagPrefix()).toBe('mcp@'); expect(getVersioningPolicy()).toBe('calver'); - // mcp did not override github.projectPath, so it inherits base github only. expect(config.github).toEqual({ owner: 'getsentry', repo: 'toolkit' }); }); @@ -298,6 +364,254 @@ describe('workspaces', () => { ); }); + test('lists concrete workspace names without requiring a selection', () => { + setActiveWorkspace(undefined); + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + const configPath = join(directory, '.craft.yml'); + writeFileSync(configPath, WS_CONFIG); + process.chdir(directory); + + expect(getWorkspaceNames()).toEqual(['cli', 'mcp']); + writeFileSync( + configPath, + ['minVersion: 2.14.0', 'workspaces:', ' cli: {}'].join('\n'), + ); + + expect(() => getWorkspaceNames()).toThrow( + `requires minVersion >= ${WORKSPACES_MIN_VERSION}`, + ); + }); + + test('expands a workspace glob into concrete directory paths', () => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + mkdirSync(join(directory, 'packages', 'cli'), { recursive: true }); + mkdirSync(join(directory, 'packages', 'mcp'), { recursive: true }); + writeFileSync(join(directory, 'packages', 'README.md'), 'not a workspace'); + writeFileSync( + join(directory, '.craft.yml'), + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ' packages/*:', + ' releaseBranchPrefix: release/package', + ].join('\n'), + ); + process.chdir(directory); + + expect(getWorkspaceNames()).toEqual(['packages/cli', 'packages/mcp']); + + setActiveWorkspace('packages/cli'); + expect(getConfiguration(true).releaseBranchPrefix).toBe('release/package'); + }); + + test('expands remote configuration globs from the repository root', () => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + mkdirSync(join(directory, 'packages', 'cli'), { recursive: true }); + const nestedDirectory = join(directory, 'nested'); + mkdirSync(nestedDirectory); + process.chdir(nestedDirectory); + + setActiveWorkspace('packages/cli'); + expect( + loadConfigurationFromString( + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ' packages/*:', + ' releaseBranchPrefix: release/package', + ].join('\n'), + directory, + ).releaseBranchPrefix, + ).toBe('release/package'); + }); + + test.each(['packages/[!a]*', 'packages/[^a]*'])( + 'expands negated character-class workspace glob %s', + workspaceGlob => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + mkdirSync(join(directory, 'packages', 'cli'), { recursive: true }); + mkdirSync(join(directory, 'packages', 'api'), { recursive: true }); + writeFileSync( + join(directory, '.craft.yml'), + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ` "${workspaceGlob}": {}`, + ].join('\n'), + ); + process.chdir(directory); + + expect(getWorkspaceNames()).toEqual(['packages/cli']); + }, + ); + + test.each([ + ['packages/{cli,mcp}', ['packages/cli', 'packages/mcp']], + [ + 'packages/{cli,{mcp,api}}', + ['packages/api', 'packages/cli', 'packages/mcp'], + ], + ['packages/?li', ['packages/cli']], + ['packages/[cm]*', ['packages/cli', 'packages/mcp']], + ['packages/**/cli', ['packages/cli', 'packages/nested/cli']], + ])('expands supported workspace glob %s', (workspaceGlob, expectedNames) => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + mkdirSync(join(directory, 'packages', 'cli'), { recursive: true }); + mkdirSync(join(directory, 'packages', 'mcp'), { recursive: true }); + mkdirSync(join(directory, 'packages', 'api'), { recursive: true }); + mkdirSync(join(directory, 'packages', 'nested', 'cli'), { + recursive: true, + }); + writeFileSync( + join(directory, '.craft.yml'), + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ` "${workspaceGlob}": {}`, + ].join('\n'), + ); + process.chdir(directory); + + expect(getWorkspaceNames()).toEqual(expectedNames); + }); + + test('does not expand workspace globs through symlinked directories', () => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + const outsideDirectory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory, outsideDirectory); + mkdirSync(join(directory, 'packages', 'internal', 'release'), { + recursive: true, + }); + mkdirSync(join(outsideDirectory, 'release'), { recursive: true }); + symlinkSync(outsideDirectory, join(directory, 'packages', 'external')); + writeFileSync( + join(directory, '.craft.yml'), + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ' packages/**/release: {}', + ].join('\n'), + ); + process.chdir(directory); + + expect(getWorkspaceNames()).toEqual(['packages/internal/release']); + }); + + test('ignores broken symlinks matched by workspace globs', () => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + mkdirSync(join(directory, 'packages', 'cli'), { recursive: true }); + symlinkSync( + join(directory, 'missing-workspace'), + join(directory, 'packages', 'broken'), + ); + writeFileSync( + join(directory, '.craft.yml'), + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ' packages/*: {}', + ].join('\n'), + ); + process.chdir(directory); + + expect( + globSync('packages/*', { cwd: directory, dot: true, posix: true }), + ).toContain('packages/broken'); + expect(getWorkspaceNames()).toEqual(['packages/cli']); + }); + + test('fails on unexpected workspace glob resolution errors', () => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + mkdirSync(join(directory, 'packages', 'cli'), { recursive: true }); + symlinkSync('loop', join(directory, 'packages', 'loop')); + writeFileSync( + join(directory, '.craft.yml'), + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ' packages/*: {}', + ].join('\n'), + ); + process.chdir(directory); + + expect(() => getWorkspaceNames()).toThrow(/ELOOP/); + }); + + test.each(['{../outside/*,packages/*}', '{/tmp/*,packages/*}'])( + 'rejects a glob with an unsafe alternative: %s', + workspaceGlob => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + process.chdir(directory); + + expect(() => + loadConfigurationFromString( + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ` "${workspaceGlob}": {}`, + ].join('\n'), + ), + ).toThrow('Workspace paths must use safe ASCII segments.'); + }, + ); + + test.each([ + 'packages/{cli', + 'packages/{cli}', + 'packages/{cli,{mcp}}', + 'packages/{cli,{{},mcp}}', + ])('rejects a malformed brace glob: %s', workspaceGlob => { + expect(() => + validateConfiguration({ workspaces: { [workspaceGlob]: {} } }), + ).toThrow('Workspace paths must use safe ASCII segments.'); + }); + + test.each([ + 'packages/./cli', + 'packages/../cli', + 'packages/__proto__/cli', + 'packages/foo]', + 'packages/foo!', + 'packages/foo^', + ])('rejects an unsafe literal workspace path: %s', workspaceName => { + expect(() => + validateConfiguration({ workspaces: { [workspaceName]: {} } }), + ).toThrow('Workspace paths must use safe ASCII segments.'); + }); + + test('rejects concrete workspace paths that match multiple globs', () => { + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + mkdirSync(join(directory, 'packages', 'cli'), { recursive: true }); + writeFileSync( + join(directory, '.craft.yml'), + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ' packages/*: {}', + ' packages/cli*: {}', + ].join('\n'), + ); + process.chdir(directory); + + expect(() => getWorkspaceNames()).toThrow( + /matches multiple workspace patterns: packages\/\*, packages\/cli\*/, + ); + + setActiveWorkspace('packages/cli'); + expect(() => getConfiguration(true)).toThrow( + /matches multiple workspace patterns: packages\/\*, packages\/cli\*/, + ); + }); + test('errors on an unknown workspace name', () => { setActiveWorkspace('nope'); expect(() => loadConfigurationFromString(WS_CONFIG)).toThrow( @@ -366,28 +680,7 @@ describe('workspaces', () => { expect(config.targets).toEqual([{ name: 'github', tagPrefix: 'cli@' }]); }); - test('does not produce an incomplete github when base has none', () => { - // A workspace that sets only github.projectPath, with NO top-level github, - // must NOT yield a truthy-but-incomplete github object (missing owner/repo) - // — that would make getGlobalGitHubConfig skip its git-remote fallback. - setActiveWorkspace('cli'); - const config = loadConfigurationFromString( - [ - `minVersion: ${WORKSPACES_MIN_VERSION}`, - 'workspaces:', - ' cli:', - ' github:', - ' projectPath: cli', - ' targets:', - ' - name: github', - ' tagPrefix: "cli@"', - ].join('\n'), - ); - // Incomplete github is dropped so git-remote detection can still run. - expect(config.github).toBeUndefined(); - }); - - test('keeps github when workspace override completes owner/repo', () => { + test('shallow-merges a workspace github owner/repo override', () => { setActiveWorkspace('cli'); const config = loadConfigurationFromString( [ @@ -397,7 +690,6 @@ describe('workspaces', () => { ' github:', ' owner: getsentry', ' repo: toolkit', - ' projectPath: cli', ' targets:', ' - name: github', ].join('\n'), @@ -405,7 +697,6 @@ describe('workspaces', () => { expect(config.github).toEqual({ owner: 'getsentry', repo: 'toolkit', - projectPath: 'cli', }); }); }); diff --git a/src/commands/__tests__/publish.test.ts b/src/commands/__tests__/publish.test.ts index f2bd73497..3e866f72d 100644 --- a/src/commands/__tests__/publish.test.ts +++ b/src/commands/__tests__/publish.test.ts @@ -2,11 +2,14 @@ import { vi, describe, test, expect, beforeEach, type Mock } from 'vitest'; import { join as pathJoin } from 'path'; import { spawnProcess, hasExecutable } from '../../utils/system'; import { + getPublishStateGitHubConfig, + getRevisionBranchName, runPostReleaseCommand, handleReleaseBranch, MergeConflictError, PushError, } from '../publish'; +import { getPublishStateFilename } from '../../utils/publishState'; import type { SimpleGit } from 'simple-git'; vi.mock('../../utils/system'); @@ -162,6 +165,77 @@ describe('runPostReleaseCommand', () => { }); }); +describe('getPublishStateGitHubConfig', () => { + test('uses the controller checkout repository only for state identity', () => { + const resolvedWorkspaceGithub = { + owner: 'release-owner', + repo: 'release-repo', + projectPath: 'packages/cli', + }; + const stateGithub = getPublishStateGitHubConfig( + resolvedWorkspaceGithub, + 'getsentry/toolkit', + ); + + expect(stateGithub).toEqual({ owner: 'getsentry', repo: 'toolkit' }); + expect( + getPublishStateFilename( + '1.2.3', + stateGithub, + '/github/workspace/__repo__/packages/cli', + 'cli', + ), + ).toBe( + getPublishStateFilename( + '1.2.3', + { owner: 'getsentry', repo: 'toolkit' }, + '/github/workspace/__repo__/packages/cli', + 'cli', + ), + ); + }); + + test('keeps the resolved GitHub configuration without controller state identity', () => { + const githubConfig = { owner: 'release-owner', repo: 'release-repo' }; + + expect(getPublishStateGitHubConfig(githubConfig, undefined)).toBe( + githubConfig, + ); + }); + + test('rejects malformed controller state repository values', () => { + expect(() => + getPublishStateGitHubConfig(null, 'getsentry/toolkit/extra'), + ).toThrow('CRAFT_PUBLISH_STATE_GITHUB_REPO'); + }); +}); + +describe('getRevisionBranchName', () => { + test('returns the named ref for a revision when available', async () => { + const git = { + raw: vi.fn().mockResolvedValue('release/1.2.3\n'), + } as unknown as SimpleGit; + + await expect(getRevisionBranchName(git, 'abc123')).resolves.toBe( + 'release/1.2.3', + ); + expect(git.raw).toHaveBeenCalledWith( + 'name-rev', + '--name-only', + '--no-undefined', + 'abc123', + ); + }); + + test('allows a detached CI-approved revision', async () => { + const git = { + raw: vi.fn().mockRejectedValue(new Error('Could not get ref name')), + } as unknown as SimpleGit; + + await expect(getRevisionBranchName(git, 'abc123')).resolves.toBe(''); + }); +}); + describe('handleReleaseBranch', () => { /** * Creates a mock SimpleGit instance where each method returns diff --git a/src/commands/prepare.ts b/src/commands/prepare.ts index fc97b201b..37410a27c 100644 --- a/src/commands/prepare.ts +++ b/src/commands/prepare.ts @@ -807,7 +807,8 @@ export async function prepareMain(argv: PrepareOptions): Promise { const configContent = await git.show([ `${argv.remote}/${argv.configFrom}:${CONFIG_FILE_NAME}`, ]); - loadConfigurationFromString(configContent); + const repositoryRoot = await git.revparse(['--show-toplevel']); + loadConfigurationFromString(configContent, repositoryRoot.trim()); } catch (error: any) { throw new ConfigurationError( `Failed to load ${CONFIG_FILE_NAME} from branch "${argv.configFrom}": ${error.message}`, diff --git a/src/commands/publish.ts b/src/commands/publish.ts index 30bb83484..dad43f31a 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -18,7 +18,10 @@ import { getActiveWorkspace, } from '../config'; import { formatTable, logger } from '../logger'; -import { TargetConfig } from '../schemas/project_config'; +import { + type GitHubGlobalConfig, + TargetConfig, +} from '../schemas/project_config'; import { getAllTargetNames, getTargetByName, SpecialTarget } from '../targets'; import { BaseTarget } from '../targets/base'; import { @@ -166,6 +169,32 @@ export interface PublishState { }; } +/** + * The Publish controller prepopulates a secure state file using the issue's + * checkout repository. That can differ from a workspace's release GitHub + * configuration, so this override is deliberately limited to state identity. + */ +export function getPublishStateGitHubConfig( + githubConfig: GitHubGlobalConfig | null, + stateRepository: string | undefined = process.env + .CRAFT_PUBLISH_STATE_GITHUB_REPO, +): GitHubGlobalConfig | null { + if (!stateRepository) { + return githubConfig; + } + + const match = stateRepository.match( + /^(?[A-Za-z0-9_.-]+)\/(?[A-Za-z0-9_.-]+)$/, + ); + if (!match?.groups) { + throw new ConfigurationError( + 'CRAFT_PUBLISH_STATE_GITHUB_REPO must be a GitHub owner/repository pair.', + ); + } + + return { owner: match.groups.owner, repo: match.groups.repo }; +} + /** * Checks that the passed version is a valid version string * @@ -602,9 +631,7 @@ export async function publishMain(argv: PublishOptions): Promise { let branchName; if (rev) { logger.debug(`Trying to get branch name for provided revision: "${rev}"`); - branchName = ( - await git.raw('name-rev', '--name-only', '--no-undefined', rev) - ).trim(); + branchName = await getRevisionBranchName(git, rev); checkoutTarget = branchName || rev; logger.debug('Checking out revision', checkoutTarget); await git.checkout(checkoutTarget); @@ -696,7 +723,7 @@ export async function publishMain(argv: PublishOptions): Promise { } const publishStateFile = getPublishStatePath( newVersion, - publishStateGithubConfig, + getPublishStateGitHubConfig(publishStateGithubConfig), process.cwd(), getActiveWorkspace(), ); @@ -903,6 +930,20 @@ export async function publishMain(argv: PublishOptions): Promise { await runPostReleaseCommand(newVersion, config.postReleaseCommand); } +export async function getRevisionBranchName( + git: SimpleGit, + revision: string, +): Promise { + try { + return ( + await git.raw('name-rev', '--name-only', '--no-undefined', revision) + ).trim(); + } catch { + // A CI-approved SHA can be checked out detached without a named ref. + return ''; + } +} + export const handler = async (args: { [argName: string]: any; }): Promise => { diff --git a/src/commands/workspace.ts b/src/commands/workspace.ts new file mode 100644 index 000000000..3fd3ba892 --- /dev/null +++ b/src/commands/workspace.ts @@ -0,0 +1,13 @@ +import { Argv, CommandBuilder } from 'yargs'; + +import * as list from './workspace_cmds/list'; + +export const command = ['workspace ']; +export const description = 'Manage release workspaces'; + +export const builder: CommandBuilder = (yargs: Argv) => + yargs.demandCommand().command(list); + +export const handler = (): void => { + /* pass */ +}; diff --git a/src/commands/workspace_cmds/__tests__/list.test.ts b/src/commands/workspace_cmds/__tests__/list.test.ts new file mode 100644 index 000000000..e979cb180 --- /dev/null +++ b/src/commands/workspace_cmds/__tests__/list.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, test, vi } from 'vitest'; + +vi.mock('../../../config', () => ({ + getWorkspaceNames: vi.fn(), +})); +vi.mock('../../../utils/strings', () => ({ + formatJson: vi.fn(value => JSON.stringify(value)), +})); + +import { getWorkspaceNames } from '../../../config'; +import { handler } from '../list'; + +describe('workspace list command', () => { + test('prints exact configured workspace names', () => { + vi.mocked(getWorkspaceNames).mockReturnValue(['cli', 'mcp.v2']); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + handler(); + + expect(log).toHaveBeenCalledWith('["cli","mcp.v2"]'); + }); + + test('prints an empty array when no workspaces are configured', () => { + vi.mocked(getWorkspaceNames).mockReturnValue([]); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + handler(); + + expect(log).toHaveBeenCalledWith('[]'); + }); +}); diff --git a/src/commands/workspace_cmds/list.ts b/src/commands/workspace_cmds/list.ts new file mode 100644 index 000000000..05816ab6f --- /dev/null +++ b/src/commands/workspace_cmds/list.ts @@ -0,0 +1,9 @@ +import { getWorkspaceNames } from '../../config'; +import { formatJson } from '../../utils/strings'; + +export const command = ['list']; +export const description = 'List defined release workspaces as a JSON array'; + +export function handler(): void { + console.log(formatJson(getWorkspaceNames())); +} diff --git a/src/config.ts b/src/config.ts index c323addf9..02188395e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,5 +1,6 @@ -import { existsSync, lstatSync, readFileSync } from 'fs'; +import { existsSync, lstatSync, readFileSync, realpathSync } from 'fs'; import path from 'path'; +import { globSync, hasMagic } from 'glob'; import { load } from 'js-yaml'; import GitUrlParse from 'git-url-parse'; @@ -85,6 +86,7 @@ export function setActiveWorkspace(name: string | undefined): void { // Invalidate resolved caches so the next read applies the new selection. _configCache = undefined as unknown as CraftProjectConfig; _globalGitHubConfigCache = undefined; + _configPathCache = undefined as unknown as string; } /** @@ -103,17 +105,23 @@ export function getActiveWorkspace(): string | undefined { * top-level value (shallow override; a workspace either declares a field or * inherits it wholesale — we do not deep-merge arrays/objects, to keep * behavior predictable). - * - `github` is shallow-merged (owner/repo/projectPath) so a workspace can - * override just `projectPath` while inheriting owner/repo. + * - `github` is shallow-merged (owner/repo) so a workspace can override either + * value while inheriting the other. * - `minVersion` and `workspaces` themselves are stripped from the result. */ function resolveWorkspaceConfig( base: CraftProjectConfig, workspaceName: string, + workspaceDirectory: string, ): CraftProjectConfig { const workspaces = base.workspaces || {}; - if (!Object.hasOwn(workspaces, workspaceName)) { - const available = Object.keys(workspaces); + const workspace = getWorkspaceConfig( + workspaces, + workspaceName, + workspaceDirectory, + ); + if (!workspace) { + const available = getWorkspaceNamesFromConfig(base, workspaceDirectory); throw new ConfigurationError( `Unknown workspace "${workspaceName}". ` + (available.length @@ -121,7 +129,6 @@ function resolveWorkspaceConfig( : 'No workspaces are defined in the configuration.'), ); } - const workspace = workspaces[workspaceName]; // eslint-disable-next-line @typescript-eslint/no-unused-vars const { workspaces: _ignoredWorkspaces, ...baseWithoutWorkspaces } = base; @@ -135,8 +142,8 @@ function resolveWorkspaceConfig( continue; } if (key === 'github') { - // Shallow-merge github so a workspace can override a single field - // (e.g. just projectPath) while inheriting owner/repo from the base. + // Shallow-merge github so a workspace can override owner or repo while + // inheriting the other value from the base configuration. const mergedGithub = { ...(base.github as GitHubGlobalConfig | undefined), ...(value as Partial), @@ -144,9 +151,7 @@ function resolveWorkspaceConfig( // Only adopt the merged github if it is complete (has owner + repo). // Otherwise leave `github` unset so getGlobalGitHubConfig() can still // fall back to git-remote detection instead of seeing a truthy-but- - // incomplete object and skipping the fallback. (A workspace that only - // sets projectPath without a base github relies on git detection for - // owner/repo, exactly like a top-level config with no github block.) + // incomplete object and skipping the fallback. if (mergedGithub.owner && mergedGithub.repo) { resolved.github = mergedGithub as GitHubGlobalConfig; } else { @@ -160,6 +165,239 @@ function resolveWorkspaceConfig( return resolved; } +function getWorkspaceConfig( + workspaces: NonNullable, + workspaceName: string, + workspaceDirectory: string, +): Workspace | undefined { + if (isWorkspacePattern(workspaceName)) { + return undefined; + } + const matchingKeys = Object.keys(workspaces).filter(key => + workspaceKeyMatches(key, workspaceName, workspaceDirectory), + ); + if (matchingKeys.length === 1) { + return workspaces[matchingKeys[0]]; + } + if (matchingKeys.length > 1) { + throw new ConfigurationError( + `Workspace "${workspaceName}" matches multiple workspace patterns: ` + + `${matchingKeys.join(', ')}.`, + ); + } + return undefined; +} + +function workspaceKeyMatches( + key: string, + workspaceName: string, + workspaceDirectory: string, +): boolean { + return isWorkspacePattern(key) + ? getWorkspaceGlobMatches(key, workspaceDirectory).includes(workspaceName) + : key === workspaceName; +} + +function getWorkspaceNamesFromConfig( + config: CraftProjectConfig, + workspaceDirectory: string, +): string[] { + const workspaces = config.workspaces || {}; + const keysByWorkspaceName = new Map(); + + for (const key of Object.keys(workspaces)) { + const workspaceNames = isWorkspacePattern(key) + ? getWorkspaceGlobMatches(key, workspaceDirectory) + : [key]; + for (const workspaceName of workspaceNames) { + const matchingKeys = keysByWorkspaceName.get(workspaceName) || []; + matchingKeys.push(key); + keysByWorkspaceName.set(workspaceName, matchingKeys); + } + } + + for (const [workspaceName, matchingKeys] of keysByWorkspaceName) { + if (matchingKeys.length > 1) { + throw new ConfigurationError( + `Workspace "${workspaceName}" matches multiple workspace patterns: ` + + `${matchingKeys.join(', ')}.`, + ); + } + } + + return Array.from(keysByWorkspaceName.keys()).sort(); +} + +function getWorkspaceGlobMatches( + workspaceGlob: string, + workspaceDirectory: string, +): string[] { + if (!isSafeWorkspaceGlob(workspaceGlob)) { + throw new ConfigurationError( + `Workspace glob "${workspaceGlob}" must remain inside the repository root.`, + ); + } + + const root = path.resolve(workspaceDirectory); + const realRoot = realpathSync(root); + return globSync(workspaceGlob, { + absolute: false, + cwd: root, + dot: true, + ignore: ['**/node_modules/**'], + posix: true, + }) + .filter(match => { + const resolvedMatch = path.resolve(root, match); + let realMatch: string; + try { + realMatch = realpathSync(resolvedMatch); + } catch (error) { + // Glob candidates may disappear or be broken symlinks before resolution. + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return false; + } + throw error; + } + return ( + isSafeWorkspacePath(match) && + (resolvedMatch === root || + resolvedMatch.startsWith(`${root}${path.sep}`)) && + (realMatch === realRoot || + realMatch.startsWith(`${realRoot}${path.sep}`)) && + lstatSync(resolvedMatch).isDirectory() + ); + }) + .sort(); +} + +function isWorkspacePattern(name: string): boolean { + return hasMagic(name, { magicalBraces: true }); +} + +function isSafeWorkspacePath(name: string): boolean { + return ( + !path.isAbsolute(name) && + !name.includes('\\') && + name + .split('/') + .every( + segment => + /^[A-Za-z0-9_.-]+$/.test(segment) && + segment !== '' && + segment !== '.' && + segment !== '..' && + segment !== '__proto__' && + !segment.startsWith('-'), + ) + ); +} + +function isSafeWorkspaceGlob(name: string): boolean { + const alternatives = expandBraceAlternatives(name); + return ( + !path.isAbsolute(name) && + !name.includes('\\') && + alternatives !== undefined && + alternatives.length > 0 && + alternatives.every(expanded => + expanded.split('/').every(isSafeWorkspaceGlobSegment), + ) + ); +} + +function isSafeWorkspaceGlobSegment(segment: string): boolean { + if ( + segment === '' || + segment === '.' || + segment === '..' || + segment === '__proto__' || + segment.startsWith('-') + ) { + return false; + } + + return isSafeWorkspaceGlobPattern(segment); +} + +function isSafeWorkspaceGlobPattern(segment: string): boolean { + if ( + segment === '' || + segment === '.' || + segment === '..' || + segment === '__proto__' || + segment.startsWith('-') + ) { + return false; + } + + return hasMagic(segment, { magicalBraces: true }) + ? /^[A-Za-z0-9_.?*[\]!^-]+$/.test(segment) + : /^[A-Za-z0-9_.-]+$/.test(segment); +} + +function expandBraceAlternatives(pattern: string): string[] | undefined { + const opening = pattern.indexOf('{'); + if (opening === -1) { + return [pattern]; + } + + let depth = 0; + let closing = -1; + for (let index = opening; index < pattern.length; index++) { + if (pattern[index] === '{') { + depth++; + } else if (pattern[index] === '}' && --depth === 0) { + closing = index; + break; + } + } + if (closing === -1) { + return undefined; + } + + const choices = splitBraceAlternatives(pattern.slice(opening + 1, closing)); + if (!choices) { + return undefined; + } + + const prefix = pattern.slice(0, opening); + const suffix = pattern.slice(closing + 1); + const alternatives: string[] = []; + for (const choice of choices) { + const expanded = expandBraceAlternatives(`${prefix}${choice}${suffix}`); + if (!expanded) { + return undefined; + } + alternatives.push(...expanded); + } + return alternatives; +} + +function splitBraceAlternatives(content: string): string[] | undefined { + const choices: string[] = []; + let depth = 0; + let start = 0; + for (let index = 0; index < content.length; index++) { + if (content[index] === '{') { + depth++; + } else if (content[index] === '}') { + if (depth === 0) { + return undefined; + } + depth--; + } else if (content[index] === ',' && depth === 0) { + choices.push(content.slice(start, index)); + start = index + 1; + } + } + if (depth !== 0) { + return undefined; + } + choices.push(content.slice(start)); + return choices.length > 1 ? choices : undefined; +} + /** * Pure check: is `minVersionRaw` (a configured minVersion) >= `requiredVersion`? * @@ -184,6 +422,15 @@ function isVersionGteMinVersion( ); } +function checkWorkspacesMinVersion(config: CraftProjectConfig): void { + if (!isVersionGteMinVersion(config.minVersion, WORKSPACES_MIN_VERSION)) { + throw new ConfigurationError( + `Using "workspaces" requires minVersion >= ${WORKSPACES_MIN_VERSION} ` + + 'in the configuration file.', + ); + } +} + /** * SemVer build metadata does not affect precedence, but the comparison helper * intentionally rejects versions carrying it. Strip it before compatibility @@ -204,6 +451,7 @@ function withoutBuildMetadata(version: SemVer): SemVer { */ function applyWorkspaceSelection( config: CraftProjectConfig, + workspaceDirectory: string, ): CraftProjectConfig { const hasWorkspaces = !!config.workspaces && Object.keys(config.workspaces).length > 0; @@ -220,7 +468,10 @@ function applyWorkspaceSelection( // Workspaces are defined: require an explicit selection (no implicit first). if (!_activeWorkspaceName) { - const available = Object.keys(config.workspaces || {}).join(', '); + const available = getWorkspaceNamesFromConfig( + config, + workspaceDirectory, + ).join(', '); throw new ConfigurationError( 'This configuration defines workspaces; select one with ' + `--workspace (or the CRAFT_WORKSPACE env var). ` + @@ -229,14 +480,13 @@ function applyWorkspaceSelection( } // Gate the feature behind minVersion, mirroring auto-versioning. - if (!isVersionGteMinVersion(config.minVersion, WORKSPACES_MIN_VERSION)) { - throw new ConfigurationError( - `Using "workspaces" requires minVersion >= ${WORKSPACES_MIN_VERSION} ` + - 'in the configuration file.', - ); - } + checkWorkspacesMinVersion(config); - return resolveWorkspaceConfig(config, _activeWorkspaceName); + return resolveWorkspaceConfig( + config, + _activeWorkspaceName, + workspaceDirectory, + ); } /** @@ -245,9 +495,10 @@ function applyWorkspaceSelection( * Returns "undefined" if no file was found. */ export function findConfigFile(): string | undefined { - if (_configPathCache) { + if (_configPathCache && existsSync(_configPathCache)) { return _configPathCache; } + _configPathCache = undefined as unknown as string; const cwd = process.cwd(); const MAX_DEPTH = 1024; @@ -341,7 +592,7 @@ export function getConfiguration(clearCache = false): CraftProjectConfig { >; const parsed = validateConfiguration(rawConfig); checkMinimalConfigVersion(parsed); - _configCache = applyWorkspaceSelection(parsed); + _configCache = applyWorkspaceSelection(parsed, path.dirname(configPath)); return _configCache; } @@ -354,15 +605,39 @@ export function getConfiguration(clearCache = false): CraftProjectConfig { */ export function loadConfigurationFromString( configContent: string, + workspaceDirectory = process.cwd(), ): CraftProjectConfig { logger.debug('Loading configuration from provided content...'); const rawConfig = load(configContent) as Record; const parsed = validateConfiguration(rawConfig); checkMinimalConfigVersion(parsed); - _configCache = applyWorkspaceSelection(parsed); + _configCache = applyWorkspaceSelection(parsed, workspaceDirectory); return _configCache; } +/** + * Lists concrete workspace names from the validated configuration without + * applying a selection. Glob patterns expand relative to the configuration + * file, allowing external controllers to discover valid release paths. + */ +export function getWorkspaceNames(): string[] { + const configPath = getConfigFilePath(); + const rawConfig = load(readFileSync(configPath, 'utf-8')) as Record< + string, + any + >; + const parsed = validateConfiguration(rawConfig); + checkMinimalConfigVersion(parsed); + const workspaceNames = getWorkspaceNamesFromConfig( + parsed, + path.dirname(configPath), + ); + if (workspaceNames.length > 0) { + checkWorkspacesMinVersion(parsed); + } + return workspaceNames; +} + /** * Checks that the current "craft" version is compatible with the configuration * diff --git a/src/index.ts b/src/index.ts index ca25418b8..b7db51a1a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,6 +26,7 @@ import * as targets from './commands/targets'; import * as config from './commands/config'; import * as artifacts from './commands/artifacts'; import * as changelog from './commands/changelog'; +import * as workspace from './commands/workspace'; function printVersion(): void { if (!process.argv.includes('-v') && !process.argv.includes('--version')) { @@ -108,6 +109,7 @@ async function main(): Promise { .command(config) .command(artifacts) .command(changelog) + .command(workspace) .demandCommand() .version(getPackageVersion()) .alias('v', 'version') diff --git a/src/schemas/project_config.ts b/src/schemas/project_config.ts index e8f02b7f6..608c0a759 100644 --- a/src/schemas/project_config.ts +++ b/src/schemas/project_config.ts @@ -1,4 +1,5 @@ import { z } from 'zod'; +import { hasMagic } from 'glob'; /** * DEPRECATED: Use changelog.policy instead. Different policies for changelog management @@ -202,34 +203,167 @@ const releaseUnitFields = { * A workspace mirrors the release-relevant subset of the top-level config; * every field is optional and inherits the top-level value when omitted. The * `github` block is *partial* (all fields optional) so a workspace can override - * just `projectPath` (or `owner`/`repo`) while inheriting the rest from the - * top-level `github`. + * `owner` and/or `repo` while inheriting the rest from the top-level `github`. */ export const WorkspaceSchema = z.object({ ...releaseUnitFields, - github: GitHubGlobalConfigSchema.partial().optional(), + github: GitHubGlobalConfigSchema.partial() + .refine(github => github.projectPath === undefined, { + message: 'Workspace github.projectPath is not supported.', + }) + .optional(), }); export type Workspace = z.infer; +function isSafeWorkspaceGlobSegment(segment: string): boolean { + if ( + segment === '' || + segment === '.' || + segment === '..' || + segment === '__proto__' || + segment.startsWith('-') + ) { + return false; + } + + return isSafeWorkspaceGlobPattern(segment); +} + +function isSafeWorkspaceGlobPattern(segment: string): boolean { + if ( + segment === '' || + segment === '.' || + segment === '..' || + segment === '__proto__' || + segment.startsWith('-') + ) { + return false; + } + + return hasMagic(segment, { magicalBraces: true }) + ? /^[A-Za-z0-9_.?*[\]!^-]+$/.test(segment) + : /^[A-Za-z0-9_.-]+$/.test(segment); +} + +function expandBraceAlternatives(pattern: string): string[] | undefined { + const opening = pattern.indexOf('{'); + if (opening === -1) { + return [pattern]; + } + + let depth = 0; + let closing = -1; + for (let index = opening; index < pattern.length; index++) { + if (pattern[index] === '{') { + depth++; + } else if (pattern[index] === '}' && --depth === 0) { + closing = index; + break; + } + } + if (closing === -1) { + return undefined; + } + + const choices = splitBraceAlternatives(pattern.slice(opening + 1, closing)); + if (!choices) { + return undefined; + } + + const prefix = pattern.slice(0, opening); + const suffix = pattern.slice(closing + 1); + const alternatives: string[] = []; + for (const choice of choices) { + const expanded = expandBraceAlternatives(`${prefix}${choice}${suffix}`); + if (!expanded) { + return undefined; + } + alternatives.push(...expanded); + } + return alternatives; +} + +function isSafeWorkspaceGlob(name: string): boolean { + const alternatives = expandBraceAlternatives(name); + return ( + alternatives !== undefined && + alternatives.length > 0 && + alternatives.every(expanded => + expanded.split('/').every(isSafeWorkspaceGlobSegment), + ) + ); +} + +function splitBraceAlternatives(content: string): string[] | undefined { + const choices: string[] = []; + let depth = 0; + let start = 0; + for (let index = 0; index < content.length; index++) { + if (content[index] === '{') { + depth++; + } else if (content[index] === '}') { + if (depth === 0) { + return undefined; + } + depth--; + } else if (content[index] === ',' && depth === 0) { + choices.push(content.slice(start, index)); + start = index + 1; + } + } + if (depth !== 0) { + return undefined; + } + choices.push(content.slice(start)); + return choices.length > 1 ? choices : undefined; +} + +const WorkspaceNameSchema = z + .string() + // Assigning this key to a regular object mutates its prototype instead of + // preserving an own workspace entry. + .refine(name => name !== '__proto__', { + message: 'Workspace name "__proto__" is not supported.', + }) + .refine(name => name !== '.' && name !== '..', { + message: 'Workspace names cannot be "." or "..".', + }) + .refine(isSafeWorkspaceGlob, { + message: 'Workspace paths must use safe ASCII segments.', + }); + /** * Craft project-specific configuration */ -export const CraftProjectConfigSchema = z.object({ - ...releaseUnitFields, - minVersion: z - .string() - .regex(/^\d+\.\d+\.\d+.*$/) - .optional(), - /** - * Named, independently-versioned release units within a single repository. - * - * When present, a release run must select one via `--workspace ` (or - * `CRAFT_WORKSPACE`). The selected workspace's fields override the top-level - * ones. When absent, craft behaves exactly as before (the top-level config is - * the single implicit release unit) — fully backward compatible. - */ - workspaces: z.record(z.string(), WorkspaceSchema).optional(), -}); +export const CraftProjectConfigSchema = z + .object({ + ...releaseUnitFields, + minVersion: z + .string() + .regex(/^\d+\.\d+\.\d+.*$/) + .optional(), + /** + * Named, independently-versioned release units within a single repository. + * + * When present, a release run must select one via `--workspace ` (or + * `CRAFT_WORKSPACE`). The selected workspace's fields override the top-level + * ones. When absent, craft behaves exactly as before (the top-level config is + * the single implicit release unit) — fully backward compatible. + */ + workspaces: z.record(WorkspaceNameSchema, WorkspaceSchema).optional(), + }) + .superRefine((config, context) => { + if ( + Object.keys(config.workspaces || {}).length > 0 && + config.github?.projectPath !== undefined + ) { + context.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Workspace configurations cannot use github.projectPath.', + path: ['github', 'projectPath'], + }); + } + }); export type CraftProjectConfig = z.infer; diff --git a/src/utils/__tests__/publishState.test.ts b/src/utils/__tests__/publishState.test.ts index ea49b2ff9..2b157fe60 100644 --- a/src/utils/__tests__/publishState.test.ts +++ b/src/utils/__tests__/publishState.test.ts @@ -98,6 +98,21 @@ describe('publishState', () => { expect(new Set(filenames)).toHaveLength(names.length); }); + test('does not collide when release versions differ by build metadata case', () => { + const versions = ['4.2.6+sentry1', '4.2.6+Sentry1']; + const filenames = versions.map(version => + getPublishStateFilename(version, { owner: 'o', repo: 'r' }, cwd), + ); + + expect(new Set(filenames)).toHaveLength(versions.length); + expect(filenames).toEqual( + expect.arrayContaining([ + expect.stringMatching(/-version-NC4yLjYrc2VudHJ5MQ\.json$/), + expect.stringMatching(/-version-NC4yLjYrU2VudHJ5MQ\.json$/), + ]), + ); + }); + test('preserves the pre-workspace filename when no workspace is selected', () => { const name = getPublishStateFilename( '1.2.3', diff --git a/src/utils/publishState.ts b/src/utils/publishState.ts index 41aef0950..9d4961201 100644 --- a/src/utils/publishState.ts +++ b/src/utils/publishState.ts @@ -55,6 +55,13 @@ function sanitiseForFilename(raw: string): string { .replace(/^_+|_+$/g, ''); } +function encodeVersionForFilename(version: string): string { + const safeVersion = sanitiseForFilename(version); + return safeVersion === version + ? safeVersion + : `version-${Buffer.from(version).toString('base64url')}`; +} + /** * Short (12-char) hex digest of the absolute cwd path. Used to * disambiguate monorepo subpaths so `packages/foo` and `packages/bar` @@ -85,7 +92,7 @@ export function getPublishStateFilename( cwd: string = process.cwd(), workspace?: string, ): string { - const safeVersion = sanitiseForFilename(version); + const safeVersion = encodeVersionForFilename(version); const workspacePrefix = workspace ? `workspace-${Buffer.from(workspace).toString('base64url')}-` : '';