Skip to content

fix(security): make product publisher branch creation atomic - #80

Draft
seonghobae wants to merge 46 commits into
fix/nanoid-cve-2026-67213from
fix/atomic-product-publisher-lease
Draft

fix(security): make product publisher branch creation atomic#80
seonghobae wants to merge 46 commits into
fix/nanoid-cve-2026-67213from
fix/atomic-product-publisher-lease

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Hardens the trusted hourly-product-development scheduler and its credential-bearing proposal publisher without adding repair automation, synthetic approval, merge authority, release authority, or deployment authority.

This PR owns the focused scheduler/publisher implementation. Repository-wide PRD/TRD/Architecture/ADR/UML/ERD/Test/Operability/Traceability remains canonical on PR #71; #71 records this work as Proposed until protected integration and operational proof.

Exact source identity

Scheduler RCA, feasibility, and continuation contract

The product-development workflow begins by reading AGENTS.md. That repository-consumed preflight requires:

exact evidence
→ reproduce/isolate
→ falsifiable root-cause hypothesis
→ materially distinct remediation candidates
→ empirical feasibility gate
→ smallest safe action
→ observable proof
→ queue top

Each candidate is classified execute_now, defer_until_trigger, read_only_dependency, external_only, or reject. Authority, capability, exact target, policy, dependency order, writer lease, reversibility, remaining budget, blast radius and a test oracle must be proved before mutation. Failed/no-op remedies become new evidence; three materially distinct failed hypotheses require architecture/contract reassessment rather than another symptom patch.

Work-conserving deliverable handoff

The scheduler now explicitly refuses to treat an intermediate artifact as invocation completion:

prompt update → repository-consumed policy and executable verification
RCA → feasible action
design → implementation
test → production code
documentation assessment → canonical repository files
local changes → intentional commit → pull request
pull request → exact-head checks → review remediation → protected merge
protected merge → protected-main operational acceptance → queue top

A blocked handoff defers only that lane and rotates to another non-conflicting action. Documentation repair must be followed by the highest-value safe non-documentation work when practical budget and the writer lease permit it. Termination requires a mandatory double fresh exit sweep or genuine practical invocation-budget exhaustion. A user-visible report is never completion. CHANGELOG.md now records this contract explicitly rather than leaving it only in scheduler policy and tests.

Stacked Security Scan trigger boundary

The central .github Security Scan currently selects supported pull-request base branches. Because #80 is stacked on the feature branch owned by #76, absence of a central Security Scan on this exact head is an event-selection boundary, not scanner success.

Required treatment:

Atomic publisher boundary

The publisher protects both mutable GitHub objects it creates:

  • proposal branch creation uses an explicit expected-absence --force-with-lease;
  • failed-run branch cleanup is bound to the exact proposal_head;
  • PR creation uses the machine-readable REST endpoint;
  • a 256-bit hidden publication marker and numeric cleanup trap are armed before creation;
  • a lost or malformed create response recovers exactly one PR only when repository, branch, marker, server-side head SHA and exact base SHA all match;
  • cleanup closes only the recovered numeric PR and then attempts exact-head branch deletion;
  • the created PR is re-read and server-side head.sha/base.sha must match verified identities;
  • the complete open-PR queue is paginated after creation and must contain only this run's PR before cleanup is disarmed.

Test-first lineage

Historical publisher/RCA lineage remains audit evidence, not current-head acceptance:

  • 97b9a2f5f604f9885c0c32e5204f6b2f9ccfed13: RED realistic-remediation contract.
  • 8649f6fe134f210b1e606b36cc60c8afaeb92624: remediation guidance GREEN.
  • a2da363eb40dfe4723475605a67a84c93bb16bcb: RED recoverable PR identity/full-queue contracts.
  • f60f4bb001e6219b2588312e1665f1e5dc806d01: publisher identity/cleanup contracts GREEN.
  • 8ae32bbaa20d8da83b3d6ceff3300715ebd28667: RED RCA/feasibility contract.
  • 13ea7eeb0dab8b33d70cb4bb6823e22484458ff9: RCA-to-action contract GREEN.
  • abd973a299ceec76148041a7cebe8a3ead32c20b: RED stacked-Security-Scan trigger contract.
  • 9103c46b72485fe83b63ac84c2d2bf51d85a5ce3: corrected event-selection contract.
  • 73bdcb646e9e14e811f5fa5b1b465f28a52ca161: removes competing repository-wide documentation ownership; fix(security): bind workflow source and document MSA boundaries #71 remains canonical.
  • 6e73aa73d06ecbabf335388d3dacc58f66791a76: RED contract requiring deliverable handoff, post-documentation continuation, double exit sweep and report-not-completion semantics.
  • 9a19f216ec133c45671e906f01672150afa3e2ab: repository-consumed AGENTS.md implementation.
  • 01f25c5f0a2d92488aaf301805004667a102b675: RED contract requiring the same continuation semantics to be recorded in CHANGELOG.md.
  • a937c59f14098de03a0b83e99359c8ac3579df69: current exact head; CHANGELOG.md implements the required handoff/double-sweep/intermediate-state record. The exact one-commit comparison from RED to GREEN changes only CHANGELOG.md with the intended insertion.

Exact-head evidence state

Freshly revalidated for exact head a937c59f14098de03a0b83e99359c8ac3579df69:

  • ci run 31314266572: terminal success;
  • reviewer-ci run 31314266576: terminal success;
  • central Security Scan: absent because the current base is a feature branch; this remains defer_until_trigger, not passing evidence;
  • formal GitHub reviews: none;
  • inline review threads: none;
  • eligible independent non-author APPROVED: none.

The PR remains Draft because terminal application/reviewer CI cannot substitute for the absent stack-ineligible Security Scan, unintegrated predecessor, enforceable governance, or qualifying approval. No predecessor-head CI/review evidence transfers to a future refreshed exact head.

Dependency and governance boundary

Do not merge #80 until:

  1. fix(security): update transitive nanoid for CVE-2026-67213 #76 integrates in dependency order;
  2. fix(security): make product publisher branch creation atomic #80 is refreshed onto the correct live protected base;
  3. the central Security Scan becomes eligible and reaches terminal success on the refreshed exact head;
  4. all other actual required exact-head CI/security/coverage/package/provenance gates succeed;
  5. current review findings and threads are addressed;
  6. applicable live ruleset and qualifying formal-review requirements are satisfied;
  7. protected-main operational acceptance verifies scheduler/publisher behavior after merge.

Issue #27 owns enforceable main governance. Issue #29 owns Reviewer/Maintainer App provisioning. Previously disproven opencode-agent and cwl-noema-review reviewer routes are not spammed or treated as approval paths.

No .github/workflows/repair-*, self-modifying Action, branch-patching contents:write, protection bypass, synthetic approval, merge, release, or deployment authority is introduced.

Related: #71, #76, #78, #27, #29

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b6c3b582-2e90-4ef6-b4bb-3b99a414d509

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae changed the base branch from main to fix/nanoid-cve-2026-67213 August 8, 2026 09:14

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please perform a substantive security review of exact head ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd. Verify the expected-absence --force-with-lease=<ref>: creation boundary, exact-proposal-head cleanup lease, trap ordering, concurrent actor behavior, least privilege, and the executable regression/doctoring contract. The live stacked base has advanced to #76 head e0106ce16b7b8b493f46bf075ec5baf58762bd95, so predecessor synthetic-merge/check evidence is historical only. Do not treat this request or a model verdict as approval or merge authority.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd'
base='e0106ce16b7b8b493f46bf075ec5baf58762bd95'

echo '== exact head =='
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$head"
echo
echo '== changed files at exact head relative to first parent =='
git diff-tree --no-commit-id --name-status -r "$head"
echo
echo '== diff =='
git diff --find-renames --find-copies "${head}^" "$head" -- .github docs test tests 2>/dev/null || true
echo
echo '== workflow and relevant test/document candidates =='
fd -t f . | rg '(^|/)(.*product.*publisher.*|.*hourly.*product.*|.*atomic.*lease.*|.*publisher.*\.(test|spec)\.[^/]+|.*doctoring.*)$' || true
echo
echo '== outlines for changed source-like files =='
git diff-tree --no-commit-id --name-only -r "$head" | while IFS= read -r f; do
  case "$f" in
    *.js|*.mjs|*.cjs|*.ts|*.mts|*.cts|*.yml|*.yaml|*.md)
      echo "--- $f"
      ast-grep outline "$f" --items all 2>/dev/null || true
      ;;
  esac
done
echo
echo '== requested and live remote refs =='
git ls-remote origin \
  'refs/heads/*' \
  | rg 'ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd|e0106ce16b7b8b493f46bf075ec5baf58762bd95' || true
echo
echo '== PR metadata and current refs =='
gh pr view 80 --repo ContextualWisdomLab/noema \
  --json number,state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,url

Length of output: 5619


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd'
base_ref='origin/fix/nanoid-cve-2026-67213'
merge_base="$(git merge-base "$head" "$base_ref")"

echo "== merge base =="
printf '%s\n' "$merge_base"
git show -s --format='subject=%s' "$merge_base"
echo
echo "== cumulative PR changes =="
git diff --name-status "$merge_base" "$head"
echo
echo "== workflow sections: permissions, publisher stage, Git operations, traps =="
workflow='.github/workflows/hourly-product-development.yml'
git show "$head:$workflow" | nl -ba | rg -n -C 12 \
  '(^|\s)(permissions:|publish_product_increment:|trap |force-with-lease|git push|git update-ref|git ls-remote|git fetch|git branch|contents:|pull-requests:|id-token:|actions:|security-events:|proposal|final_candidate|cleanup)' || true
echo
echo "== relevant workflow full file =="
git show "$head:$workflow" | nl -ba
echo
echo "== lease regression test =="
git show "$head:test/hourly-product-development-publisher-lease.test.ts" | nl -ba
echo
echo "== workflow-order and cleanup tests =="
for f in \
  test/hourly-product-development-workflow.test.ts \
  test/hourly-product-development-final-candidate-cleanup.test.ts \
  test/hourly-product-development-git-mode-boundary.test.ts \
  test/hourly-product-development-publication-prerequisite.test.ts \
  test/hourly-product-development-runner-isolation.test.ts; do
  echo "--- $f"
  git show "$head:$f" | nl -ba | rg -n -C 8 \
    '(force-with-lease|proposal|cleanup|trap|push|delete|permission|contents|publisher|git mode|ls-remote)' || true
done
echo
echo "== doctoring record =="
git show "$head:docs/doctoring/atomic-product-publisher-lease.md" | nl -ba
echo
echo "== workflow references and invocation paths =="
rg -n -C 5 \
  '(hourly-product-development|publish_product_increment|workflow_call|workflow_dispatch|schedule)' \
  .github README.md docs 2>/dev/null || true

Length of output: 2263


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd'
first='1a7a6eea0e4345f45d743efee9070d2265c779df'
impl='f60c4d93fe54cff211675b47c0f6a0950aadf8bd'
compat='16941a9139bc154403410bd22ee6c6e19c96e3ed'
workflow='.github/workflows/hourly-product-development.yml'

echo '== stated lineage and parents =='
for c in "$first" "$impl" "$compat" "$head"; do
  git show -s --format='commit=%H%nparent=%P%nsubject=%s%n' "$c"
done

lineage_base="$(git rev-parse "${first}^")"
echo "== cumulative changes from lineage base $lineage_base =="
git diff --name-status "$lineage_base" "$head"
echo
echo '== workflow: complete exact-head content =='
git show "$head:$workflow" | nl -ba
echo
echo '== workflow: mutation and privilege context =='
git show "$head:$workflow" | nl -ba | rg -n -C 15 \
  '(permissions:|publish_product_increment:|trap |force-with-lease|git push|git fetch|git ls-remote|git config|proposal|final_candidate|cleanup|contents:|pull-requests:|id-token:|environment:)' || true
echo
echo '== exact-head executable contracts =='
for f in \
  test/hourly-product-development-publisher-lease.test.ts \
  test/hourly-product-development-workflow.test.ts \
  test/hourly-product-development-final-candidate-cleanup.test.ts \
  test/hourly-product-development-git-mode-boundary.test.ts \
  test/hourly-product-development-publication-prerequisite.test.ts \
  test/hourly-product-development-runner-isolation.test.ts \
  docs/doctoring/atomic-product-publisher-lease.md; do
  echo "--- $f"
  git show "$head:$f" | nl -ba
done
echo
echo '== local references to workflow or publisher stage =='
rg -n -C 5 \
  '(hourly-product-development|publish_product_increment|workflow_call|workflow_dispatch|schedule)' \
  .github README.md docs 2>/dev/null || true
echo
echo '== GitHub PR file list and current branch refs =='
gh api 'repos/ContextualWisdomLab/noema/pulls/80/files?per_page=100' \
  --jq '.[] | [.filename, .status, .sha] | `@tsv`'
gh api 'repos/ContextualWisdomLab/noema/git/ref/heads/fix%2Fatomic-product-publisher-lease' --jq '.object.sha'
gh api 'repos/ContextualWisdomLab/noema/git/ref/heads/fix%2Fnanoid-cve-2026-67213' --jq '.object.sha'

Length of output: 3527


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd'
workflow='.github/workflows/hourly-product-development.yml'
number() { awk '{printf "%6d  %s\n", NR, $0}'; }

echo '== workflow: complete exact-head content =='
git show "$head:$workflow" | number
echo
echo '== workflow: mutation and privilege context =='
git show "$head:$workflow" | number | rg -n -C 18 \
  '(permissions:|publish_product_increment:|trap |force-with-lease|git push|git fetch|git ls-remote|git config|proposal|final_candidate|cleanup|contents:|pull-requests:|id-token:|environment:)' || true

echo
echo '== exact-head regression and doctoring files =='
for f in \
  test/hourly-product-development-publisher-lease.test.ts \
  test/hourly-product-development-workflow.test.ts \
  docs/doctoring/atomic-product-publisher-lease.md; do
  echo "--- $f"
  git show "$head:$f" | number
done

echo
echo '== exact-head diffs for implementation and tests =='
for c in \
  f60c4d93fe54cff211675b47c0f6a0950aadf8bd \
  16941a9139bc154403410bd22ee6c6e19c96e3ed \
  ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd; do
  echo "--- $c"
  git show --format= --find-renames "$c" -- \
    .github/workflows/hourly-product-development.yml \
    test/hourly-product-development-publisher-lease.test.ts \
    test/hourly-product-development-workflow.test.ts \
    docs/doctoring/atomic-product-publisher-lease.md
done

echo
echo '== all workflow invocation and permission context =='
rg -n -C 8 \
  '(hourly-product-development|publish_product_increment|workflow_call|workflow_dispatch|schedule|permissions:|contents:|pull-requests:)' \
  .github README.md docs 2>/dev/null || true

echo
echo '== GitHub PR files and current refs =='
gh api 'repos/ContextualWisdomLab/noema/pulls/80/files?per_page=100' \
  --jq '.[] | [.filename, .status, .sha] | `@tsv`' || true
printf 'head: '; gh api 'repos/ContextualWisdomLab/noema/git/ref/heads/fix%2Fatomic-product-publisher-lease' --jq '.object.sha' || true
printf 'base: '; gh api 'repos/ContextualWisdomLab/noema/git/ref/heads/fix%2Fnanoid-cve-2026-67213' --jq '.object.sha' || true

Length of output: 197010


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/noema'
head='ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd'

echo '== exact-head combined status =='
gh api "repos/$repo/commits/$head/status" \
  --jq '{state, sha, statuses: [.statuses[] | {context, state, creator: .creator.login, target_url}]}'

echo
echo '== exact-head check runs =='
gh api "repos/$repo/commits/$head/check-runs?per_page=100" \
  --jq '.check_runs[] | {name, status, conclusion, app: .app.slug, started_at, completed_at, details_url}'

echo
echo '== workflow runs directly associated with exact head =='
gh api "repos/$repo/actions/runs?head_sha=$head&per_page=100" \
  --jq '.workflow_runs[] | {name, event, status, conclusion, head_sha, head_branch, run_number, html_url, created_at, updated_at}'

echo
echo '== PR review and merge metadata =='
gh pr view 80 --repo "$repo" \
  --json isDraft,mergeStateStatus,mergeable,reviewDecision,statusCheckRollup,reviews \
  --jq '{isDraft, mergeStateStatus, mergeable, reviewDecision, reviews: [.reviews[] | {author: .author.login, state, submittedAt, commit: .commit.oid}], checks: [.statusCheckRollup[] | {name, status, conclusion, workflowName}]}'

echo
echo '== repository rulesets and main branch protection metadata =='
gh api "repos/$repo/rulesets?includes_parents=true" \
  --jq '.[] | {name, target, enforcement, conditions, rules}' || true
gh api "repos/$repo/branches/main/protection" \
  --jq '{required_status_checks, required_pull_request_reviews, enforce_admins, restrictions, required_linear_history, required_signatures}' || true

echo
echo '== current PR branch and base refs =='
gh pr view 80 --repo "$repo" \
  --json headRefOid,baseRefOid,headRefName,baseRefName,isDraft,updatedAt,url

Length of output: 4533


@seonghobae, I completed the scoped security review of exact head ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd.

I found no actionable security defect in the reviewed change.

  • Atomic creation: .github/workflows/hourly-product-development.yml, Line 862 captures and validates the 40-character proposal_head. Line 867 creates the ref with --force-with-lease="refs/heads/${branch}:". The empty expected value requires server-observed absence of the destination ref. The prior ls-remote authorization pattern is removed.
  • Concurrent actors: A competing creator causes the creation push to fail. The workflow emits proposal_branch_create_lease_rejected and does not create a pull request.
  • Exact-head cleanup: Lines 872-874 delete only with --force-with-lease="refs/heads/${branch}:${proposal_head}". A concurrent update, replacement, or recreation prevents deletion of the foreign ref.
  • Trap order: The cleanup trap is installed only after the leased create succeeds at Line 875. gh pr create is within the trap scope. Line 884 clears the trap only after successful pull request creation.
  • Least privilege: The publisher job has read-only GITHUB_TOKEN permissions: actions: read, contents: read, and pull-requests: read. The late-bound Maintainer App token is repository-scoped and requests only metadata: read, contents: write, and pull-requests: write. I found no GITHUB_TOKEN write fallback, repair workflow, protection bypass, approval, merge, release, or deploy authority.
  • Regression contract: test/hourly-product-development-publisher-lease.test.ts asserts the expected-absence create lease, captured proposal head, exact-head cleanup lease, and required ordering. It rejects the predecessor unguarded push, ls-remote authorization, and unconditional deletion patterns.
  • Doctoring: docs/doctoring/atomic-product-publisher-lease.md correctly defines the Git server ref update as the concurrency boundary. It documents fail-closed creation and non-destructive cleanup.

The live fix/nanoid-cve-2026-67213 ref remains e0106ce16b7b8b493f46bf075ec5baf58762bd95. GitHub PR metadata also reports base snapshot 9b81b9291dac874fa8f75ec310672eb038428620. The successful ci and reviewer-ci runs associated with this head are historical integration evidence because they predate the current stacked-base context. I did not treat them as current-base acceptance evidence.

The PR remains Draft. It has no recorded reviews. The branch-protection API was unavailable to this integration with HTTP 403. This review is not approval or merge authority. Current-base exact-head CI, security, coverage, governance, and an eligible independent approval remain required.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

Exact-head security review for ab0819a0f504a00dccaa3e0e3d6c41de4c38a5fd (not an approval): the new expected-absence creation lease and exact-proposal-head cleanup lease correctly remove the original check-then-act overwrite/delete race, but one publication TOCTOU boundary remains.

After the leased branch creation succeeds, gh pr create consumes the mutable branch ref and the workflow immediately clears the error trap. There is no post-create assertion that the created PR still has head.sha == proposal_head and base.sha == the previously verified proposal base. A collaborator/App able to move the proposal ref (or main) in the interval can therefore cause the publisher to report a successfully opened PR whose server-side head/base no longer matches the artifact/base that the preceding jobs verified. Later PR governance should catch a moved head, but the publisher itself must fail closed rather than declare publication success on mismatched evidence.

Required test-first follow-up in this same PR:

  1. Extend the publisher regression contract before implementation so it requires a post-gh pr create server read of the created PR and exact equality of both head SHA and base SHA to proposal_head / the verified base.
  2. Keep cleanup armed until that read succeeds. If creation succeeded but identity revalidation fails, close the PR created by this run and retain the existing exact-head branch deletion lease; never unconditionally delete a ref another actor advanced.
  3. Clear the trap only after the PR identity check succeeds. Treat malformed/missing PR identity evidence as failure.

Do not add a pre-create ls-remote check as write authority, do not weaken the current lease, and do not convert this into a repair workflow. This finding is separate from the stale stacked-base evidence already documented in the PR body.

@seonghobae
seonghobae marked this pull request as ready for review August 8, 2026 12:22

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please perform a substantive security review of exact head d144f016a18d164de0363246a9b9f575d1d27dd5 against live stacked base head e0106ce16b7b8b493f46bf075ec5baf58762bd95. Focus on the expected-absence branch creation lease, exact-proposal-head cleanup lease, post-gh pr create server-side head.sha/base.sha binding, cleanup trap ordering, malformed/missing/mismatched PR identity evidence, concurrent actor behavior, least privilege, executable regression coverage, and doctoring. Application CI and reviewer-ci are terminal-success on this exact head, but this review is evidence only and must not be treated as an independent approval or merge authority.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/noema'
head='d144f016a18d164de0363246a9b9f575d1d27dd5'
base='e0106ce16b7b8b493f46bf075ec5baf58762bd95'
workflow='.github/workflows/hourly-product-development.yml'

echo '== exact head and merge-base relationship =='
git show -s --format='head=%H%nparents=%P%nsubject=%s' "$head"
git cat-file -e "${base}^{commit}"
printf 'merge_base='
git merge-base "$head" "$base"
git merge-base --is-ancestor "$base" "$head" && echo 'base_is_ancestor=true' || echo 'base_is_ancestor=false'
echo

echo '== cumulative changes from specified base =='
git diff --name-status "$base" "$head"
echo

echo '== workflow mutation, cleanup, PR creation, and token contexts =='
git show "$head:$workflow" | nl -ba | rg -n -C 18 \
  '(permissions:|publish_product_increment:|trap |force-with-lease|git push|gh pr create|gh pr view|gh pr close|proposal_head|proposal_base|head\.sha|base\.sha|cleanup|Maintainer|contents:|pull-requests:|GITHUB_TOKEN|token)' || true
echo

echo '== focused workflow slice around all relevant lines =='
git show "$head:$workflow" | nl -ba | sed -n '800,980p'
echo

echo '== exact-head contract and doctoring files =='
for f in \
  test/hourly-product-development-publisher-lease.test.ts \
  test/hourly-product-development-workflow.test.ts \
  test/hourly-product-development-final-candidate-cleanup.test.ts \
  test/hourly-product-development-git-mode-boundary.test.ts \
  docs/doctoring/atomic-product-publisher-lease.md; do
  echo "--- $f"
  git show "$head:$f" | nl -ba | rg -n -C 8 \
    '(force-with-lease|proposal_head|proposal_base|gh pr create|gh pr view|gh pr close|head\.sha|base\.sha|trap|cleanup|malformed|missing|mismatch|identity|permission|ls-remote)' \
    || true
done
echo

echo '== changed test and doctoring files relative to base =='
git diff --find-renames --unified=35 "$base" "$head" -- \
  "$workflow" \
  test/hourly-product-development-publisher-lease.test.ts \
  test/hourly-product-development-workflow.test.ts \
  docs/doctoring/atomic-product-publisher-lease.md
echo

echo '== current PR, refs, checks, and review state =='
gh pr view 80 --repo "$repo" \
  --json state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,mergeable,mergeStateStatus,reviewDecision,reviews,url
printf 'live_base_ref='
gh api "repos/$repo/git/ref/heads/fix%2Fnanoid-cve-2026-67213" --jq '.object.sha'
printf 'pr_head_ref='
gh api "repos/$repo/git/ref/heads/fix%2Fatomic-product-publisher-lease" --jq '.object.sha'
gh api "repos/$repo/commits/$head/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name,.status,.conclusion,.app.slug] | `@tsv`'

Length of output: 3705


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/noema'
head='d144f016a18d164de0363246a9b9f575d1d27dd5'
base='e0106ce16b7b8b493f46bf075ec5baf58762bd95'
workflow='.github/workflows/hourly-product-development.yml'
number() { awk '{printf "%6d  %s\n", NR, $0}'; }

echo '== workflow permissions and publisher job context =='
git show "$head:$workflow" | number | grep -n -E -C 15 \
  'permissions:|publish_product_increment:|Maintainer|GITHUB_TOKEN|contents:|pull-requests:|actions:|id-token:' || true

echo
echo '== workflow publication and cleanup sequence =='
git show "$head:$workflow" | number | sed -n '800,980p'

echo
echo '== all workflow mutation and PR identity operations =='
git show "$head:$workflow" | number | grep -n -E -C 12 \
  'trap |force-with-lease|git push|gh pr create|gh pr view|gh pr close|proposal_head|proposal_base|head\.sha|base\.sha|cleanup|ls-remote' || true

echo
echo '== full publisher lease regression =='
git show "$head:test/hourly-product-development-publisher-lease.test.ts" | number

echo
echo '== matching sections from workflow regression =='
git show "$head:test/hourly-product-development-workflow.test.ts" | number | grep -n -E -C 15 \
  'force-with-lease|proposal_head|proposal_base|gh pr create|gh pr view|gh pr close|head\.sha|base\.sha|trap|cleanup|ls-remote|identity' || true

echo
echo '== full atomic publisher lease doctoring =='
git show "$head:docs/doctoring/atomic-product-publisher-lease.md" | number

echo
echo '== exact-head changes from requested base =='
git diff --find-renames --unified=20 "$base" "$head" -- \
  "$workflow" \
  test/hourly-product-development-publisher-lease.test.ts \
  test/hourly-product-development-workflow.test.ts \
  docs/doctoring/atomic-product-publisher-lease.md

echo
echo '== PR, live refs, check state, and reviews =='
gh pr view 80 --repo "$repo" \
  --json state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,mergeable,mergeStateStatus,reviewDecision,reviews,url
printf 'live_base_ref='
gh api "repos/$repo/git/ref/heads/fix%2Fnanoid-cve-2026-67213" --jq '.object.sha'
printf 'pr_head_ref='
gh api "repos/$repo/git/ref/heads/fix%2Fatomic-product-publisher-lease" --jq '.object.sha'
gh api "repos/$repo/commits/$head/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name,.status,.conclusion,.app.slug] | `@tsv`'

Length of output: 22883


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/noema'
head='d144f016a18d164de0363246a9b9f575d1d27dd5'
number() { awk '{printf "%6d  %s\n", NR, $0}'; }

echo '== complete publisher lease regression =='
git show "$head:test/hourly-product-development-publisher-lease.test.ts" | number

echo
echo '== complete workflow regression =='
git show "$head:test/hourly-product-development-workflow.test.ts" | number

echo
echo '== complete atomic publisher lease doctoring =='
git show "$head:docs/doctoring/atomic-product-publisher-lease.md" | number

echo
echo '== workflow lines 638 through 930 =='
git show "$head:.github/workflows/hourly-product-development.yml" | number | sed -n '638,930p'

echo
echo '== current PR metadata =='
gh pr view 80 --repo "$repo" \
  --json state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,mergeable,mergeStateStatus,reviewDecision,reviews,url

echo
echo '== current PR branch and requested live base ref =='
printf 'live_base_ref='
gh api "repos/$repo/git/ref/heads/fix%2Fnanoid-cve-2026-67213" --jq '.object.sha'
printf 'pr_head_ref='
gh api "repos/$repo/git/ref/heads/fix%2Fatomic-product-publisher-lease" --jq '.object.sha'

echo
echo '== exact-head check runs =='
gh api "repos/$repo/commits/$head/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name,.status,.conclusion,.app.slug] | `@tsv`'

Length of output: 52795


@seonghobae, I completed the requested exact-head security review.

I found one actionable fail-closed cleanup defect.

Finding — Malformed gh pr create output can leave an unverified pull request open.

In .github/workflows/hourly-product-development.yml, Lines 897-900 reject a malformed pr_url. The active ERR trap then calls gh pr close "$pr_url" at Line 892. A malformed URL cannot identify the pull request. The close command fails and its error is suppressed. The workflow then deletes the branch only through the correct exact-head lease.

If gh pr create created the pull request but returned malformed output, the publisher fails without closing that pull request. This conflicts with the documented fail-closed behavior for malformed evidence in docs/doctoring/atomic-product-publisher-lease.md, Line 54.

Capture a usable created-PR identifier in a machine-readable response from the creation operation. Install the created-PR cleanup trap after that identifier is available. On malformed or missing identity evidence, close only that identifier and then retain the existing exact-proposal-head branch deletion lease. Add a regression case that requires this cleanup path.

The remaining requested controls are correctly implemented at exact head:

  • Creation lease: Lines 868-875 capture a canonical proposal_head and use --force-with-lease="refs/heads/${branch}:". A concurrent creator causes the push to fail.
  • Branch cleanup lease: Lines 878-881 delete only when the remote branch remains at proposal_head. A concurrent update or replacement prevents deletion.
  • Post-create binding: Lines 903-921 re-read the created PR from GitHub. The workflow requires canonical 40-character head.sha and base.sha values. It requires equality with proposal_head and expected_base.
  • Trap ordering: The workflow installs branch cleanup only after successful branch creation. It retains cleanup through PR creation and server-side identity validation. It clears the trap only at Line 923.
  • Unavailable, invalid, and mismatched identity evidence: Lines 903-920 fail closed. When pr_url is valid, the trap closes that created PR and applies the exact-head branch cleanup lease.
  • Least privilege: The publisher job has read-only GITHUB_TOKEN permissions. The late-bound Maintainer App token is repository-scoped and requests only metadata: read, contents: write, and pull-requests: write. I found no write-capable GITHUB_TOKEN fallback, repair workflow, protection bypass, approval, merge, release, or deployment authority.
  • Regression and doctoring: The lease regression covers expected absence, exact-head deletion, post-create PR re-read, head/base comparisons, and trap clearing order. It does not cover the malformed-URL cleanup case. The doctoring accurately explains the server-side lease and PR identity model, except for the malformed-URL cleanup claim above.

The requested commit has merge base e0106ce16b7b8b493f46bf075ec5baf58762bd95. The verify and reviewer checks succeeded on that exact commit.

At inspection time, PR #80 pointed to 97b9a2f5f604f9885c0c32e5204f6b2f9ccfed13, not d144f016a18d164de0363246a9b9f575d1d27dd5. This review applies only to the requested exact head. It is not approval or merge authority.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae
seonghobae marked this pull request as draft August 9, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant