diff --git a/.github/scripts/lint-action-pinning.py b/.github/scripts/lint-action-pinning.py index a481a19d94..eeee7d2670 100755 --- a/.github/scripts/lint-action-pinning.py +++ b/.github/scripts/lint-action-pinning.py @@ -15,6 +15,9 @@ from pathlib import Path # A valid pinned ref is exactly 40 lowercase hex characters. SHA_RE = re.compile(r"^[0-9a-f]{40}$") +# Docker container actions are pinned by immutable registry digest. +DOCKER_DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$") + # Matches a `uses:` step line, capturing everything after `uses:` up to an # optional trailing YAML comment. The outer group is intentionally lazy so # trailing `# tag` comments are consumed by the optional group rather than @@ -25,7 +28,14 @@ USES_RE = re.compile(r"^\s*-?\s*uses:\s+(.+?)(?:\s+#.*)?$") workflows_dir = Path(__file__).parent.parent / "workflows" -violations: list[str] = [] +violations: list[tuple[str, int, str]] = [] + + +def strip_balanced_quotes(value: str) -> str: + if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: + return value[1:-1] + return value + for path in sorted(p for ext in ("*.yml", "*.yaml") for p in workflows_dir.glob(ext)): for lineno, line in enumerate(path.read_text().splitlines(), start=1): @@ -33,35 +43,44 @@ for path in sorted(p for ext in ("*.yml", "*.yaml") for p in workflows_dir.glob( if not m: continue + uses_value = strip_balanced_quotes(m.group(1).strip()) + + # Local actions and reusable workflows reference checked-out source, not + # an external registry. + if uses_value.startswith("./"): + continue + + if "${{" in uses_value: + rel = path.relative_to(Path(__file__).parent.parent.parent) + violations.append((str(rel), lineno, line.strip())) + continue + # Split on the last `@` to separate the action name from its ref. # rpartition returns ("", "", value) when `@` is absent. - uses_value = m.group(1).strip() action, sep, ref = uses_value.rpartition("@") - if not sep: - # No `@` — local action (`./...`) without a pin; skip. - continue - # Local reusable workflows: uses: ./.github/workflows/foo.yml@ref - # These reference local content with no external registry. - if action.startswith("./"): - continue - - # Docker container actions: uses: docker://image@sha256:digest - # The ref after `@` is a registry digest, not a commit SHA. if action.startswith("docker://"): + if sep and DOCKER_DIGEST_RE.match(ref): + continue + rel = path.relative_to(Path(__file__).parent.parent.parent) + violations.append((str(rel), lineno, line.strip())) continue - if not SHA_RE.match(ref): + if not sep or not SHA_RE.match(ref): rel = path.relative_to(Path(__file__).parent.parent.parent) violations.append((str(rel), lineno, line.strip())) if violations: for file, lineno, text in violations: # GitHub Actions annotation — shown inline on the PR diff. - print(f"::error file={file},line={lineno}::Unpinned action ref (use a 40-char commit SHA): {text}") + print( + f"::error file={file},line={lineno}::Unpinned action ref " + f"(use a 40-char commit SHA, or @sha256: for docker://): {text}" + ) print( f"\n{len(violations)} unpinned ref(s) found. " - "Pin each `uses:` to a commit SHA and keep the version tag as a comment:\n" + "Pin each GitHub action `uses:` to a commit SHA, and each docker:// action to a digest. " + "Keep the version tag as a comment:\n" " uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6", file=sys.stderr, ) diff --git a/.github/scripts/lint-action-pinning_test.py b/.github/scripts/lint-action-pinning_test.py new file mode 100644 index 0000000000..3496642735 --- /dev/null +++ b/.github/scripts/lint-action-pinning_test.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 +"""Regression tests for the workflow action pinning linter.""" + +import shutil +import subprocess +import sys +import tempfile +import textwrap +import unittest +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[2] +LINTER = REPO_ROOT / ".github" / "scripts" / "lint-action-pinning.py" + + +class LintActionPinningTest(unittest.TestCase): + def run_lint(self, workflow: str) -> subprocess.CompletedProcess[str]: + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + scripts_dir = tmp_path / ".github" / "scripts" + workflows_dir = tmp_path / ".github" / "workflows" + scripts_dir.mkdir(parents=True) + workflows_dir.mkdir(parents=True) + + shutil.copy2(LINTER, scripts_dir / "lint-action-pinning.py") + (workflows_dir / "test.yml").write_text(textwrap.dedent(workflow).strip() + "\n") + + return subprocess.run( + [sys.executable, str(scripts_dir / "lint-action-pinning.py")], + capture_output=True, + text=True, + check=False, + ) + + def test_rejects_docker_action_pinned_to_tag(self) -> None: + result = self.run_lint( + """ + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: docker://alpine:latest + """ + ) + + output = result.stdout + result.stderr + self.assertNotEqual(result.returncode, 0, output) + self.assertIn("docker://alpine:latest", output) + + def test_rejects_dynamic_uses_value(self) -> None: + result = self.run_lint( + """ + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: ${{ inputs.action }} + """ + ) + + output = result.stdout + result.stderr + self.assertNotEqual(result.returncode, 0, output) + self.assertIn("${{ inputs.action }}", output) + + def test_rejects_dynamic_action_source_pinned_to_sha(self) -> None: + result = self.run_lint( + """ + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: ${{ inputs.action }}@df4cb1c069e1874edd31b4311f1884172cec0e10 + """ + ) + + output = result.stdout + result.stderr + self.assertNotEqual(result.returncode, 0, output) + self.assertIn("${{ inputs.action }}@df4cb1c069e1874edd31b4311f1884172cec0e10", output) + + def test_rejects_docker_action_with_short_digest(self) -> None: + result = self.run_lint( + """ + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: docker://alpine@sha256:abc123 + """ + ) + + output = result.stdout + result.stderr + self.assertNotEqual(result.returncode, 0, output) + self.assertIn("docker://alpine@sha256:abc123", output) + + def test_rejects_docker_action_pinned_to_non_digest_ref(self) -> None: + result = self.run_lint( + """ + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: docker://alpine@v1.0 + """ + ) + + output = result.stdout + result.stderr + self.assertNotEqual(result.returncode, 0, output) + self.assertIn("docker://alpine@v1.0", output) + + def test_rejects_unpinned_action_without_at_sign(self) -> None: + result = self.run_lint( + """ + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout + """ + ) + + output = result.stdout + result.stderr + self.assertNotEqual(result.returncode, 0, output) + self.assertIn("uses: actions/checkout", output) + + def test_rejects_quoted_unpinned_action(self) -> None: + result = self.run_lint( + """ + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: "actions/checkout@v1" + """ + ) + + output = result.stdout + result.stderr + self.assertNotEqual(result.returncode, 0, output) + self.assertIn('"actions/checkout@v1"', output) + + def test_allows_pinned_action_local_action_and_docker_digest(self) -> None: + digest = "a" * 64 + result = self.run_lint( + f""" + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: ./.github/actions/setup-fixture + - uses: ./.github/workflows/reusable.yml@main + - uses: docker://alpine@sha256:{digest} + """ + ) + + output = result.stdout + result.stderr + self.assertEqual(result.returncode, 0, output) + + def test_allows_quoted_pinned_local_and_docker_uses_values(self) -> None: + digest = "b" * 64 + result = self.run_lint( + f""" + name: test + on: push + jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: "actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10" # v6 + - uses: './.github/actions/setup-fixture' + - uses: "docker://alpine@sha256:{digest}" + """ + ) + + output = result.stdout + result.stderr + self.assertEqual(result.returncode, 0, output) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/lint-action-pinning.yml b/.github/workflows/lint-action-pinning.yml index c8d0609e6f..c90768d030 100644 --- a/.github/workflows/lint-action-pinning.yml +++ b/.github/workflows/lint-action-pinning.yml @@ -6,11 +6,13 @@ on: paths: - ".github/workflows/**" - ".github/scripts/lint-action-pinning.py" + - ".github/scripts/lint-action-pinning_test.py" pull_request: branches: [main] paths: - ".github/workflows/**" - ".github/scripts/lint-action-pinning.py" + - ".github/scripts/lint-action-pinning_test.py" concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -31,5 +33,8 @@ jobs: with: persist-credentials: false + - name: Test action-pinning lint + run: python3 .github/scripts/lint-action-pinning_test.py + - name: Verify action refs run: python3 .github/scripts/lint-action-pinning.py diff --git a/Makefile b/Makefile index 265c67977e..273d1366a7 100644 --- a/Makefile +++ b/Makefile @@ -419,6 +419,7 @@ test-cli: .PHONY: test-scripts test-scripts: @printf "$(CYAN)Running script tests...$(RESET)\n" + @python3 .github/scripts/lint-action-pinning_test.py @bash scripts/pr-state.test.sh @bash scripts/opencode-code-review.test.sh @python3 scripts/opencode-code-review.test.py diff --git a/apps/cli/src/release-config.test.ts b/apps/cli/src/release-config.test.ts index 71d910b2fc..8602891897 100644 --- a/apps/cli/src/release-config.test.ts +++ b/apps/cli/src/release-config.test.ts @@ -94,7 +94,7 @@ function findPnpmSetupVersion( } function matchPnpmSetupUsesLine(line: string): RegExpMatchArray | null { - return line.match(/^(\s*)(?:-\s*)?uses:\s*["']?pnpm\/action-setup@v\d+["']?\s*(?:#.*)?$/); + return line.match(/^(\s*)(?:-\s*)?uses:\s*["']?pnpm\/action-setup@[0-9a-f]{40}["']?\s*(?:#.*)?$/); } function findStepIndent(lines: string[], usesLineIndex: number, usesIndent: number): number { @@ -233,7 +233,7 @@ describe("release desktop artifacts", () => { expect(workflow).toContain( 'rustup toolchain install stable --profile minimal --target "${{ matrix.rust_target }}"', ); - expect(workflow).toContain("Swatinem/rust-cache@v2"); + expect(workflow).toMatch(/Swatinem\/rust-cache@[0-9a-f]{40}\s+# v2/); for (const platform of desktopPlatforms) { expect(workflow, `release.yml must include desktop platform ${platform}`).toContain( diff --git a/apps/web/components/task/dockview-desktop-layout.tsx b/apps/web/components/task/dockview-desktop-layout.tsx index d9ea6b5bbb..f12df51583 100644 --- a/apps/web/components/task/dockview-desktop-layout.tsx +++ b/apps/web/components/task/dockview-desktop-layout.tsx @@ -536,6 +536,32 @@ type DockviewDesktopLayoutProps = { compact?: boolean; }; +function ReviewDialogSlot({ + sessionId, + review, +}: { + sessionId: string | null; + review: ReturnType; +}) { + if (!sessionId) { + return null; + } + + return ( + + ); +} + export const DockviewDesktopLayout = memo(function DockviewDesktopLayout({ sessionId, repository, @@ -643,19 +669,7 @@ export const DockviewDesktopLayout = memo(function DockviewDesktopLayout({ - {effectiveSessionId && ( - - )} + ); }); diff --git a/scripts/opencode-code-review.test.sh b/scripts/opencode-code-review.test.sh index 99858e04cc..18efa9eb89 100644 --- a/scripts/opencode-code-review.test.sh +++ b/scripts/opencode-code-review.test.sh @@ -100,7 +100,7 @@ if [[ "$trusted_script_count" != "2" ]]; then fi pass "OpenCode review executes the parser script from the trusted base commit in both workflow paths" -artifact_upload_count="$(count_occurrences 'uses: actions/upload-artifact@v4')" +artifact_upload_count="$(count_occurrences 'uses: actions/upload-artifact@')" if [[ "$artifact_upload_count" != "2" ]]; then fail "OpenCode review artifacts are uploaded from both workflow paths" fi