diff --git a/.github/workflows/hourly-product-development.yml b/.github/workflows/hourly-product-development.yml index 324333371..06b96d37d 100644 --- a/.github/workflows/hourly-product-development.yml +++ b/.github/workflows/hourly-product-development.yml @@ -838,9 +838,17 @@ jobs: GH_TOKEN: ${{ steps.maintainer_app.outputs.token }} run: | set -euo pipefail + umask 077 title="$(cat "$RUNNER_TEMP/pr-title.txt")" body_file="$RUNNER_TEMP/pr-body.md" branch="nim-agent/product-dev-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" + expected_base="${{ needs.propose_product_increment.outputs.base_sha }}" + repo_owner="${GITHUB_REPOSITORY%%/*}" + + if ! [[ "$expected_base" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::proposal_base_invalid" + exit 1 + fi git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" @@ -848,37 +856,130 @@ jobs: git -c core.hooksPath=/dev/null commit -m "$title" gh auth setup-git - set +e - git ls-remote --exit-code --heads origin "refs/heads/${branch}" >/dev/null 2>&1 - remote_status=$? - set -e - case "$remote_status" in - 0) - echo "::error::proposal_branch_already_exists" - exit 1 - ;; - 2) ;; - *) - echo "::error::proposal_branch_inventory_unavailable" - exit 1 - ;; - esac + proposal_head="$(git rev-parse HEAD)" + if ! [[ "$proposal_head" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::proposal_head_invalid" + exit 1 + fi + if ! git push --force-with-lease="refs/heads/${branch}:" origin "HEAD:refs/heads/${branch}"; then + echo "::error::proposal_branch_create_lease_rejected" + exit 1 + fi cleanup_remote_branch() { - git push origin --delete "$branch" >/dev/null 2>&1 || true + git push --force-with-lease="refs/heads/${branch}:${proposal_head}" origin ":refs/heads/${branch}" >/dev/null 2>&1 || true } trap cleanup_remote_branch ERR - git push origin "HEAD:refs/heads/${branch}" - pr_url="$( - gh pr create \ - --repo "$GITHUB_REPOSITORY" \ - --base "$DEFAULT_BRANCH" \ - --head "$branch" \ - --title "$title" \ - --body-file "$body_file" - )" + + marker_nonce="$(od -An -N32 -tx1 /dev/urandom | tr -d '[:space:]')" + if ! [[ "$marker_nonce" =~ ^[0-9a-f]{64}$ ]]; then + echo "::error::publication_marker_generation_failed" + false + fi + publication_marker="noema-publication-${marker_nonce}" + pr_request_file="$RUNNER_TEMP/pr-create.json" + jq -n \ + --arg title "$title" \ + --arg head "$branch" \ + --arg base "$DEFAULT_BRANCH" \ + --rawfile body "$body_file" \ + --arg marker "$publication_marker" \ + '{title: $title, head: $head, base: $base, body: ($body + "\n\n")}' \ + >"$pr_request_file" + chmod 0600 "$pr_request_file" + + pr_number="" + recover_created_pr_number() { + local candidates candidate candidate_json candidate_head candidate_base candidate_body recovered="" + if ! candidates="$( + gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/pulls?state=open&head=${repo_owner}:${branch}&per_page=100" \ + --jq '.[].number' + )"; then + return 1 + fi + while IFS= read -r candidate; do + [ -n "$candidate" ] || continue + if ! [[ "$candidate" =~ ^[1-9][0-9]*$ ]]; then + return 1 + fi + if ! candidate_json="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${candidate}")"; then + return 1 + fi + candidate_head="$(jq -r '.head.sha // empty' <<<"$candidate_json")" + candidate_base="$(jq -r '.base.sha // empty' <<<"$candidate_json")" + candidate_body="$(jq -r '.body // empty' <<<"$candidate_json")" + if [ "$candidate_head" = "$proposal_head" ] \ + && [ "$candidate_base" = "$expected_base" ] \ + && grep -Fq -- "$publication_marker" <<<"$candidate_body"; then + if [ -n "$recovered" ]; then + return 1 + fi + recovered="$candidate" + fi + done <<<"$candidates" + [ -n "$recovered" ] || return 1 + printf '%s\n' "$recovered" + } + + cleanup_created_pr() { + trap - ERR + pr_number="$(recover_created_pr_number 2>/dev/null || true)" + if [[ "${pr_number:-}" =~ ^[1-9][0-9]*$ ]]; then + gh api --method PATCH "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}" -f state=closed >/dev/null 2>&1 || true + fi + cleanup_remote_branch + } + trap cleanup_created_pr ERR + + if ! created_pr_json="$( + gh api --method POST "repos/${GITHUB_REPOSITORY}/pulls" --input "$pr_request_file" + )"; then + echo "::error::created_pull_request_request_failed" + false + fi + pr_number="$(jq -r '.number // empty' <<<"$created_pr_json")" + if ! [[ "$pr_number" =~ ^[1-9][0-9]*$ ]]; then + echo "::error::created_pull_request_number_invalid" + false + fi + pr_url="https://github.com/${GITHUB_REPOSITORY}/pull/${pr_number}" + + if ! created_pr_json="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}")"; then + echo "::error::created_pull_request_identity_unavailable" + false + fi + live_pr_head="$(jq -r '.head.sha // empty' <<<"$created_pr_json")" + live_pr_base="$(jq -r '.base.sha // empty' <<<"$created_pr_json")" + if ! [[ "$live_pr_head" =~ ^[0-9a-f]{40}$ ]] \ + || ! [[ "$live_pr_base" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::created_pull_request_identity_invalid" + false + fi + if [ "$live_pr_head" != "$proposal_head" ]; then + echo "::error::created_pull_request_head_mismatch" + false + fi + if [ "$live_pr_base" != "$expected_base" ]; then + echo "::error::created_pull_request_base_mismatch" + false + fi + + if ! open_pr_numbers="$( + gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" \ + --jq '.[].number' + )"; then + echo "::error::created_pull_request_queue_inventory_unavailable" + false + fi + if [ "$open_pr_numbers" != "$pr_number" ]; then + echo "::error::created_pull_request_queue_conflict" + false + fi + trap - ERR { echo "Opened bounded pull request: $pr_url" echo "hourly-commercial-readiness owns review, repair, exact-head revalidation, and merge." - } >>"$GITHUB_STEP_SUMMARY" + } >>"$GITHUB_STEP_SUMMARY" \ No newline at end of file diff --git a/docs/doctoring/atomic-product-publisher-lease.md b/docs/doctoring/atomic-product-publisher-lease.md new file mode 100644 index 000000000..fa2002aa4 --- /dev/null +++ b/docs/doctoring/atomic-product-publisher-lease.md @@ -0,0 +1,75 @@ +# Atomic product-publisher ref and pull-request lease + +## Status and scope + +Reviewed on 2026-08-16 against protected `main` `28af0b1c2e76d066a5d41ef1da56157209c89431`. This record applies only to the credential-bearing `publish_product_increment` stage in `.github/workflows/hourly-product-development.yml`. It does not grant review, merge, release, deployment, or licensing authority, and it does not change the NVIDIA NIM proposer/verifier trust split already present on protected main. + +This successor rebuilds only the unique atomic publisher behavior from #378 on the current protected lineage after #373 advanced `main`. The #378 branch and its checks/reviews remain predecessor evidence; no CI, review, scanner, or coverage result transfers to this successor. + +## Problem and RED condition + +Protected `main` first inventories a generated branch with `git ls-remote`, then performs an unconditional branch push. On later failure it deletes the branch by name without proving that the remote ref still equals the proposal commit. That is a check-then-act race: another actor may create the same ref after inventory, or may advance/recreate a publisher-created ref before cleanup. + +`test/hourly-product-development-publisher-lease.test.ts` is the executable RED contract. Against the protected-main implementation it rejects the unguarded push/delete sequence and requires: + +- expected-absence branch creation in the same Git ref update; +- exact-proposal-head cleanup rather than deletion by name; +- machine-readable REST pull-request creation; +- cleanup armed before a possibly successful create request can lose its response; +- server-side head/base identity revalidation; and +- fully paginated post-create open-PR inventory before publication is accepted. + +## Decision + +### Explicit expected-absence ref creation + +The publisher captures the exact local proposal commit and uses Git's explicit lease form: + +```sh +proposal_head="$(git rev-parse HEAD)" +git push --force-with-lease="refs/heads/${branch}:" \ + origin "HEAD:refs/heads/${branch}" +``` + +An empty explicit expectation means the mutation is accepted only while the destination ref is absent. The preliminary `git ls-remote` inventory is removed because it is observation, not write authority. + +### Exact-head cleanup + +Cleanup is conditioned on the branch still identifying the exact proposal commit created by this run: + +```sh +cleanup_remote_branch() { + git push --force-with-lease="refs/heads/${branch}:${proposal_head}" \ + origin ":refs/heads/${branch}" >/dev/null 2>&1 || true +} +``` + +If another actor advances or replaces the ref, cleanup fails harmlessly rather than deleting foreign state. + +### Recoverable pull-request identity + +The publisher creates a pull request through GitHub's REST endpoint with a 256-bit correlation marker in the bounded body. A cleanup trap is armed before `POST /pulls`. If the client response is lost or malformed after possible server-side success, the publisher performs a paginated head-scoped query and accepts exactly one candidate only when its `head.sha`, `base.sha`, and hidden marker all match this run. Cleanup does not trust a previously returned numeric PR identifier: immediately before any close it discards that identifier, repeats the marker/head/base recovery, and closes only the single recovered positive integer pull-request number. If recovery is unavailable, ambiguous, or no longer matches the exact proposal identity, the PR is left open for manual investigation rather than being closed by stale identity. + +After creation, the publisher re-reads that exact pull request and requires `head.sha == proposal_head` and `base.sha == expected_base`. It then paginates the complete open-PR queue and accepts publication only when the created pull request is the sole open PR. Missing, malformed, ambiguous, or unavailable evidence remains failure. + +## Authority and rollback boundaries + +The proposer may use `NVIDIA_NIM_API_KEY` but has no shell execution authority. A separate uncredentialed job executes `npm run release:verify`; the credential-bearing publisher reconstructs and publishes only the already verified immutable proposal. The Maintainer App token never becomes model/verifier authority. This change does not weaken the central Security Scan, configured coverage, package, SBOM/provenance, review, protected-base, or release gates. + +Rollback is source rollback of this bounded publisher change. No force-push, destructive rebase, branch-protection bypass, self-approval, repair workflow, or alternate credential is required. + +## Source-supported rationale + +Git 2.55.0 documents `--force-with-lease=:` as conditioning a ref update on the named ref still having the explicitly supplied current value. This is the relevant compare-and-swap primitive for both expected-absence creation and exact-head cleanup. + +GitHub's current pull-request REST API provides structured creation, retrieval, head filtering, and pagination. These primitives avoid parsing human-oriented CLI output and allow the publisher to rebind publication to exact server-observed commit identities. + +NIST SSDF 1.1 requires organizations to protect software and development environments from unauthorized access and tampering and to maintain provenance/integrity controls across the software lifecycle. The repository therefore keeps model generation, executable verification, publication credentials, checks, review, merge, release, and deployment as separate authorities. + +## References (APA 7th) + +Git Project. (2026). *git-push documentation (Version 2.55.0).* https://git-scm.com/docs/git-push + +GitHub, Inc. (2026). *REST API endpoints for pull requests.* https://docs.github.com/en/rest/pulls/pulls + +Souppaya, M., Scarfone, K., & Dodson, D. (2022). *Secure software development framework (SSDF) version 1.1: Recommendations for mitigating the risk of software vulnerabilities* (NIST Special Publication 800-218). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218 diff --git a/test/hourly-product-development-publisher-lease.test.ts b/test/hourly-product-development-publisher-lease.test.ts new file mode 100644 index 000000000..5855bfce4 --- /dev/null +++ b/test/hourly-product-development-publisher-lease.test.ts @@ -0,0 +1,118 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const workflowPath = ".github/workflows/hourly-product-development.yml"; + +/** Return the trusted publication job so lease assertions cannot match an untrusted stage. */ +function publisherJob(): string { + const workflow = readFileSync(workflowPath, "utf8"); + const start = workflow.indexOf(" publish_product_increment:"); + expect(start).toBeGreaterThan(-1); + return workflow.slice(start); +} + +describe("hourly product-development publisher ref lease", () => { + it("atomically creates a proposal ref only when the remote ref is absent", () => { + const publisher = publisherJob(); + const push = 'git push --force-with-lease="refs/heads/${branch}:" origin "HEAD:refs/heads/${branch}"'; + + expect(publisher).toContain(push); + expect(publisher).not.toContain('git push origin "HEAD:refs/heads/${branch}"'); + expect(publisher).not.toContain("git ls-remote --exit-code --heads origin"); + }); + + it("never deletes a raced or subsequently changed proposal ref during cleanup", () => { + const publisher = publisherJob(); + const headCapture = 'proposal_head="$(git rev-parse HEAD)"'; + const createPush = 'git push --force-with-lease="refs/heads/${branch}:" origin "HEAD:refs/heads/${branch}"'; + const cleanupLease = 'git push --force-with-lease="refs/heads/${branch}:${proposal_head}" origin ":refs/heads/${branch}"'; + const trap = "trap cleanup_remote_branch ERR"; + + const headIndex = publisher.indexOf(headCapture); + const pushIndex = publisher.indexOf(createPush); + const trapIndex = publisher.indexOf(trap); + + expect(headIndex).toBeGreaterThan(-1); + expect(pushIndex).toBeGreaterThan(headIndex); + expect(trapIndex).toBeGreaterThan(pushIndex); + expect(publisher).toContain(cleanupLease); + expect(publisher).not.toContain('git push origin --delete "$branch"'); + }); + + it("arms recoverable numeric cleanup before machine-readable PR creation", () => { + const publisher = publisherJob(); + const marker = "publication_marker="; + const createPr = 'gh api --method POST "repos/${GITHUB_REPOSITORY}/pulls" --input "$pr_request_file"'; + const parseNumber = 'pr_number="$(jq -r'; + const recoverNumber = "recover_created_pr_number"; + const installCreatedPrCleanup = "trap cleanup_created_pr ERR"; + const closeCreatedPr = 'gh api --method PATCH "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}" -f state=closed'; + const readCreatedPr = 'gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}"'; + const headGuard = '[ "$live_pr_head" != "$proposal_head" ]'; + const baseGuard = '[ "$live_pr_base" != "$expected_base" ]'; + const clearTrap = "trap - ERR"; + + const markerIndex = publisher.indexOf(marker); + const cleanupTrapIndex = publisher.indexOf(installCreatedPrCleanup, markerIndex); + const createPrIndex = publisher.indexOf(createPr, cleanupTrapIndex); + const parseNumberIndex = publisher.indexOf(parseNumber, createPrIndex); + const readCreatedPrIndex = publisher.indexOf(readCreatedPr, parseNumberIndex); + const headGuardIndex = publisher.indexOf(headGuard, readCreatedPrIndex); + const baseGuardIndex = publisher.indexOf(baseGuard, readCreatedPrIndex); + const clearTrapIndex = publisher.indexOf( + clearTrap, + Math.max(headGuardIndex, baseGuardIndex), + ); + + expect(markerIndex).toBeGreaterThan(-1); + expect(cleanupTrapIndex).toBeGreaterThan(markerIndex); + expect(createPrIndex).toBeGreaterThan(cleanupTrapIndex); + expect(parseNumberIndex).toBeGreaterThan(createPrIndex); + expect(readCreatedPrIndex).toBeGreaterThan(parseNumberIndex); + expect(headGuardIndex).toBeGreaterThan(readCreatedPrIndex); + expect(baseGuardIndex).toBeGreaterThan(readCreatedPrIndex); + expect(clearTrapIndex).toBeGreaterThan(Math.max(headGuardIndex, baseGuardIndex)); + expect(publisher).toContain(recoverNumber); + expect(publisher).toContain("pulls?state=open&head="); + expect(publisher).toContain(closeCreatedPr); + expect(publisher).not.toContain("gh pr create"); + expect(publisher).not.toContain('gh pr close "$pr_url"'); + }); + + it("revalidates a known PR number before cleanup can close it", () => { + const publisher = publisherJob(); + const cleanupStart = publisher.indexOf("cleanup_created_pr() {"); + const cleanupEnd = publisher.indexOf("trap cleanup_created_pr ERR", cleanupStart); + const cleanup = publisher.slice(cleanupStart, cleanupEnd); + const recover = 'pr_number="$(recover_created_pr_number 2>/dev/null || true)"'; + const closeCreatedPr = 'gh api --method PATCH "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}" -f state=closed'; + + expect(cleanupStart).toBeGreaterThan(-1); + expect(cleanupEnd).toBeGreaterThan(cleanupStart); + expect(cleanup.indexOf(recover)).toBeGreaterThan(-1); + expect(cleanup.indexOf(closeCreatedPr)).toBeGreaterThan(cleanup.indexOf(recover)); + expect(cleanup).not.toContain('if ! [[ "${pr_number:-}" =~ ^[1-9][0-9]*$ ]]'); + }); + + it("rechecks the fully paginated open-PR queue after creation before accepting publication", () => { + const publisher = publisherJob(); + const baseGuard = '[ "$live_pr_base" != "$expected_base" ]'; + const paginatedInventory = "gh api --paginate"; + const openPullsEndpoint = 'repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100'; + const numberProjection = "--jq '.[].number'"; + const queueConflict = "created_pull_request_queue_conflict"; + const clearTrap = "trap - ERR"; + + const baseGuardIndex = publisher.indexOf(baseGuard); + const inventoryIndex = publisher.indexOf(paginatedInventory, baseGuardIndex); + const queueConflictIndex = publisher.indexOf(queueConflict, inventoryIndex); + const clearTrapIndex = publisher.indexOf(clearTrap, queueConflictIndex); + + expect(baseGuardIndex).toBeGreaterThan(-1); + expect(inventoryIndex).toBeGreaterThan(baseGuardIndex); + expect(publisher).toContain(openPullsEndpoint); + expect(publisher).toContain(numberProjection); + expect(queueConflictIndex).toBeGreaterThan(inventoryIndex); + expect(clearTrapIndex).toBeGreaterThan(queueConflictIndex); + }); +}); diff --git a/test/hourly-product-development-workflow.test.ts b/test/hourly-product-development-workflow.test.ts index 25f9e9ee6..ddf43dbd8 100644 --- a/test/hourly-product-development-workflow.test.ts +++ b/test/hourly-product-development-workflow.test.ts @@ -297,6 +297,8 @@ describe("hourly NVIDIA NIM OpenCode product-development workflow", () => { it("cleans failed candidates, verifies twice, and packages at most one bounded pull request", () => { const workflow = workflowText(); + const pullRequestCreate = + 'gh api --method POST "repos/${GITHUB_REPOSITORY}/pulls" --input "$pr_request_file"'; expect(workflow).toContain( 'timeout --kill-after="${OPENCODE_KILL_GRACE_SECONDS}s" "${OPENCODE_RUN_TIMEOUT_SECONDS}s"', @@ -313,9 +315,9 @@ describe("hourly NVIDIA NIM OpenCode product-development workflow", () => { expect(workflow).toContain( 'branch="nim-agent/product-dev-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}"', ); - expect(workflow.match(/gh pr create/g)).toHaveLength(1); - expect(workflow).toContain('--base "$DEFAULT_BRANCH"'); - expect(workflow).toContain('--head "$branch"'); + expect(workflow.match(/gh api --method POST "repos\/\$\{GITHUB_REPOSITORY\}\/pulls"/g)).toHaveLength(1); + expect(workflow).toContain('--arg base "$DEFAULT_BRANCH"'); + expect(workflow).toContain('--arg head "$branch"'); expect(workflow).toContain("PR_MESSAGE.md"); expect(workflow).toContain("git status --porcelain"); expect(workflow).toContain( @@ -327,7 +329,7 @@ describe("hourly NVIDIA NIM OpenCode product-development workflow", () => { expect(workflow).toContain("core.hooksPath=/dev/null"); expect(workflow).toContain("cleanup_remote_branch"); expect(workflow.indexOf("npm run release:verify")).toBeLessThan( - workflow.indexOf("gh pr create"), + workflow.indexOf(pullRequestCreate), ); expect(workflow).not.toMatch(/gh pr merge|gh release create|wrangler deploy/); }); @@ -338,8 +340,12 @@ describe("hourly NVIDIA NIM OpenCode product-development workflow", () => { const revalidationIndex = publisher.indexOf( "Revalidate queue and default-branch head", ); - const pushIndex = publisher.indexOf("git push origin"); - const createIndex = publisher.indexOf("gh pr create"); + const pushIndex = publisher.indexOf( + 'git push --force-with-lease="refs/heads/${branch}:" origin "HEAD:refs/heads/${branch}"', + ); + const createIndex = publisher.indexOf( + 'gh api --method POST "repos/${GITHUB_REPOSITORY}/pulls" --input "$pr_request_file"', + ); expect(workflow).toContain("id: base"); expect(workflow).toContain("base_sha=$(git rev-parse HEAD)"); @@ -354,7 +360,10 @@ describe("hourly NVIDIA NIM OpenCode product-development workflow", () => { ); expect(workflow).toContain("open_pull_request_after_generation"); expect(workflow).toContain("base_branch_advanced"); + expect(workflow).toContain("proposal_branch_create_lease_rejected"); expect(revalidationIndex).toBeGreaterThan(-1); + expect(pushIndex).toBeGreaterThan(-1); + expect(createIndex).toBeGreaterThan(-1); expect(revalidationIndex).toBeLessThan(pushIndex); expect(revalidationIndex).toBeLessThan(createIndex); });