Skip to content

fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) - #294

Merged
rapiercraft-forgedock[bot] merged 1 commit into
stagingfrom
forgedock/issue-202-1-41a6-995b-9ace9f9f01ab
Aug 17, 2026
Merged

fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186)#294
rapiercraft-forgedock[bot] merged 1 commit into
stagingfrom
forgedock/issue-202-1-41a6-995b-9ace9f9f01ab

Conversation

@rapiercraft-forgedock

Copy link
Copy Markdown

Closes #202

ForgeDock verified handoff

  • Run: run_420dedb9-9031-41a6-995b-9ace9f9f01ab
  • Build Packet: art_24789818-8f8c-445e-b452-8c54175b24e6 (durable artifact on the linked issue)
  • Build Result: art_9b70e4b2-0c0e-46c1-8be9-1724c80da3e9 (durable artifact on the linked issue)
  • Verified head: e70c2a305d73c2d6db435fb61bc13e577f1b053e

Summary

Implemented independent npm and GitHub Packages recovery with fail-closed exact-version queries, per-registry publication gating, cleanup, reconciliation barrier, and focused workflow regression coverage.

Changed paths

  • .github/workflows/publish.yml
  • bin/tests/publish-workflow.test.mjs

Acceptance evidence

  • passed — Against the current staging target, the reviewed-SHA finding (8aa2c7a) is reproducible at the named workflow boundary and the remediation is bounded: .github/workflows/publish.yml retains both registry publication steps, removes the npm-derived shared recovery decision, and the changed-path set is exactly the two expected files.: Updated only the workflow and its regression test; both publication steps remain, REGISTRY_ALREADY_PUBLISHED is absent from workflow logic/comments, and the test asserts the preserved path guards and publish-main serialization. Controller-observed subject evidence: GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 labels: workflow:engine-error, review-finding, needs-validation, priority:P1 | GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 body: ## Problem npm-only recovery suppresses a needed GitHub Packages retry Source: PR Deploy: staging → main #186 — Deploy: staging → main Reviewed SHA: 8aa2c7ae638699e9fdbcd27b37549ccba6993683 Run: run_72407589-e453-408e-afa1-8572c6ae712c Reviewers: correctness Source findings: correctness:review-correctness-part-1-of-10:COR-186-002 Reviewer sessions: bfc29693 Confidence: HIGH Severity: HIGH Controller disposition: blocking Scope disposition: in_scope Scope rationale: The publish workflow is in this execution group's inventory and already defines both registry publications; sep…
  • passed — The Bump version step records independent exact-version presence for npm and GitHub Packages: forgedock@$NEXT_VERSION is checked on npmjs and @rapiercraftstudios/forgedock@$NEXT_VERSION is checked on npm.pkg.github.com with its token/config. Only a confirmed not-found response means absent; auth, network, malformed, or ambiguous results fail closed before publication or finalization, while npm-present reruns still set VERSION to NEXT_VERSION.: Bump version now queries both exact package specs with explicit registries, a GH_TOKEN-authenticated RUNNER_TEMP npmrc for GitHub Packages, exact output matching, explicit 404 classification, and nonzero fail-closed branches; npm-present reruns use --allow-same-version and set VERSION to NEXT_VERSION. Regression assertions cover these adapters and classifications. Controller-observed subject evidence: GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 labels: workflow:engine-error, review-finding, needs-validation, priority:P1 | GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 body: ## Problem npm-only recovery suppresses a needed GitHub Packages retry Source: PR Deploy: staging → main #186 — Deploy: staging → main Reviewed SHA: 8aa2c7ae638699e9fdbcd27b37549ccba6993683 Run: run_72407589-e453-408e-afa1-8572c6ae712c Reviewers: correctness Source findings: correctness:review-correctness-part-1-of-10:COR-186-002 Reviewer sessions: bfc29693 Confidence: HIGH Severity: HIGH Controller disposition: blocking Scope disposition: in…
  • passed — The publication state matrix is independent and idempotent: neither registry present publishes both, npm present/GitHub Packages absent skips npm and runs GitHub Packages, npm absent/GitHub Packages present runs npm and skips GitHub Packages, and both present skips both. Save/package-name mutation/restore are tied to the GitHub Packages attempt rather than npm state.: NPM_ALREADY_PUBLISHED and GITHUB_PACKAGES_ALREADY_PUBLISHED independently gate their matching publish steps and are set after successful publication; GitHub Packages save/mutation/restore uses the GitHub Packages state and attempt marker. The test contains all four matrix pairs and verifies the partial npm-success/GitHub-retry path. Controller-observed subject evidence: GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 labels: workflow:engine-error, review-finding, needs-validation, priority:P1 | GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 body: ## Problem npm-only recovery suppresses a needed GitHub Packages retry Source: PR Deploy: staging → main #186 — Deploy: staging → main Reviewed SHA: 8aa2c7ae638699e9fdbcd27b37549ccba6993683 Run: run_72407589-e453-408e-afa1-8572c6ae712c Reviewers: correctness Source findings: correctness:review-correctness-part-1-of-10:COR-186-002 Reviewer sessions: bfc29693 Confidence: HIGH Severity: HIGH Controller disposition: blocking Scope disposition: in_scope Scope rationale: The …
  • passed — Release finalization is behind an explicit both-registries-reconciled barrier. Sync plugin.json, Push version bump, and Create GitHub Release cannot run after a failed or cancelled publication; a failed GitHub Packages attempt restores the package name when possible but leaves metadata unreconciled so a rerun rechecks both registries and retries only the missing publication.: Added Verify registry reconciliation after always cleanup; it requires both states true and the unscoped package name before writing REGISTRIES_RECONCILED=true. Sync, Push, and Release require that barrier without always(), while failed GitHub Packages attempts restore package.json and cannot reach finalization. Controller-observed subject evidence: GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 labels: workflow:engine-error, review-finding, needs-validation, priority:P1 | GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 body: ## Problem npm-only recovery suppresses a needed GitHub Packages retry Source: PR Deploy: staging → main #186 — Deploy: staging → main Reviewed SHA: 8aa2c7ae638699e9fdbcd27b37549ccba6993683 Run: run_72407589-e453-408e-afa1-8572c6ae712c Reviewers: correctness Source findings: correctness:review-correctness-part-1-of-10:COR-186-002 Reviewer sessions: bfc29693 Confidence: HIGH Severity: HIGH Controller disposition: blocking Scope disposition: in_scope Scope rationale: The publish workflow is i…
  • passed — bin/tests/publish-workflow.test.mjs provides deterministic regression coverage for the exact queries, fail-closed handling, four-state matrix, partial-publication rerun, cleanup/finalization barrier, and absence of REGISTRY_ALREADY_PUBLISHED coupling, while preserving the existing publish path guards, serialized concurrency policy, and version-push rebase/retry contract.: Replaced the shared-flag assertions with focused step-block contract tests for queries, auth/config, error handling, matrix behavior, cleanup, barrier consumers, old-symbol absence, path guards, concurrency, and rebase/retry commands. Frozen test and git diff --check verification passed. Controller-observed subject evidence: GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 labels: workflow:engine-error, review-finding, needs-validation, priority:P1 | GitHub issue fix: npm-only recovery suppresses a needed GitHub Packages retry (review finding — PR #186) #202 body: ## Problem npm-only recovery suppresses a needed GitHub Packages retry Source: PR Deploy: staging → main #186 — Deploy: staging → main Reviewed SHA: 8aa2c7ae638699e9fdbcd27b37549ccba6993683 Run: run_72407589-e453-408e-afa1-8572c6ae712c Reviewers: correctness Source findings: correctness:review-correctness-part-1-of-10:COR-186-002 Reviewer sessions: bfc29693 Confidence: HIGH Severity: HIGH Controller disposition: blocking Scope disposition: in_scope Scope rationale: The publish workflow is in this execution group's …

Verification

  • passedgit diff --check — warning: in the working copy of '.github/workflows/publish.yml', LF will be replaced by CRLF the next time Git touches it | warning: in the working copy of 'bin/tests/publish-workflow.test.mjs', LF will be replaced by CRLF the next time Git touches it
  • passedC:\Program Files\nodejs\node.exe C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js run build — > forgedock@1.8.0 build | > tsc -p tsconfig.json
  • passedC:\Program Files\nodejs\node.exe C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js run docs:build — - generating sitemap... | ✓ generating sitemap... | build complete in 7.32s.
  • passedC:\Program Files\nodejs\node.exe C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js test — # skipped 8 | # todo 0 | # duration_ms 102765.9986

Residual risks

  • Registry behavior remains externally dependent; staging review should validate npm CLI 404/auth/network response semantics against live runner behavior.

@rapiercraft-forgedock

Copy link
Copy Markdown
Author

ForgeDock Review Evidence

One bounded projection for the complete frozen reviewer wave. The controller's consolidated Review Verdict remains authoritative.

Completed groups

review-correctness · correctness · completed
  • Session lineage: 01a00d24-a860-7f59-a590-440f32748f16
  • Summary: No actionable correctness findings identified in the reviewed execution-group paths.
  • Findings:
    • No actionable findings reported.
review-security · security · completed
  • Session lineage: 01a00d24-a822-7f72-baaf-dad93dbd7153
  • Summary: Found one high-severity credential-lifetime issue in the publish workflow.
  • Findings:
    • HIGH · GitHub token is exposed to unrelated npm lifecycle and test code.github/workflows/publish.yml:167-171, 243-258, 272-281
      • Evidence: .github/workflows/publish.yml:167-171 writes the full GH_TOKEN to $RUNNER_TEMP/forgedock-github-packages.npmrc. The step-wide environment at :252-253 gives that token to the same step that runs npm version at :245/:248; the file is created before that command and is not removed before Publish to npm at :255-258. It also remains for later commands when the GitHub Packages branch is skipped (for example, the npm-present/GitHub-Packages-present or npm-absent/GitHub-Packages-present recovery states), including the later npm test. The repository's current package.json declares a prepack lifecycle script, so npm publish executes repository-controlled build/lifecycle code while the predictable token file is present. A compromised version/prepack/test hook or build dependency can read the file via RUNNER_TEMP and exfiltrate the contents:write/packages:write GITHUB_TOKEN; log masking does not protect a readable file or environment variable.
      • Remediation: Scope GH_TOKEN only to the registry-query operation, then remove the query npmrc and unset the token before running npm version, the npm publication, or finalization/test code. Recreate a fresh, mode-600 config only inside the GitHub Packages publication boundary and delete it with cleanup on every exit path. If the publication lifecycle itself must be fully untrusted, prepack without credentials and publish the resulting artifact in an isolated credentialed command.
review-infrastructure · infrastructure · completed
  • Session lineage: 01a00d24-a7db-7477-b53c-aaec946b70aa
  • Summary: No actionable release-infrastructure findings in the scoped workflow change.
  • Findings:
    • No actionable findings reported.

Failed groups

None.

@rapiercraft-forgedock

Copy link
Copy Markdown
Author

ForgeDock · Review Verdict

Run run_420dedb9-9031-41a6-995b-9ace9f9f01ab · artifact art_a04cc8f5-9536-4457-9f6b-dfc421ccc5fe

Disposition: approve · Reviewed SHA: e70c2a305d73c2d6db435fb61bc13e577f1b053e

Reviewer roles: correctness, security, infrastructure

Review plan

Risk: high · Specialist group budget: 3
Identity: review-plan-5068a31728f9188531f0 · Generation: 1 · Frozen: yes
Absolute budget: 3 logical sessions (3 specialist groups), 2 attempts/group

Required capabilities

  • acceptance-correctness · score 1000 · mandatory evidence
  • release · score 120 · mandatory evidence
  • security · score 110 · mandatory evidence

Execution groups

  • review-correctness (correctness) · acceptance-correctness — mandatory intent, acceptance, target-authority, and correctness review
  • review-security (security) · security — Build Packet declares a security/trust risk; diff changes security/trust semantics: .github/workflows/publish.yml
  • review-infrastructure (infrastructure) · release — deployment, CI, or infrastructure path: .github/workflows/publish.yml

Non-executing specialist roles

  • data · score 0 · below-threshold — no qualifying evidence
  • api-compatibility · score 0 · below-threshold — no qualifying evidence
  • frontend · score 0 · below-threshold — no qualifying evidence
  • concurrency · score 35 · below-threshold — diff changes concurrency or coordination semantics: bin/tests/publish-workflow.test.mjs

Finding projection

Policy: all · candidates: 1 · materialized: 1 · suppressed: 0

Findings

  • HIGH · GitHub token is exposed to unrelated npm lifecycle and test code · reviewers: security
    .github/workflows/publish.yml:167-171 writes the full GH_TOKEN to $RUNNER_TEMP/forgedock-github-packages.npmrc. The step-wide environment at :252-253 gives that token to the same step that runs npm version at :245/:248; the file is created before that command and is not removed before Publish to npm at :255-258. It also remains for later commands when the GitHub Packages branch is skipped (for example, the npm-present/GitHub-Packages-present or npm-absent/GitHub-Packages-present recovery states), including the later npm test. The repository's current package.json declares a prepack lifecycle script, so npm publish executes repository-controlled build/lifecycle code while the predictable token file is present. A compromised version/prepack/test hook or build dependency can read the file via RUNNER_TEMP and exfiltrate the contents:write/packages:write GITHUB_TOKEN; log masking does not protect a readable file or environment variable.
    Location: .github/workflows/publish.yml:167-171, 243-258, 272-281
    Sources: security:SEC-294-001
    Sessions: 01a00d24-a822-7f72-baaf-dad93dbd7153
    Remediation: Scope GH_TOKEN only to the registry-query operation, then remove the query npmrc and unset the token before running npm version, the npm publication, or finalization/test code. Recreate a fresh, mode-600 config only inside the GitHub Packages publication boundary and delete it with cleanup on every exit path. If the publication lifecycle itself must be fully untrusted, prepack without credentials and publish the resulting artifact in an isolated credentialed command.

Verification

Command Status Duration Summary
git diff --check passed 58 ms warning: in the working copy of '.github/workflows/publish.yml', LF will be replaced by CRLF the next time Git touches it | warning: in the working copy of 'bin/tests/publish-workflow.test.mjs', LF will be replaced by CRLF the next time Git touches it
C:\Program Files\nodejs\node.exe C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js run build passed 13949 ms > forgedock@1.8.0 build | > tsc -p tsconfig.json
C:\Program Files\nodejs\node.exe C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js run docs:build passed 9517 ms - generating sitemap... | ✓ generating sitemap... | build complete in 7.32s.
C:\Program Files\nodejs\node.exe C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js test passed 148493 ms # skipped 8 | # todo 0 | # duration_ms 102765.9986

@rapiercraft-forgedock
rapiercraft-forgedock Bot merged commit c059532 into staging Aug 17, 2026
5 checks passed
@rapiercraft-forgedock
rapiercraft-forgedock Bot deleted the forgedock/issue-202-1-41a6-995b-9ace9f9f01ab branch August 17, 2026 00:48
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