From ed50fb05b5dc91a52eea829318b50b77e3fb2759 Mon Sep 17 00:00:00 2001 From: Alassane <79718216+yattdev@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:48:11 -0400 Subject: [PATCH] fix(ci): use trusted preview deploy command (#3456) Co-authored-by: ayattara Co-authored-by: Carlos Florencio --- .../preview-env-workflow-contract_test.py | 123 ++++++++- .github/workflows/preview-env.yml | 252 +++++++++++++++--- apps/backend/cmd/preview/build.go | 25 +- apps/backend/cmd/preview/build_test.go | 72 ++++- apps/backend/cmd/preview/deploy.go | 104 +++++++- apps/backend/cmd/preview/deploy_test.go | 153 +++++++++++ apps/backend/cmd/preview/main.go | 4 + apps/backend/cmd/preview/package.go | 45 ++++ apps/backend/cmd/preview/package_test.go | 63 +++++ .../2026-08-24-unified-fork-approval-label.md | 25 +- .../2026-09-07-isolate-fork-preview-builds.md | 48 ++++ docs/decisions/INDEX.md | 1 + .../unified-contributor-pr-automation.md | 18 +- .../unified-contributor-pr-automation.md | 39 ++- 14 files changed, 895 insertions(+), 77 deletions(-) create mode 100644 apps/backend/cmd/preview/deploy_test.go create mode 100644 apps/backend/cmd/preview/package.go create mode 100644 apps/backend/cmd/preview/package_test.go create mode 100644 docs/decisions/2026-09-07-isolate-fork-preview-builds.md diff --git a/.github/scripts/preview-env-workflow-contract_test.py b/.github/scripts/preview-env-workflow-contract_test.py index 980eb8ff10..53189757df 100644 --- a/.github/scripts/preview-env-workflow-contract_test.py +++ b/.github/scripts/preview-env-workflow-contract_test.py @@ -8,6 +8,7 @@ import unittest REPO_ROOT = Path(__file__).resolve().parents[2] WORKFLOW = REPO_ROOT / ".github" / "workflows" / "preview-env.yml" +PREVIEW_BUILD = REPO_ROOT / "apps" / "backend" / "cmd" / "preview" / "build.go" def workflow_job(workflow: str, name: str) -> str: @@ -22,7 +23,7 @@ def workflow_job(workflow: str, name: str) -> str: class PreviewEnvironmentWorkflowContractTest(unittest.TestCase): def test_safe_to_review_and_allowlists_authorize_fork_preview_deployment(self) -> None: workflow = WORKFLOW.read_text(encoding="utf-8") - _, separator, deploy_job = workflow.partition(" deploy-fork:") + _, separator, deploy_job = workflow.partition(" build-fork-preview:") self.assertTrue(separator, "Fork preview deploy job is missing") self.assertIn("github.event_name == 'pull_request_target'", deploy_job) @@ -44,7 +45,7 @@ class PreviewEnvironmentWorkflowContractTest(unittest.TestCase): def test_safe_to_review_approval_survives_follow_up_pushes(self) -> None: workflow = WORKFLOW.read_text(encoding="utf-8") - _, separator, deploy_job = workflow.partition(" deploy-fork:") + _, separator, deploy_job = workflow.partition(" build-fork-preview:") self.assertTrue(separator, "Fork preview deploy job is missing") self.assertIn( @@ -81,6 +82,124 @@ class PreviewEnvironmentWorkflowContractTest(unittest.TestCase): self.assertIn("preview_url=", job) self.assertIn("--skip-description", workflow_job(workflow, "cleanup-preview")) + def test_fork_build_is_tokenless_and_deploy_uses_only_a_validated_artifact(self) -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + build_job = workflow_job(workflow, "build-fork-preview") + package_job = workflow_job(workflow, "package-fork-preview") + deploy_job = workflow_job(workflow, "deploy-fork") + + self.assertIn("permissions: {}", build_job) + self.assertNotIn('token: ""', build_job) + self.assertIn("github.event.pull_request.head.repo.full_name", build_job) + self.assertIn("pnpm -C apps install --frozen-lockfile", build_job) + self.assertIn("pnpm -C apps --filter @kandev/web build", build_job) + self.assertIn("actions/upload-artifact", build_job) + self.assertNotIn("secrets.", build_job) + for credential in ( + "GITHUB_TOKEN", + "GH_TOKEN", + "SPRITES_API_TOKEN", + "ACTIONS_ID_TOKEN_REQUEST_TOKEN", + "ACTIONS_RUNTIME_TOKEN", + ): + self.assertIn(f'{credential}: ""', build_job) + + self.assertIn("needs: package-fork-preview", deploy_job) + self.assertIn("needs: [build-trusted-preview-cli, build-fork-preview]", package_job) + self.assertIn("fork-preview-input-${{ github.run_id }}", package_job) + self.assertIn("trusted-preview-cli-${{ github.run_id }}", package_job) + self.assertIn("--skip-web-build", package_job) + self.assertIn("actions/download-artifact", deploy_job) + self.assertIn("sha256sum --check", deploy_job) + self.assertIn( + '(cd "$RUNNER_TEMP" && /usr/bin/sha256sum kandev-preview.tar.gz) ' + '> "$RUNNER_TEMP/kandev-preview.tar.gz.sha256"', + package_job, + ) + self.assertRegex( + deploy_job, + re.compile( + r"ref: \$\{\{ github\.workflow_sha \}\}\n" + r"\s+fetch-depth: 1\n" + r"\s+persist-credentials: false" + ), + ) + self.assertNotIn("github.event.pull_request.head.repo.full_name", deploy_job) + self.assertIn( + 'go run ./cmd/preview deploy --artifact "$ARTIFACT"', + deploy_job, + ) + self.assertIn("SPRITES_API_TOKEN", deploy_job) + + def test_trusted_preview_cli_has_a_fresh_runner_boundary_from_fork_lifecycle_scripts(self) -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + trusted_build = workflow_job(workflow, "build-trusted-preview-cli") + fork_build = workflow_job(workflow, "build-fork-preview") + package_job = workflow_job(workflow, "package-fork-preview") + + self.assertIn("permissions: {}", trusted_build) + self.assertIn("REPOSITORY: ${{ github.repository }}", trusted_build) + self.assertIn("REF: ${{ github.workflow_sha }}", trusted_build) + self.assertIn( + 'go build -o "$RUNNER_TEMP/kandev-preview-deploy" ./cmd/preview', + trusted_build, + ) + self.assertIn('GOWORK: "off"', trusted_build) + self.assertIn("trusted-preview-cli-${{ github.run_id }}", trusted_build) + self.assertIn( + "github.event.pull_request.head.repo.full_name != github.repository", + trusted_build, + ) + self.assertIn( + "contains(github.event.pull_request.labels.*.name, 'safe-to-review')", + trusted_build, + ) + + self.assertNotIn("actions/download-artifact", fork_build) + self.assertNotIn("trusted-preview-cli", fork_build) + self.assertNotIn("package --artifact", fork_build) + self.assertIn("fork-preview-input-${{ github.run_id }}", fork_build) + + self.assertIn("permissions: {}", package_job) + self.assertNotIn("github.event.pull_request.head.repo.full_name", package_job) + self.assertNotIn("pnpm -C apps install", package_job) + self.assertIn("actions/download-artifact", package_job) + self.assertIn("trusted-preview-cli-${{ github.run_id }}", package_job) + self.assertIn("fork-preview-input-${{ github.run_id }}", package_job) + self.assertIn('BASH_ENV: ""', package_job) + self.assertIn('env -i PATH="$TRUSTED_PATH"', package_job) + self.assertIn('"$RUNNER_TEMP/kandev-preview-deploy" package', package_job) + self.assertIn("--skip-web-build", package_job) + + def test_tokenless_checkouts_use_noninteractive_public_git_fetches(self) -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + + for name in ("build-trusted-preview-cli", "build-fork-preview"): + job = workflow_job(workflow, name) + self.assertNotIn("actions/checkout", job) + self.assertIn("git init", job) + self.assertIn("-c credential.helper= fetch --depth=1 origin", job) + self.assertIn("checkout --detach FETCH_HEAD", job) + self.assertIn('GIT_TERMINAL_PROMPT: "0"', job) + self.assertIn("GIT_ASKPASS: /bin/false", job) + self.assertIn("-c credential.helper=", job) + self.assertIn('GITHUB_TOKEN: ""', job) + self.assertIn('GH_TOKEN: ""', job) + + def test_fork_build_subprocesses_cannot_inherit_deployment_credentials(self) -> None: + build_source = PREVIEW_BUILD.read_text(encoding="utf-8") + + self.assertIn("func untrustedBuildEnv", build_source) + for credential in ( + "SPRITES_API_TOKEN", + "GH_TOKEN", + "GITHUB_TOKEN", + "ACTIONS_ID_TOKEN_REQUEST_TOKEN", + "ACTIONS_RUNTIME_TOKEN", + ): + self.assertIn(credential, build_source) + self.assertEqual(build_source.count("untrustedBuildEnv(os.Environ())"), 3) + if __name__ == "__main__": unittest.main() diff --git a/.github/workflows/preview-env.yml b/.github/workflows/preview-env.yml index ac56e85e59..745396aa5b 100644 --- a/.github/workflows/preview-env.yml +++ b/.github/workflows/preview-env.yml @@ -73,22 +73,11 @@ jobs: SPRITES_API_TOKEN: ${{ secrets.SPRITES_API_TOKEN }} # ----------------------------------------------------------------------- - # Deploy / update preview (fork PRs, gated by allowlist) - # Uses pull_request_target so secrets are available. - # Three approval paths: - # - PREVIEW_ENV_ALLOWLIST repo variable (trusted login list, all events; - # JSON array of logins) - # - CLAUDE_REVIEW_ALLOWLIST repo variable (trusted login list, all events; - # the Claude workflow also adds the approval labels on open) - # - `safe-to-review` label (maintainer opt-in, durable until removed). It - # authorizes the current and subsequent fork heads. This matters more - # here than for claude-code-review.yml because deploy actually runs the - # fork's `cmd/preview` code with SPRITES_API_TOKEN. + # Build the preview CLI from the immutable workflow revision before any + # runner checks out or executes fork-controlled content. The fork build job + # downloads this artifact only after its lifecycle scripts have completed. # ----------------------------------------------------------------------- - # Branch order matters: keep cheap/safe checks before `fromJSON(vars.*)`, - # which throws (and skips the whole job) if the repo variable is - # non-empty but unparseable JSON. - deploy-fork: + build-trusted-preview-cli: if: > github.event.action != 'closed' && github.event_name == 'pull_request_target' && @@ -97,30 +86,142 @@ jobs: (vars.PREVIEW_ENV_ALLOWLIST != '' && contains(fromJSON(vars.PREVIEW_ENV_ALLOWLIST), github.event.pull_request.user.login)) || (vars.CLAUDE_REVIEW_ALLOWLIST != '' && contains(fromJSON(vars.CLAUDE_REVIEW_ALLOWLIST), github.event.pull_request.user.login))) runs-on: ubuntu-latest - outputs: - preview_url: ${{ steps.deploy.outputs.preview_url }} - permissions: - contents: read + permissions: {} steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 - with: - # pull_request_target checks out base by default; explicitly use PR head. - # Safe: the preview CLI only reads files to build artifacts. - # NOTE: The allowlist gate (safe-to-review, PREVIEW_ENV_ALLOWLIST, - # CLAUDE_REVIEW_ALLOWLIST) - # authorizes running the fork's cmd/preview code with SPRITES_API_TOKEN. - # The safe-to-review label remains an explicit maintainer approval for - # follow-up pushes until the maintainer removes it. - repository: ${{ github.event.pull_request.head.repo.full_name }} - ref: ${{ github.event.pull_request.head.sha }} - persist-credentials: false + - name: Check out immutable preview command source without credentials + env: + REPOSITORY: ${{ github.repository }} + REF: ${{ github.workflow_sha }} + GIT_TERMINAL_PROMPT: "0" + GIT_ASKPASS: /bin/false + GITHUB_TOKEN: "" + GH_TOKEN: "" + run: | + set -euo pipefail + git init "$GITHUB_WORKSPACE" + git -C "$GITHUB_WORKSPACE" remote add origin "https://github.com/${REPOSITORY}.git" + git -C "$GITHUB_WORKSPACE" -c credential.helper= fetch --depth=1 origin "$REF" + git -C "$GITHUB_WORKSPACE" checkout --detach FETCH_HEAD - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version-file: apps/backend/go.mod cache-dependency-path: apps/backend/go.sum + - name: Build trusted preview command + working-directory: apps/backend + env: + GOWORK: "off" + run: go build -o "$RUNNER_TEMP/kandev-preview-deploy" ./cmd/preview + + - name: Upload trusted preview command + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: trusted-preview-cli-${{ github.run_id }} + path: ${{ runner.temp }}/kandev-preview-deploy + if-no-files-found: error + retention-days: 1 + + # ----------------------------------------------------------------------- + # Build the fork-controlled frontend and collect its inert source/dist input + # without credentials. This job ends before the trusted preview command is + # downloaded or invoked, so a fork lifecycle script cannot retain a process + # that can alter the trusted command. + # Three approval paths: + # - PREVIEW_ENV_ALLOWLIST repo variable (trusted login list, all events; + # JSON array of logins) + # - CLAUDE_REVIEW_ALLOWLIST repo variable (trusted login list, all events; + # the Claude workflow also adds the approval labels on open) + # - `safe-to-review` label (maintainer opt-in, durable until removed). It + # authorizes the current and subsequent fork heads for tokenless input + # build and trusted artifact deployment. + # ----------------------------------------------------------------------- + # Branch order matters: keep cheap/safe checks before `fromJSON(vars.*)`, + # which throws (and skips the whole job) if the repo variable is + # non-empty but unparseable JSON. + build-fork-preview: + if: > + github.event.action != 'closed' && + github.event_name == 'pull_request_target' && + github.event.pull_request.head.repo.full_name != github.repository && + ((contains(github.event.pull_request.labels.*.name, 'safe-to-review')) || + (vars.PREVIEW_ENV_ALLOWLIST != '' && contains(fromJSON(vars.PREVIEW_ENV_ALLOWLIST), github.event.pull_request.user.login)) || + (vars.CLAUDE_REVIEW_ALLOWLIST != '' && contains(fromJSON(vars.CLAUDE_REVIEW_ALLOWLIST), github.event.pull_request.user.login))) + runs-on: ubuntu-latest + permissions: {} + + steps: + - uses: pnpm/action-setup@a8198c4bff370c8506180b035930dea56dbd5288 # v5 + with: + version: "9.15.9" + + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 + with: + node-version: "24" + + - name: Check out fork source without credentials + # The fork controls its product source and build hooks. This job has no + # repository or deployment credentials. + env: + REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }} + REF: ${{ github.event.pull_request.head.sha }} + GIT_TERMINAL_PROMPT: "0" + GIT_ASKPASS: /bin/false + GITHUB_TOKEN: "" + GH_TOKEN: "" + run: | + set -euo pipefail + git init "$GITHUB_WORKSPACE" + git -C "$GITHUB_WORKSPACE" remote add origin "https://github.com/${REPOSITORY}.git" + git -C "$GITHUB_WORKSPACE" -c credential.helper= fetch --depth=1 origin "$REF" + git -C "$GITHUB_WORKSPACE" checkout --detach FETCH_HEAD + + - name: Install frontend dependencies + env: + GITHUB_TOKEN: "" + GH_TOKEN: "" + SPRITES_API_TOKEN: "" + ACTIONS_ID_TOKEN_REQUEST_TOKEN: "" + ACTIONS_RUNTIME_TOKEN: "" + run: pnpm -C apps install --frozen-lockfile + + - name: Build fork frontend + env: + GITHUB_TOKEN: "" + GH_TOKEN: "" + SPRITES_API_TOKEN: "" + ACTIONS_ID_TOKEN_REQUEST_TOKEN: "" + ACTIONS_RUNTIME_TOKEN: "" + run: pnpm -C apps --filter @kandev/web build + + - name: Upload fork preview input + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + # The packager receives only source and already-built static assets. + name: fork-preview-input-${{ github.run_id }} + path: | + apps/backend + apps/web/dist + if-no-files-found: error + retention-days: 1 + + # ----------------------------------------------------------------------- + # Assemble a fork preview in a fresh tokenless runner. No fork checkout or + # lifecycle command precedes this job, so only this trusted executable can + # start the package operation. + # ----------------------------------------------------------------------- + package-fork-preview: + needs: [build-trusted-preview-cli, build-fork-preview] + if: needs.build-trusted-preview-cli.result == 'success' && needs.build-fork-preview.result == 'success' + runs-on: ubuntu-latest + permissions: {} + + steps: + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + with: + go-version: "1.26.0" + - uses: pnpm/action-setup@a8198c4bff370c8506180b035930dea56dbd5288 # v5 with: version: "9.15.9" @@ -129,15 +230,97 @@ jobs: with: node-version: "24" - - name: Install frontend dependencies - run: pnpm -C apps install --frozen-lockfile + - name: Record trusted tool paths + id: trusted-tools + run: | + set -euo pipefail + pnpm_dir="$(dirname "$(command -v pnpm)")" + node_dir="$(dirname "$(command -v node)")" + go_dir="$(dirname "$(command -v go)")" + printf 'path=%s:%s:%s:/usr/bin:/bin\n' "$pnpm_dir" "$node_dir" "$go_dir" >> "$GITHUB_OUTPUT" + + - name: Download trusted preview command + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: trusted-preview-cli-${{ github.run_id }} + path: ${{ runner.temp }} + + - name: Download fork preview input + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: fork-preview-input-${{ github.run_id }} + path: ${{ runner.temp }}/fork-preview-input + + - name: Package fork preview artifact + working-directory: ${{ runner.temp }}/fork-preview-input/backend + env: + BASH_ENV: "" + TRUSTED_PATH: ${{ steps.trusted-tools.outputs.path }} + run: | + set -euo pipefail + /usr/bin/mkdir -p "$RUNNER_TEMP/kandev-preview-home" + /usr/bin/chmod 500 "$RUNNER_TEMP/kandev-preview-deploy" + /usr/bin/env -i PATH="$TRUSTED_PATH" HOME="$RUNNER_TEMP/kandev-preview-home" \ + "$RUNNER_TEMP/kandev-preview-deploy" package --artifact "$RUNNER_TEMP/kandev-preview.tar.gz" --skip-web-install --skip-web-build + (cd "$RUNNER_TEMP" && /usr/bin/sha256sum kandev-preview.tar.gz) > "$RUNNER_TEMP/kandev-preview.tar.gz.sha256" + + - name: Upload fork preview artifact + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: fork-preview-${{ github.run_id }} + path: | + ${{ runner.temp }}/kandev-preview.tar.gz + ${{ runner.temp }}/kandev-preview.tar.gz.sha256 + if-no-files-found: error + retention-days: 1 + + # ----------------------------------------------------------------------- + # Deploy a completed fork bundle. This fresh job checks out only the + # immutable workflow revision and never runs the fork checkout or its hooks. + # ----------------------------------------------------------------------- + deploy-fork: + needs: package-fork-preview + if: needs.package-fork-preview.result == 'success' + runs-on: ubuntu-latest + outputs: + preview_url: ${{ steps.deploy.outputs.preview_url }} + permissions: + contents: read + + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + repository: ${{ github.repository }} + ref: ${{ github.workflow_sha }} + fetch-depth: 1 + persist-credentials: false + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + with: + go-version-file: apps/backend/go.mod + cache-dependency-path: apps/backend/go.sum + + - name: Download and validate fork preview artifact + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: fork-preview-${{ github.run_id }} + path: ${{ runner.temp }}/fork-preview + + - name: Validate fork preview artifact + env: + ARTIFACT: ${{ runner.temp }}/fork-preview/kandev-preview.tar.gz + run: | + set -euo pipefail + cd "$(dirname "$ARTIFACT")" + sha256sum --check kandev-preview.tar.gz.sha256 + tar -tzf "$ARTIFACT" > /dev/null - name: Deploy preview environment id: deploy working-directory: apps/backend run: | set -euo pipefail - deploy_output="$(go run ./cmd/preview deploy --pr "$PR" --sha "$SHA" --repo "$REPO" --skip-web-install --skip-description)" + deploy_output="$(go run ./cmd/preview deploy --artifact "$ARTIFACT" --pr "$PR" --sha "$SHA" --repo "$REPO" --skip-description)" printf '%s\n' "$deploy_output" preview_url="$(printf '%s\n' "$deploy_output" | sed -n 's/^preview URL: //p' | tail -n 1)" test -n "$preview_url" @@ -146,6 +329,7 @@ jobs: PR: ${{ github.event.pull_request.number }} SHA: ${{ github.event.pull_request.head.sha }} REPO: ${{ github.repository }} + ARTIFACT: ${{ runner.temp }}/fork-preview/kandev-preview.tar.gz SPRITES_API_TOKEN: ${{ secrets.SPRITES_API_TOKEN }} # ----------------------------------------------------------------------- diff --git a/apps/backend/cmd/preview/build.go b/apps/backend/cmd/preview/build.go index b37bb3edd9..29049d0928 100644 --- a/apps/backend/cmd/preview/build.go +++ b/apps/backend/cmd/preview/build.go @@ -11,6 +11,7 @@ import ( "os/exec" "path/filepath" "regexp" + "strings" ) // appsDir is the apps/ workspace root relative to the working directory (apps/backend/). @@ -19,6 +20,26 @@ const appsDir = ".." // goDockerImage is used to cross-compile CGO binaries on non-linux/amd64 hosts. const goDockerImage = "golang:1.26-bookworm" +var untrustedBuildCredentialNames = map[string]struct{}{ + "SPRITES_API_TOKEN": {}, + "GH_TOKEN": {}, + "GITHUB_TOKEN": {}, + "ACTIONS_ID_TOKEN_REQUEST_TOKEN": {}, + "ACTIONS_RUNTIME_TOKEN": {}, +} + +// untrustedBuildEnv removes credentials before starting build tooling from a fork checkout. +func untrustedBuildEnv(env []string) []string { + sanitized := make([]string, 0, len(env)) + for _, entry := range env { + name, _, _ := strings.Cut(entry, "=") + if _, isCredential := untrustedBuildCredentialNames[name]; !isCredential { + sanitized = append(sanitized, entry) + } + } + return sanitized +} + // buildLinuxBinaries compiles kandev, agentctl, and mock-agent for linux/amd64. // Run this from apps/backend/. agentctl and mock-agent use CGO_ENABLED=0 and // always build natively. kandev requires CGO (SQLite) and is always built inside @@ -36,7 +57,7 @@ func buildLinuxBinaries(ctx context.Context, outDir string) error { out := filepath.Join(outDir, b.name) fmt.Fprintf(os.Stderr, " go build %s -> %s\n", b.pkg, out) cmd := exec.CommandContext(ctx, "go", "build", "-ldflags", "-s -w", "-o", out, b.pkg) - cmd.Env = append(os.Environ(), "GOOS=linux", "GOARCH=amd64", "CGO_ENABLED=0") + cmd.Env = append(untrustedBuildEnv(os.Environ()), "GOOS=linux", "GOARCH=amd64", "CGO_ENABLED=0") cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr if err := cmd.Run(); err != nil { return fmt.Errorf("build %s: %w", b.name, err) @@ -82,6 +103,7 @@ func buildKandevDocker(ctx context.Context, out string) error { "-o", containerOut, "./cmd/kandev", ) + cmd.Env = untrustedBuildEnv(os.Environ()) cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr if err := cmd.Run(); err != nil { return fmt.Errorf("docker build kandev: %w", err) @@ -131,6 +153,7 @@ func buildWeb(ctx context.Context, skipInstall bool) error { fmt.Fprintf(os.Stderr, " %s\n", step.desc) cmd := exec.CommandContext(ctx, step.args[0], step.args[1:]...) cmd.Dir = appsDir + cmd.Env = untrustedBuildEnv(os.Environ()) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { diff --git a/apps/backend/cmd/preview/build_test.go b/apps/backend/cmd/preview/build_test.go index c64f850999..8c9c6a1726 100644 --- a/apps/backend/cmd/preview/build_test.go +++ b/apps/backend/cmd/preview/build_test.go @@ -1,6 +1,76 @@ package main -import "testing" +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestPreviewArtifactExists(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + missing := filepath.Join(tempDir, "missing.tar.gz") + if exists, err := previewArtifactExists(missing); err != nil || exists { + t.Fatalf("previewArtifactExists(missing) = (%v, %v), want (false, nil)", exists, err) + } + + if exists, err := previewArtifactExists(tempDir); err == nil || exists { + t.Fatalf("previewArtifactExists(directory) = (%v, %v), want (false, error)", exists, err) + } + + artifact := filepath.Join(tempDir, "preview.tar.gz") + if err := os.WriteFile(artifact, []byte("bundle"), 0o600); err != nil { + t.Fatal(err) + } + if exists, err := previewArtifactExists(artifact); err != nil || !exists { + t.Fatalf("previewArtifactExists(file) = (%v, %v), want (true, nil)", exists, err) + } +} + +func TestUntrustedBuildEnvRemovesCredentials(t *testing.T) { + t.Parallel() + + buildEnv := untrustedBuildEnv([]string{ + "PATH=/usr/bin", + "HOME=/tmp/kandev", + "SPRITES_API_TOKEN=sprites-secret", + "GH_TOKEN=github-secret", + "GITHUB_TOKEN=github-actions-secret", + "ACTIONS_ID_TOKEN_REQUEST_TOKEN=oidc-secret", + "ACTIONS_RUNTIME_TOKEN=runtime-secret", + }) + + got := strings.Join(buildEnv, "\n") + for _, credential := range []string{ + "SPRITES_API_TOKEN=", "GH_TOKEN=", "GITHUB_TOKEN=", "ACTIONS_ID_TOKEN_REQUEST_TOKEN=", "ACTIONS_RUNTIME_TOKEN=", + } { + if strings.Contains(got, credential) { + t.Fatalf("untrustedBuildEnv() retained %q", credential) + } + } + if got != "PATH=/usr/bin\nHOME=/tmp/kandev" { + t.Fatalf("untrustedBuildEnv() = %q, want preserved non-credential environment", got) + } +} + +func TestRunPackageRequiresArtifact(t *testing.T) { + if got := runPackage(context.Background(), nil); got != 2 { + t.Fatalf("runPackage(nil) = %d, want 2", got) + } +} + +func TestRunDispatchesPackageCommand(t *testing.T) { + previousArgs := os.Args + os.Args = []string{"preview", "package"} + t.Cleanup(func() { os.Args = previousArgs }) + + if got := run(); got != 2 { + t.Fatalf("run() = %d, want 2 for package command without an artifact", got) + } +} func TestViteIndexHasEntrypoint(t *testing.T) { t.Parallel() diff --git a/apps/backend/cmd/preview/deploy.go b/apps/backend/cmd/preview/deploy.go index 416b3541ab..471a94821e 100644 --- a/apps/backend/cmd/preview/deploy.go +++ b/apps/backend/cmd/preview/deploy.go @@ -19,6 +19,7 @@ func runDeploy(ctx context.Context, args []string) int { port := fs.Int("port", ports.Backend, "kandev backend port exposed by the sprite") skipWebInstall := fs.Bool("skip-web-install", false, "skip pnpm install (CI already ran it)") skipDescription := fs.Bool("skip-description", false, "skip the PR description update") + artifact := fs.String("artifact", "", "prebuilt preview bundle to deploy") if err := fs.Parse(args); err != nil { fmt.Fprintf(os.Stderr, "preview deploy: %v\n", err) @@ -32,6 +33,18 @@ func runDeploy(ctx context.Context, args []string) int { fmt.Fprintln(os.Stderr, "preview deploy: --repo or GITHUB_REPOSITORY is required") return 2 } + artifactProvided := *artifact != "" + if artifactProvided { + exists, err := previewArtifactExists(*artifact) + if err != nil { + fmt.Fprintf(os.Stderr, "preview deploy: %v\n", err) + return 2 + } + if !exists { + fmt.Fprintf(os.Stderr, "preview deploy: prebuilt artifact %s does not exist\n", *artifact) + return 2 + } + } spritesToken := os.Getenv("SPRITES_API_TOKEN") if spritesToken == "" { @@ -53,10 +66,12 @@ func runDeploy(ctx context.Context, args []string) int { } defer func() { _ = os.RemoveAll(tmpDir) }() - binDir := filepath.Join(tmpDir, "bin") - tarPath := filepath.Join(tmpDir, "kandev-preview.tar.gz") + tarPath := *artifact + if tarPath == "" { + tarPath = filepath.Join(tmpDir, "kandev-preview.tar.gz") + } - previewURL, err := deployArtifacts(ctx, binDir, tarPath, spritesToken, spriteName, *port, *skipWebInstall) + previewURL, err := deployArtifacts(ctx, tarPath, spritesToken, spriteName, *port, *skipWebInstall, !artifactProvided) if err != nil { fmt.Fprintf(os.Stderr, "preview deploy: %v\n", err) return 1 @@ -76,24 +91,89 @@ func runDeploy(ctx context.Context, args []string) int { return 0 } -// deployArtifacts builds the bundle, deploys it to the sprite, and returns the public URL. +// deployArtifacts deploys an archive or builds one when buildIfMissing is true. // Using a single client for the full flow avoids redundant auth round-trips. -func deployArtifacts(ctx context.Context, binDir, tarPath, spritesToken, spriteName string, port int, skipWebInstall bool) (string, error) { - fmt.Fprintln(os.Stderr, "building linux/amd64 binaries...") - if err := buildLinuxBinaries(ctx, binDir); err != nil { - return "", fmt.Errorf("build binaries: %w", err) +func deployArtifacts(ctx context.Context, tarPath, spritesToken, spriteName string, port int, skipWebInstall, buildIfMissing bool) (string, error) { + return deployArtifactsWithHooks( + ctx, + tarPath, + spritesToken, + spriteName, + port, + skipWebInstall, + buildIfMissing, + buildPreviewArtifact, + deployPreviewArtifact, + ) +} + +type previewArtifactBuilder func(context.Context, string, string, bool, bool) error +type previewArtifactDeployer func(context.Context, string, string, string, int) (string, error) + +func deployArtifactsWithHooks( + ctx context.Context, + tarPath, spritesToken, spriteName string, + port int, + skipWebInstall, buildIfMissing bool, + build previewArtifactBuilder, + deploy previewArtifactDeployer, +) (string, error) { + exists, err := previewArtifactExists(tarPath) + if err != nil { + return "", err } - fmt.Fprintln(os.Stderr, "building web frontend...") - if err := buildWeb(ctx, skipWebInstall); err != nil { - return "", fmt.Errorf("build web: %w", err) + if !exists { + if !buildIfMissing { + return "", fmt.Errorf("prebuilt artifact %s does not exist", tarPath) + } + binDir := filepath.Join(filepath.Dir(tarPath), "bin") + if err := build(ctx, binDir, tarPath, skipWebInstall, false); err != nil { + return "", err + } + } + + return deploy(ctx, tarPath, spritesToken, spriteName, port) +} + +func previewArtifactExists(tarPath string) (bool, error) { + if tarPath == "" { + return false, fmt.Errorf("preview artifact path is required") + } + info, err := os.Stat(tarPath) + if os.IsNotExist(err) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("stat preview artifact %s: %w", tarPath, err) + } + if !info.Mode().IsRegular() { + return false, fmt.Errorf("preview artifact %s is not a regular file", tarPath) + } + return true, nil +} + +func buildPreviewArtifact(ctx context.Context, binDir, tarPath string, skipWebInstall, skipWebBuild bool) error { + fmt.Fprintln(os.Stderr, "building linux/amd64 binaries...") + if err := buildLinuxBinaries(ctx, binDir); err != nil { + return fmt.Errorf("build binaries: %w", err) + } + + if !skipWebBuild { + fmt.Fprintln(os.Stderr, "building web frontend...") + if err := buildWeb(ctx, skipWebInstall); err != nil { + return fmt.Errorf("build web: %w", err) + } } fmt.Fprintln(os.Stderr, "packaging bundle...") if err := packageBundle(binDir, tarPath); err != nil { - return "", fmt.Errorf("package bundle: %w", err) + return fmt.Errorf("package bundle: %w", err) } + return nil +} +func deployPreviewArtifact(ctx context.Context, tarPath, spritesToken, spriteName string, port int) (string, error) { client := newSpriteClient(spritesToken) defer func() { _ = client.Close() }() diff --git a/apps/backend/cmd/preview/deploy_test.go b/apps/backend/cmd/preview/deploy_test.go new file mode 100644 index 0000000000..66938e0f97 --- /dev/null +++ b/apps/backend/cmd/preview/deploy_test.go @@ -0,0 +1,153 @@ +package main + +import ( + "bytes" + "context" + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRunDeployRejectsMissingExplicitArtifactBeforeCredentials(t *testing.T) { + t.Setenv("SPRITES_API_TOKEN", "") + t.Setenv("GH_TOKEN", "") + + artifact := filepath.Join(t.TempDir(), "missing.tar.gz") + stderr, restoreStderr := capturePreviewStderr(t) + + code := runDeploy(context.Background(), []string{ + "--pr", "3456", + "--repo", "kdlbs/kandev", + "--artifact", artifact, + "--skip-description", + }) + restoreStderr() + + if code != 2 { + t.Fatalf("runDeploy() = %d, want 2", code) + } + if !strings.Contains(stderr.String(), "prebuilt artifact") { + t.Fatalf("stderr = %q, want a prebuilt artifact error", stderr.String()) + } +} + +func TestDeployArtifactsDoesNotBuildExistingArtifact(t *testing.T) { + artifact := filepath.Join(t.TempDir(), "preview.tar.gz") + if err := os.WriteFile(artifact, []byte("bundle"), 0o600); err != nil { + t.Fatal(err) + } + + built := false + deployed := false + got, err := deployArtifactsWithHooks( + context.Background(), artifact, "test-token", "kandev-pr-3456", 3000, true, false, + func(context.Context, string, string, bool, bool) error { + built = true + return nil + }, + func(_ context.Context, tarPath, _, _ string, _ int) (string, error) { + deployed = true + if tarPath != artifact { + t.Fatalf("deployed artifact = %q, want %q", tarPath, artifact) + } + return "https://preview.example", nil + }, + ) + if err != nil { + t.Fatalf("deployArtifactsWithHooks() error = %v", err) + } + if got != "https://preview.example" { + t.Fatalf("deployArtifactsWithHooks() = %q, want preview URL", got) + } + if built { + t.Fatal("deployArtifactsWithHooks() built an existing artifact") + } + if !deployed { + t.Fatal("deployArtifactsWithHooks() did not deploy the existing artifact") + } +} + +func TestDeployArtifactsBuildsWhenArtifactIsNotProvided(t *testing.T) { + artifact := filepath.Join(t.TempDir(), "preview.tar.gz") + + built := false + got, err := deployArtifactsWithHooks( + context.Background(), artifact, "test-token", "kandev-pr-3456", 3000, true, true, + func(_ context.Context, _, tarPath string, _, _ bool) error { + built = true + return os.WriteFile(tarPath, []byte("bundle"), 0o600) + }, + func(_ context.Context, tarPath, _, _ string, _ int) (string, error) { + if tarPath != artifact { + t.Fatalf("deployed artifact = %q, want %q", tarPath, artifact) + } + return "https://preview.example", nil + }, + ) + if err != nil { + t.Fatalf("deployArtifactsWithHooks() error = %v", err) + } + if got != "https://preview.example" { + t.Fatalf("deployArtifactsWithHooks() = %q, want preview URL", got) + } + if !built { + t.Fatal("deployArtifactsWithHooks() did not build the missing artifact") + } +} + +func TestDeployArtifactsRejectsMissingArtifactWhenBuildDisabled(t *testing.T) { + artifact := filepath.Join(t.TempDir(), "preview.tar.gz") + built := false + deployed := false + + _, err := deployArtifactsWithHooks( + context.Background(), artifact, "test-token", "kandev-pr-3456", 3000, true, false, + func(context.Context, string, string, bool, bool) error { + built = true + return nil + }, + func(context.Context, string, string, string, int) (string, error) { + deployed = true + return "", nil + }, + ) + if err == nil || !strings.Contains(err.Error(), "prebuilt artifact") { + t.Fatalf("deployArtifactsWithHooks() error = %v, want missing prebuilt artifact error", err) + } + if built { + t.Fatal("deployArtifactsWithHooks() built an artifact when build was disabled") + } + if deployed { + t.Fatal("deployArtifactsWithHooks() deployed a missing artifact") + } +} + +func capturePreviewStderr(t *testing.T) (*bytes.Buffer, func()) { + t.Helper() + + oldStderr := os.Stderr + readEnd, writeEnd, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe() error = %v", err) + } + os.Stderr = writeEnd + + var output bytes.Buffer + copyDone := make(chan struct{}) + go func() { + _, _ = io.Copy(&output, readEnd) + close(copyDone) + }() + + restore := func() { + _ = writeEnd.Close() + <-copyDone + _ = readEnd.Close() + os.Stderr = oldStderr + } + t.Cleanup(restore) + + return &output, restore +} diff --git a/apps/backend/cmd/preview/main.go b/apps/backend/cmd/preview/main.go index 94bbbb4790..051f885ea5 100644 --- a/apps/backend/cmd/preview/main.go +++ b/apps/backend/cmd/preview/main.go @@ -2,6 +2,7 @@ // // Usage: // +// preview package --artifact PATH [--skip-web-install] [--skip-web-build] // preview deploy --pr N --sha S [--repo owner/repo] [--port 38429] [--skip-description] // preview update-description --pr N --sha S --url URL [--repo owner/repo] // preview cleanup --pr N [--repo owner/repo] [--skip-description] @@ -34,6 +35,8 @@ func run() int { defer cancel() switch os.Args[1] { + case "package": + return runPackage(ctx, os.Args[2:]) case "deploy": return runDeploy(ctx, os.Args[2:]) case "update-description": @@ -56,6 +59,7 @@ func usage() { fmt.Fprintln(os.Stderr, `preview — deploy and manage PR preview environments on Sprites.dev Usage: + preview package --artifact PATH [--skip-web-install] [--skip-web-build] preview deploy --pr N --sha S [--repo owner/repo] [--port 38429] [--skip-description] preview update-description --pr N --sha S --url URL [--repo owner/repo] preview cleanup --pr N [--repo owner/repo] [--skip-description] diff --git a/apps/backend/cmd/preview/package.go b/apps/backend/cmd/preview/package.go new file mode 100644 index 0000000000..226744f7f8 --- /dev/null +++ b/apps/backend/cmd/preview/package.go @@ -0,0 +1,45 @@ +package main + +import ( + "context" + "flag" + "fmt" + "os" + "path/filepath" +) + +func runPackage(ctx context.Context, args []string) int { + return runPackageWithArtifactBuilder(ctx, args, buildPreviewArtifact) +} + +func runPackageWithArtifactBuilder(ctx context.Context, args []string, buildArtifact func(context.Context, string, string, bool, bool) error) int { + fs := flag.NewFlagSet("package", flag.ContinueOnError) + artifact := fs.String("artifact", "", "path for the preview bundle (required)") + skipWebInstall := fs.Bool("skip-web-install", false, "skip pnpm install (CI already ran it)") + skipWebBuild := fs.Bool("skip-web-build", false, "package existing web dist without running fork build scripts") + if err := fs.Parse(args); err != nil { + fmt.Fprintf(os.Stderr, "preview package: %v\n", err) + return 2 + } + if *artifact == "" { + fmt.Fprintln(os.Stderr, "preview package: --artifact is required") + return 2 + } + + binDir, err := os.MkdirTemp("", "kandev-preview-bin-*") + if err != nil { + fmt.Fprintf(os.Stderr, "preview package: mktemp: %v\n", err) + return 1 + } + defer func() { _ = os.RemoveAll(binDir) }() + + if err := os.MkdirAll(filepath.Dir(*artifact), 0o755); err != nil { + fmt.Fprintf(os.Stderr, "preview package: create artifact directory: %v\n", err) + return 1 + } + if err := buildArtifact(ctx, binDir, *artifact, *skipWebInstall, *skipWebBuild); err != nil { + fmt.Fprintf(os.Stderr, "preview package: %v\n", err) + return 1 + } + return 0 +} diff --git a/apps/backend/cmd/preview/package_test.go b/apps/backend/cmd/preview/package_test.go new file mode 100644 index 0000000000..c30661820d --- /dev/null +++ b/apps/backend/cmd/preview/package_test.go @@ -0,0 +1,63 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +func TestRunPackageRejectsMalformedFlags(t *testing.T) { + if got := runPackage(context.Background(), []string{"--unknown"}); got != 2 { + t.Fatalf("runPackage() = %d, want 2", got) + } +} + +func TestRunPackageReturnsErrorWhenArtifactDirectoryCannotBeCreated(t *testing.T) { + parentFile := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(parentFile, []byte("not a directory"), 0o600); err != nil { + t.Fatal(err) + } + + buildCalled := false + got := runPackageWithArtifactBuilder(context.Background(), []string{"--artifact", filepath.Join(parentFile, "preview.tar.gz")}, func(context.Context, string, string, bool, bool) error { + buildCalled = true + return nil + }) + if got != 1 { + t.Fatalf("runPackageWithArtifactBuilder() = %d, want 1", got) + } + if buildCalled { + t.Fatal("runPackageWithArtifactBuilder() called the artifact builder after directory setup failed") + } +} + +func TestRunPackageForwardsSkipWebBuildToArtifactBuilder(t *testing.T) { + artifact := filepath.Join(t.TempDir(), "preview", "preview.tar.gz") + var gotSkipWebBuild bool + buildCalled := false + + got := runPackageWithArtifactBuilder(context.Background(), []string{"--artifact", artifact, "--skip-web-build"}, func(_ context.Context, binDir, gotArtifact string, skipWebInstall, skipWebBuild bool) error { + buildCalled = true + if binDir == "" { + t.Fatal("artifact builder received an empty binary directory") + } + if gotArtifact != artifact { + t.Fatalf("artifact builder artifact = %q, want %q", gotArtifact, artifact) + } + if skipWebInstall { + t.Fatal("artifact builder skipWebInstall = true, want false") + } + gotSkipWebBuild = skipWebBuild + return nil + }) + if got != 0 { + t.Fatalf("runPackageWithArtifactBuilder() = %d, want 0", got) + } + if !buildCalled { + t.Fatal("runPackageWithArtifactBuilder() did not call the artifact builder") + } + if !gotSkipWebBuild { + t.Fatal("artifact builder skipWebBuild = false, want true") + } +} diff --git a/docs/decisions/2026-08-24-unified-fork-approval-label.md b/docs/decisions/2026-08-24-unified-fork-approval-label.md index 41713a8999..621dbd1c13 100644 --- a/docs/decisions/2026-08-24-unified-fork-approval-label.md +++ b/docs/decisions/2026-08-24-unified-fork-approval-label.md @@ -1,6 +1,6 @@ # ADR-2026-08-24-unified-fork-approval-label: Use One Maintainer Approval Label for Contributor PR Automation -**Status:** accepted +**Status:** superseded by 2026-09-07-isolate-fork-preview-builds **Date:** 2026-08-24 **Area:** infra, workflow, security @@ -42,11 +42,11 @@ context, and does not check out or execute contributor code in its secret-bearing worktree. Publication and pull request linking keep their existing separate jobs and minimum permissions. -Reuse the existing privileged fork preview path for `safe-to-review`. That path -checks out the contributor head and runs `go run ./cmd/preview deploy` with -`SPRITES_API_TOKEN` and `GITHUB_TOKEN`. Applying `safe-to-review` is an -explicit maintainer decision that accepts this trust boundary for the selected -contributor and current pull request. +At the time of this decision, reuse the privileged fork preview path for +`safe-to-review`. That path checked out the contributor head and ran +`go run ./cmd/preview deploy` with `SPRITES_API_TOKEN` and `GITHUB_TOKEN`. +The later isolation decision in +`2026-09-07-isolate-fork-preview-builds.md` supersedes this preview boundary. Keep approval labels durable until a maintainer removes them. Keep `generate-pr-walkthrough` as a same-repository operational rerun label, not as @@ -58,9 +58,9 @@ a contributor trust label. - Allowlisted contributors keep automatic access through the existing direct allowlist gates and receive one visible approval label. - A contributor push keeps the approval active until a maintainer removes it. -- A labeled contributor pull request can run privileged preview code. The - repository allowlist and maintainer label process must remain restricted to - contributors the maintainers trust. +- At the time, a labeled contributor pull request could run privileged preview + code. The repository allowlist and maintainer label process had to remain + restricted to contributors the maintainers trusted. - PR walkthrough generation gains fork support without allowing contributor files to replace its executable workflow assets. - Existing `safe-to-test` labels become inert after rollout and require a @@ -77,9 +77,10 @@ a contributor trust label. - Use only the label written by the allowlist bridge. Rejected because `GITHUB_TOKEN` label writes do not provide a reliable downstream workflow trigger. -- Redesign preview isolation before enabling the unified label. Rejected for - this change because the user selected the existing privileged preview path; - a preview isolation redesign remains a separate security initiative. +- Redesign preview isolation before enabling the unified label. Deferred at the + time because the user selected the existing privileged preview path. The + later isolation decision is recorded in + `2026-09-07-isolate-fork-preview-builds.md`. - Keep fork walkthroughs disabled. Rejected because the walkthrough is the requested explanation of contributor changes and can preserve a stronger trusted-input boundary than the existing privileged preview job. diff --git a/docs/decisions/2026-09-07-isolate-fork-preview-builds.md b/docs/decisions/2026-09-07-isolate-fork-preview-builds.md new file mode 100644 index 0000000000..6860142b63 --- /dev/null +++ b/docs/decisions/2026-09-07-isolate-fork-preview-builds.md @@ -0,0 +1,48 @@ +# ADR-2026-09-07-isolate-fork-preview-builds: Isolate contributor preview builds from deployment credentials + +**Status:** accepted +**Date:** 2026-09-07 +**Area:** infra, workflow, security + +## Context + +The preview workflow uses `pull_request_target` so a maintainer can approve a +fork pull request and deploy a preview. This event can access base repository +secrets. A contributor build must not run in the same process boundary as a +deployment credential. + +The previous path checked out the contributor head and ran the preview command +with `SPRITES_API_TOKEN`. A build hook could read or misuse that token. + +## Decision + +Build fork-controlled frontend input in a tokenless job. Build the preview +command from the immutable base workflow revision in a separate trusted job. +Use a fresh tokenless package job to run only that trusted command and create a +single archive with a SHA-256 file. + +Use a fresh deploy job that checks out only the immutable base workflow +revision. It must verify the archive digest and table before it passes the +archive to the trusted preview command. Only the fork deploy job receives +`SPRITES_API_TOKEN` in this flow. The preview command must fail closed when an +explicit archive is missing or is not a regular file. It may build on demand +only when the caller supplies no archive, such as the same-repository path. + +## Consequences + +- Contributor build commands cannot read the deployment credential from their + job or from the tokenless package process. +- The deploy job has a smaller trusted input surface and cannot silently build + a missing explicit archive. +- The workflow uses more jobs and short-lived artifacts. +- The deployed backend still contains contributor code and must run only in + the isolated preview service. + +## Alternatives Considered + +- Keep the contributor build and deployment in one secret-bearing job. Rejected + because build hooks could read the deployment credential. +- Remove only known credential names from the build environment. Rejected + because future credentials or alternate process paths could bypass the list. +- Use one runner for tokenless build and trusted deployment. Rejected because + a contributor process could remain alive across the job boundary. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index d4ae561599..f2e3e4da36 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -252,6 +252,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-09-06-opt-in-external-e2e-runners | [Opt in selected Linux CI jobs to external runners](2026-09-06-opt-in-external-e2e-runners.md) | accepted | infra, workflow, security | 2026-09-06 | | 2026-09-06-explicit-pr-auto-fix-outcomes | [Require Explicit Outcomes for PR Auto-Fix Attempts](2026-09-06-explicit-pr-auto-fix-outcomes.md) | proposed | backend, frontend, protocol, workflow, GitHub | 2026-09-06 | | 2026-09-05-bounded-progressive-storage-analysis | [Use bounded progressive storage analysis](2026-09-05-bounded-progressive-storage-analysis.md) | accepted | backend, frontend, protocol | 2026-09-05 | +| 2026-09-07-isolate-fork-preview-builds | [Isolate contributor preview builds from deployment credentials](2026-09-07-isolate-fork-preview-builds.md) | accepted | infra, workflow, security | 2026-09-07 | | 2026-09-05-script-capable-html-preview-isolation | [Capability-Free Runtime for Script-Capable HTML Preview](2026-09-05-script-capable-html-preview-isolation.md) | proposed | frontend, desktop, security | 2026-09-05 | | 2026-09-05-trusted-browser-html-preview | [Treat HTML Preview as Trusted Workspace Code](2026-09-05-trusted-browser-html-preview.md) | accepted | frontend, backend, agentctl, security | 2026-09-05 | | 2026-09-07-activate-managed-runtime-defaults | [Activate Shipped Managed Runtime Defaults](2026-09-07-activate-managed-runtime-defaults.md) | accepted | backend, frontend, protocol | 2026-09-07 | diff --git a/docs/specs/ci/requirements/unified-contributor-pr-automation.md b/docs/specs/ci/requirements/unified-contributor-pr-automation.md index 4fbb470430..69d567a3a0 100644 --- a/docs/specs/ci/requirements/unified-contributor-pr-automation.md +++ b/docs/specs/ci/requirements/unified-contributor-pr-automation.md @@ -25,9 +25,9 @@ their current workflow behavior. - **Approval label:** The maintainer-applied `safe-to-review` label. - **Direct allowlist:** An existing repository variable that names trusted contributor logins. Direct allowlists remain independent authorization paths. -- **Privileged preview path:** The existing fork preview job that checks out the - contributor head and runs `go run ./cmd/preview deploy` with its current - deployment credentials. +- **Privileged preview path:** The preview pipeline that builds contributor + input without deployment credentials, packages a validated archive, and + deploys it from trusted base code. ## Requirements @@ -102,10 +102,15 @@ review or preview implementation. `safe-to-review`, the existing Claude and OpenCode fork review workflows shall retain their current provider-specific event and permission contracts. - **AC-CI-PR-REVIEW-004.2:** When a contributor pull request is authorized by - `safe-to-review`, the preview workflow shall reuse its existing privileged - fork path, including its current head checkout and deployment credentials. + `safe-to-review`, the preview workflow shall build contributor input in + tokenless jobs and deploy only a validated archive with trusted base code + and deployment credentials. It shall not check out or execute contributor + source in the secret-bearing deploy job. - **AC-CI-PR-REVIEW-004.3:** Existing direct review and preview allowlists shall remain valid as independent authorization sources. +- **AC-CI-PR-REVIEW-004.4:** When the preview deploy command receives an + explicit archive path that is missing or not a regular file, it shall fail + before it starts build commands or uses deployment credentials. ### REQ-CI-PR-FAIL-005: Fail closed and preserve unrelated behavior @@ -125,8 +130,7 @@ permissions or changing same-repository automation. ## Out of scope -- Redesigning the preview executor or isolating its existing privileged fork - deployment path. +- Changing the preview service runtime or archive contents. - Changing review provider prompts, models, or follow-up review policies. - Removing `CLAUDE_REVIEW_ALLOWLIST`, `OPENCODE_REVIEW_ALLOWLIST`, or `PREVIEW_ENV_ALLOWLIST`. diff --git a/docs/specs/ci/system-design/unified-contributor-pr-automation.md b/docs/specs/ci/system-design/unified-contributor-pr-automation.md index 5c5eea5041..513e882e76 100644 --- a/docs/specs/ci/system-design/unified-contributor-pr-automation.md +++ b/docs/specs/ci/system-design/unified-contributor-pr-automation.md @@ -18,9 +18,10 @@ Actions in the `kdlbs/kandev` repository. It covers fork pull request authorization, event routing, and the boundaries between review, preview, walkthrough generation, publication, and pull request linking. -The design does not change the review providers, the preview command, the -walkthrough skill, or the application. Those components keep their existing -contracts after the shared authorization gate changes. +The design does not change the review providers, the walkthrough skill, or the +application. It defines the preview command's archive contract for the staged +fork workflow. Those components keep their existing contracts after the +shared authorization gate changes. ## Requirement mapping @@ -38,7 +39,7 @@ contracts after the shared authorization gate changes. | --- | --- | | `.github/workflows/claude-code-review.yml` | Adds the approval label for allowlisted contributors and runs the existing Claude fork review path. | | `.github/workflows/opencode-code-review.yml` | Runs the existing OpenCode fork review path with durable `safe-to-review` authorization. | -| `.github/workflows/preview-env.yml` | Runs the existing privileged fork preview path. | +| `.github/workflows/preview-env.yml` | Builds fork input without deployment credentials, packages it with trusted code, and deploys the validated archive. | | `.github/workflows/pr-walkthrough.yml` | Generates, publishes, and links walkthroughs for authorized fork and same-repository pull requests. | | `.github/workflows/pr-walkthrough-reconcile.yml` | Repairs an existing stale walkthrough callout after an authorized pull request description edit. | | `.github/scripts/*workflow_contract_test.py` | Protects job gates, labels, event filters, permissions, and trusted input provenance. | @@ -138,6 +139,21 @@ The public object key remains 5. Publication and linking run in separate jobs with their existing minimal permissions and credentials. +### Preview flow + +1. The fork gate evaluates `safe-to-review` and the direct preview allowlist. +2. A trusted runner builds the preview command from `github.workflow_sha`. +3. A separate tokenless runner fetches the exact contributor head and builds + the frontend. It uploads source and static assets as a run-scoped artifact. +4. A fresh tokenless runner downloads both inputs and invokes only the trusted + preview command. The command builds the backend and writes one archive. +5. The package job writes a SHA-256 file. A fresh deploy job checks the digest + and archive table, then runs trusted base code with `--artifact` and the + Sprites credential. +6. The deploy command fails closed when an explicit archive is missing or is + not a regular file. Build-on-demand remains available only when no archive + is supplied, such as the same-repository path. + ## Failure and recovery - Missing, malformed, or non-matching allowlists fail closed. @@ -155,6 +171,9 @@ The public object key remains - A description-write race causes a fresh merge and bounded retry. A writer reports failure if its post-write readback does not contain its expected marker state. +- A missing or non-regular explicit preview archive fails before build + commands or deployment credentials are used. A missing archive cannot cause + the deploy command to build contributor source. - Contract tests fail when the old label appears in an active authorization expression or when a privileged job loses its trust or permission boundary. @@ -182,10 +201,13 @@ trigger. credentials or pull request write permission. - The review workflows retain their existing read-only agent policies and advisory posting boundaries. -- The preview workflow intentionally reuses the existing privileged fork path. - That path checks out the contributor head and runs `go run ./cmd/preview - deploy` with `SPRITES_API_TOKEN` and `GITHUB_TOKEN`. `safe-to-review` is an - explicit maintainer trust decision for that execution. +- The preview workflow builds contributor input in a tokenless job. A separate + tokenless package job runs only the trusted preview command from + `github.workflow_sha` with a restricted environment. +- The deploy job checks out only `github.workflow_sha`, verifies the archive + digest and table, and passes the archive to the trusted command. Only the + fork deploy job receives `SPRITES_API_TOKEN` in this flow; it does not receive + contributor source or run contributor lifecycle scripts. - Publication receives only the bucket-scoped R2 credentials needed for its upload. Linking receives only the existing pull request write permission and trusted helper. @@ -207,6 +229,7 @@ trigger. ## Related decisions - [Use one maintainer approval label for contributor PR automation](../../../decisions/2026-08-24-unified-fork-approval-label.md) +- [Isolate contributor preview builds from deployment credentials](../../../decisions/2026-09-07-isolate-fork-preview-builds.md) - [Persist Fork Approval Labels Across Pushes](../../../decisions/2026-08-22-persistent-fork-approval-labels.md) - [Use the Claude Allowlist as a Trusted Preview Gate](../../../decisions/2026-08-07-claude-allowlist-label-bridge.md) - [Use a Filesystem Contract for PR Walkthrough Runners](../../../decisions/2026-08-22-pr-walkthrough-filesystem-runner.md)