Skip to content

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

Draft
seonghobae wants to merge 62 commits into
mainfrom
fix/atomic-product-publisher-lease
Draft

fix(security): make product publisher branch creation atomic#80
seonghobae wants to merge 62 commits into
mainfrom
fix/atomic-product-publisher-lease

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Purpose

Hardens hourly-product-development, its atomic proposal publisher, and the NVIDIA NIM credential compartment without adding merge/release/deployment authority or repair/self-modifying workflows. Repository-wide architecture remains canonical on #71; this behavior is Proposed until protected integration and operational proof.

Fresh source identity

  • PR base: main.
  • Protected main: c85d710804139c0697d7ef8fa47d02b1389e6d84 (fix(security): update transitive nanoid for CVE-2026-67213 #76 integrated; nanoid@3.3.17 protected truth).
  • Head branch: fix/atomic-product-publisher-lease.
  • Exact current head: 62b74b65d417b5a9f2519bb67abf0cf101deea17.
  • Fresh compare against protected main: diverged, 62 commits ahead / 1 behind, merge base 9adb784902cae19a150146a8fadd6a0491f67814.
  • GitHub reports this Draft non-mergeable.

The old PR body described #76 as unintegrated and #87/#89 as current sibling owners. Those facts are historical: #76 is protected truth; clean direct-main successors #90 and #91 now own governance/AGENTS and deterministic package-manager/install-script controls respectively.

Current convergence hazards

The stale #80 lineage overlaps current protected/successor authority in material paths:

Therefore a non-destructive metadata retarget or sibling merge is not enough. Only #80's unique scheduler/publisher/NIM delta should be rebuilt on the final combined protected lineage after #90/#91 integrate or an equivalent protected baseline is independently proven. No predecessor checks/reviews transfer.

Implemented authority boundary

The scheduler contract requires fresh evidence, falsifiable RCA, materially distinct remedies, feasibility proof, smallest safe action, observable acceptance and immediate queue continuation. Prompt/docs/RCA/test/commit/review/merge/blocker states are intermediate.

The publisher uses expected-absence branch lease, exact proposal-head cleanup, structured PR identity, a hidden high-entropy marker, paginated recovery, exact server-side head/base revalidation and fail-closed ambiguity handling.

The proposer may receive NVIDIA_NIM_API_KEY but cannot claim executable RED/GREEN. A separate uncredentialed verifier executes the immutable proposal; a credential-bearing non-executing publisher performs publication:

credential-bearing proposal
→ uncredentialed executable verification
→ credential-bearing non-executing publication

Deterministic governance, formal review, merge, release and deployment authority remain outside model judgement.

Historical exact-head evidence

For current stale head 62b74b65d417b5a9f2519bb67abf0cf101deea17, application CI 31346804416 and reviewer-ci 31346804418 were terminal success. Central Security Scan was absent under the then-feature-base event path and was non-passing defer_until_trigger. Formal reviews and inline review threads were absent at the latest fresh read.

That evidence proves only the historical exact head. It does not make the diverged head mergeable or transfer to a future rebuilt successor.

Safe continuation

  1. Keep fix(security): make product publisher branch creation atomic #80 Draft and stable; do not patch the stale lineage merely to make it look current.
  2. Integrate fix(governance): restack independent approval audit after nanoid #90 and build(reproducibility): restack deterministic npm toolchain #91 (or equivalent current protected governance/package-manager controls) under actual live policy.
  3. Rebuild only fix(security): make product publisher branch creation atomic #80's unique scheduler/publisher/NIM delta from the then-current protected head, explicitly preserving AGENTS.md, CI/package-manager/lockfile controls and CHANGELOG evidence.
  4. Re-run fresh exact-head application CI, reviewer-ci, eligible protected-base Security Scan, coverage/package/provenance and current review evidence.
  5. Satisfy live chore(governance): protect main and enforce release checks #27/chore(operations): provision and activate hourly maintainer App #29 governance and qualifying independent formal-review requirements.
  6. After protected merge, prove publisher atomicity, work-conserving continuation, NIM secret compartment and proposer/verifier/publisher authority separation from protected source.

No protection bypass, audit waiver, synthetic approval, force push, version bump, release or deployment is introduced.

Related: #27, #29, #71, #76, #77, #79, #90, #91

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

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: 33535c97-f408-4f4f-8090-8e03d591e802

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 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