Repository navigation
fix(standing-pr): validate the manifest against source before publishing - #587
Merged
Merged
Conversation
Contributor
|
Standing release PR: #582 · v0.41.1 queued (4 packages) · open 3h 2m · ✅ ready to merge Release Preview — this PR contributes no changes
Currently queued for release
Updated automatically by ReleaseKit |
goosewobbler
force-pushed
the
fix/standing-pr-publish-hardening
branch
from
July 19, 2026 22:36
3bdd5f3 to
ac9c026
Compare
goosewobbler
force-pushed
the
fix/standing-pr-publish-hardening
branch
4 times, most recently
from
July 19, 2026 23:51
be37871 to
88ac24c
Compare
The standing-PR publish path trusted the release manifest comment verbatim: a write-access actor could edit the comment's base64 to forge `versionOutput` and drive arbitrary git tags and registry publishes under the project's identity (the draft flow already recomputed; standing-pr did not). Harden the publish boundary with three layers, and cap release-notes length so an inflated body can't wedge standing-PR updates. #556 — before creating any tag or touching a registry, `publishFromManifest` now: - Recompute-and-compare: reads each package's actual version from the merged commit (the reviewed-and-merged source of truth) and refuses a manifest that names an unknown package, a version that doesn't match the source, or a consumer tag encoding no published version; the baseSha ancestry check is upgraded from a warning to a hard refusal. - Schema-validate: parseManifest runs the decoded payload through a Zod schema, requires an exact schemaVersion (an absent version no longer slips through), and rejects oversized/malformed manifests. - Author-bind: ForgeComment surfaces the comment author; findComment prefers a bot-authored match, upsertMarkerComment only adopts a bot-authored comment, and the publish refuses a human-authored manifest comment. #558 — per-package release notes are truncated at a documented bound with a "…(truncated)" marker, and the notes region joins the standing-PR body-cap shed order so an over-long body degrades gracefully instead of hitting GitHub's 422. Closes #556 Closes #558 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BT6fVBZrmExXZj7vQ29881
goosewobbler
force-pushed
the
fix/standing-pr-publish-hardening
branch
from
July 20, 2026 00:09
88ac24c to
1e0645d
Compare
Owner
Author
|
@greptileai review plz |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens the standing-PR publish path, which previously trusted the
<!-- releasekit-manifest -->comment verbatim — a repo write-access actor could edit the comment's hidden base64 to forgeversionOutputand drive arbitrary git tags and registry publishes under the project's identity (an insider/confused-deputy supply-chain path). The draft flow already recomputed-and-compared; standing-pr did not.New trust model — what is now recomputed / validated / authenticated
Before
publishFromManifestcreates any tag or touches any registry:assertManifestMatchesSourcereads each package's actual on-disk version (getWorkspacePackageVersions, ecosystem-aware) and refuses if the manifest names a package the workspace lacks, a version that diverges from the source, or a consumer tag that encodes no published version. ThebaseShaancestry check is upgraded from awarn()to a hard refusal (a manifest computed off this branch's history is rejected). A single divergence rejects the whole manifest before anything is published.parseManifestnow runs the decoded payload through a Zod schema, requires an exactschemaVersion ∈ {1,2}(the old< MIN || > MAXrange let an absent version through — both comparisons are false forundefined), and rejects oversized/malformed manifests.ForgeCommentnow carries the comment author.findCommentprefers a bot-authored marker match,upsertMarkerCommentonly adopts a bot-authored comment (fixing the pre-seed/adoption race for every marker), and the publish refuses a manifest comment that isn't bot-authored.Before:
createReleaseTags/runPublishStepran onmanifest.versionOutputas-is; the only guards were who merged, ancestry (warn-only), and override-label staleness — never the version/tag/target content, and no schema or author check. After: the package set, versions and tags are re-derived from and cross-checked against the merged source, the manifest is schema-validated, and the manifest comment must come from the bot.DoS hardening (#558)
Release-notes length was unbounded and the body cap never shed the notes region, so inflated notes could push the PR body past GitHub's limit and 422 every standing-PR update. Now per-package notes are truncated at a documented bound (
…(truncated)), and the notes region is part of the body-cap shed order — an over-long body collapses/drops notes instead of failing the update.Test plan
baseShais refused; an absentschemaVersionis rejected. Unbounded release-notes length wedges standing-PR updates (body-cap DoS) #558:renderNotesRegion/truncatePackageNotestruncate oversized per-package notes at the bound with the marker; an inflated multi-package notes body renders under the cap andupdatePullRequestis never handed an over-limit body.main(clean); re-verified the CI way:biome check .exit 0, forcedturbo run typecheck test→ 24/24.Closes #556
Closes #558
🤖 Generated with Claude Code
https://claude.ai/code/session_01BT6fVBZrmExXZj7vQ29881
Greptile Summary
This PR hardens the standing-PR publish path against insider/confused-deputy supply-chain attacks where a write-access actor could edit the hidden base64 in the release manifest comment to drive arbitrary git tags and registry publishes. It also caps per-package release notes to prevent inflated notes from wedging the standing PR via GitHub's 65 536-char body limit.
assertManifestMatchesSourcerecomputes on-disk package versions and validates every tag in the manifest against them before any publish operation;parseManifestnow schema-validates via Zod and requires an exact knownschemaVersion(closing theundefinedbypass in the old range); andfindComment/upsertMarkerCommentnow prefer bot-authored matches and refuse to adopt human-authored marker comments.publishFromManifesthard-rejects a manifest comment not authored by the bot;fetchStandingPRSnapshotapplies the same check so a forged manifest isn't surfaced in the preview either.truncatePackageNotescaps each package's notes at 8 000 chars; the progressive body-shedding logic collapses/drops the notes region when even a fully-shed changelog still overflows.Confidence Score: 5/5
Safe to merge — the validation guards are well-structured and every new code path has a dedicated test.
All three hardening layers are correctly implemented. The tag-boundary regex blocks digit-suffix and hyphen/plus-suffix version-embedding bypasses. The schema version membership check closes the undefined-bypass. No correctness issues found.
No files require special attention.
Important Files Changed
Comments Outside Diff (1)
packages/release/src/standing-pr/standing-pr.ts, line 607-624 (link)fetchStandingPRSnapshotdoes not author-check the manifest commentfindManifestCommentnow returns a human-authored comment as a fallback when no bot-authored one exists.fetchStandingPRSnapshotparses that comment without callingisBotComment, so a pre-seeded forged manifest would be surfaced in the preview. This is a display-only path (it feedspreview, notpublishFromManifest), so the impact is showing misleading version information rather than triggering a publish — but it is an inconsistency with the rest of the hardening.Reviews (7): Last reviewed commit: "fix(standing-pr): validate the manifest ..." | Re-trigger Greptile