RFC 0002: zero-trust publishing via GitHub Actions OIDC - #86
Merged
Conversation
Proposes replacing the shared PKG_PR_BRIDGE_ADMIN_TOKEN on the publish path with GitHub Actions OIDC tokens the Worker verifies against GitHub's JWKS, so fork PRs can publish preview builds. Fork pull_request runs cannot mint OIDC tokens either, so the consumer workflow splits into an untrusted build leg (pull_request, no credentials, packs tarballs into an artifact) and a trusted publish leg (workflow_run, base-repo context, mints the token). Section 8 records six security requirements found while reviewing the design. Two are load-bearing: the trusted leg must re-check the preview-build label against the API, because the build leg's own check runs in a file the PR author can edit; and the JWT verifier must pin RS256 in code rather than read alg from the token header. It also documents an existing hole on the admin-token path, where a caller-supplied prUrl lets a publisher retarget another PR's pr-<n> dist-tag, and what the label gate does and does not bound. Status: Proposed. No implementation yet.
The alternatives section described pkg.pr.new as a GitHub App whose server pulls artifacts from the GitHub API, and rejected it on CPU grounds. That was wrong on both counts. Their CLI sends no credential and uploads tarballs directly; authorization comes from a publish window their App opens on workflow_run webhooks, keyed by a hash of public run metadata. Fork PRs work because nothing token-shaped is involved. Rebuilt the rejection on accurate grounds: the App private key and webhook secret reintroduce the stored credential this RFC removes. Also noted that their maintainers declined OIDC trusted publishing in issue #535 precisely because it loses fork-PR support, which independently confirms constraint 1, and that their README recommends an approval gate matching our preview-build label. Separately, the GHCR entry in accepted risks implied fork code could already reach ghcr.io/voidzero-dev/vite-plus:pr-<n>. It cannot: fork pull_request runs get a read-only GITHUB_TOKEN, so the push fails today. Moving the Docker job into the workflow_run leg is what makes it succeed. Recorded as open question 6 rather than a settled decision.
Adds SR-7: pin repository_id and repository_owner_id alongside workflow_ref. workflow_ref embeds a repository name, and names are mutable and reusable, so a rename, transfer, or deletion could later let a repo the org does not control satisfy a string match. Also pins owner id, because repository_id alone follows a repo transferred out of the org. Rewrites SR-6 from "bound the parsing" into a canonical archive policy: an explicit reject list (duplicate normalized paths, multiple package/package.json, traversal, absolute paths, non-file entry types, entry-count and size caps, entries outside package/), and a requirement that the trusted leg rebuild the tarball with its own codec and hash its own output rather than forwarding fork bytes. The duplicate-manifest case is the reason: tar permits duplicate entries and extractors disagree on precedence, so a validator reading the first entry can approve metadata pnpm never extracts. Emitting a fresh archive removes the ambiguity instead of trying to match every extractor. Extends SR-3 with parse bounds (token and decoded-segment size caps, kid length, strict three-segment split, strict Base64URL, claim type checks). Records commit-binding as open question 3 with a concrete mechanism (audience = <OIDC_AUDIENCE>#<sha>, bridge requires every published version to match), proposed for adoption: it limits authority rather than reuse, which jti replay protection does not. Notes in section 6 that since the trusted leg now rebuilds, pack mode could shrink to raw pnpm pack output and move the rewrite, pin, repack and hash to the trusted side. Left for the action PR; it changes the artifact contract between the two legs.
Implements RFC 0002 rollout step 1. The three publish endpoints (tarball upload, /-/publish, /-/register) now accept either the operator's admin token or a short-lived GitHub-signed OIDC token. /-/purge stays admin-only, so a CI identity can add preview builds and nothing else. src/security/oidc.ts verifies the token on WebCrypto rather than pulling a JWT library, which means owning the pitfalls explicitly (SR-3): RS256 is hardcoded on both the key import and the verify so the token header never selects an algorithm, the key comes from GitHub's JWKS by kid, and every length is capped before any parsing or crypto runs. Identity is anchored on the immutable repository_id and repository_owner_id ahead of workflow_ref (SR-7), because that claim embeds a repository name and names can be renamed, transferred, or released and reclaimed. All four OIDC vars are required together; a partial config throws 503 rather than silently disabling the path, so a deployment missing just the repository id fails loudly instead of rejecting every token as 401. SR-2, containing the pr-<n> dist-tag: /-/register rejects a prUrl outside the token's repository claim, and refuses to re-point an existing ref at a different prUrl. Note this is NOT the rule the RFC first proposed (rejecting a PR number already bound to another commit), which would have broken every multi-commit PR, since a PR accumulates one ref per pushed commit and that is how pr-<n> advances to the head build. A test covers that case next to the hijack case.
Implementing SR-2 showed the second bridge-side rule was wrong. Rejecting a prUrl whose PR number is already bound to a ref under a different commit would break every multi-commit PR: a PR accumulates one commit.<sha> ref per pushed commit, all sharing one prUrl, and that is exactly how latestVersionByPr advances pr-<n> to the PR's head build. Replaced with per-ref immutability: a given commit's prUrl cannot be rewritten, which still blocks dragging another PR's tag onto a commit without touching the legitimate case. Also states why the bridge cannot do better alone (verifying a commit belongs to a PR needs a GitHub API call, which belongs in the trusted leg that is already calling the API for SR-1).
Member
Author
|
@codex review |
fengmk2
force-pushed
the
rfc-oidc-trusted-publishing
branch
from
August 7, 2026 14:51
7b45888 to
e70e020
Compare
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
fengmk2
marked this pull request as ready for review
August 9, 2026 11:54
Member
Author
|
@codex review |
Implements RFC 0002 rollout step 2. The action gains a `mode` input:
publish (default) pack and upload in one job, admin-token, unchanged
pack raw `pnpm pack` output to output-dir, no network,
no credentials; safe in a job building a fork PR
upload read input-dir, validate, rebuild, publish; runs in
the trusted workflow_run leg
Settles the section 6 question in favour of the thinner build leg: `pack`
emits raw pnpm pack output and every transform whose result the bridge
trusts (version rewrite, batch pinning, repack, hash) now runs in the
trusted leg. The extra CPU there is free; the constraint in RFC 0001 was
the Worker's, never CI's.
src/tarball/validateArchive.ts is the SR-6 canonical policy: bounded
inflate against gzip bombs, entry-count and per-file caps, no duplicate
normalized paths, exactly one package/package.json, files and directories
only, nothing outside package/. Published bytes are always rebuilt through
buildPreviewTarball, so a crafted archive cannot make the validator and
pnpm disagree about which manifest is authoritative.
admin-token becomes optional; without it the action mints a GitHub Actions
OIDC token from the runner, cached and re-minted near expiry so a stalled
upload never sends an expired one, and dropped on a 401.
Two notes for reviewers. nanotar sanitizes entry names on both parse and
write (it resolves .. and strips /, C:/), so escaping paths are normally
refused for landing outside package/ rather than by our traversal check;
the check stays as the thing that refuses if that ever changes, and the
tests say which mechanism they exercise. And GitHub exposes inputs as
INPUT_<NAME> with dashes preserved, not underscored, which warm.mjs also
relies on.
Rewrites docs/ci-setup.md, which still described the admin-token single-job setup that no longer matches the code. Covers the build leg (mode: pack plus artifact upload), the authorize job, the trusted workflow_run leg, and the four bridge vars with vite-plus's real repository and owner ids. Calls out the parts that are easy to get wrong rather than only the happy path: the preview-build label is not the security boundary (a PR author can edit the build leg's own check, and workflow_run matches on workflow name), download-artifact's run-id defaults to the CURRENT run so omitting it looks in the wrong place, permissions are scoped per job so nothing that runs preview code can mint a token, and pr-url must come from the API because the bridge maps it to the pr-<n> tag that VP_PR_VERSION resolves. README gains a line noting the publish endpoints accept OIDC while /-/purge stays admin-only; self-hosting.md cross-references the full wiring. Also adds the config-state tests that were missing: with the OIDC vars entirely unset (the state right after this deploys, and what the staging smoke runs under) the admin token keeps working and a JWT is a plain 401; with the vars HALF set every publish 503s including the admin path, because the config resolves before the credential is routed. That last one is a deployment hazard worth having pinned by a test.
fengmk2
force-pushed
the
rfc-oidc-trusted-publishing
branch
from
August 9, 2026 12:14
b8825ef to
fd9ad0e
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8825ef5a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Four findings from the automated review, all valid. P1, tar extension records. parseTar handles extendedHeader, globalExtendedHeader and the GNU long-name types with `continue`: it decodes their payload into strings and drops them from the returned files, so the entry-count, per-file and entry-type checks never observed them. An archive could carry unlimited metadata records, or one claiming hundreds of megabytes, and reach large allocations inside the parser. Adds a raw 512-byte record scan that runs BEFORE parseTar and bounds every record including the hidden ones. Real npm tarballs do contain these records (a path over 100 bytes cannot fit the ustar name field), so they are bounded to 64KB rather than rejected. Also rejects the GNU base-256 size form and non-octal size fields. P1, OIDC in publish mode. The default publish mode reached the OIDC fallback whenever admin-token was omitted. That mode packs and uploads in one job, and `pnpm pack` runs the packed package's own prepack/prepare scripts, so it would put token minting in a job that executes packaged code, which is the separation SR-5 exists to keep. OIDC is now confined to upload mode and anything else fails with an explanatory error. P2, JWT-shaped admin tokens. looksLikeJwt routed on shape before the admin comparison ran, so an ADMIN_TOKEN containing three dot-separated segments would be sent to OIDC verification and rejected, breaking every publish and `pnpm warm` while still working on /-/purge. The constant-time admin comparison now runs first; shape only picks the path when the value is not the operator token. P2, stale pack output. pack mode created output-dir but never cleaned it, so a rerun producing fewer packages left higher pkg-<n> indices behind for the upload leg to republish under the new commit version or collide on. Adds prepareOutputDir, which removes only action-owned names and leaves anything else for the upload leg to refuse. One note on the entry-count message: the raw scan now rejects an oversized archive before assertCanonicalEntries sees it, so that assertion moved and the entry-level bound is covered by a direct test instead.
Closes the gap flagged on the SR-6 review thread. The tar header's mode is
attacker-controlled for any archive the bridge did not build, and it passed
straight through the repack, so a crafted tarball's setuid, setgid or sticky
bit survived into the published one. Small exposure, since npm and pnpm
apply their own modes on extract, but "the bytes we publish are ones we
constructed" is only true if the metadata is ours too.
Modes now collapse to 755 or 644, preserving the one bit that means anything
in an npm package (executable, which `bin` entries need) and discarding the
rest. Ownership is flattened to 0/0 with empty user and group: uid, gid,
user and group describe the packing machine, never anything a consumer
should honour.
Correcting my own reply on that thread: I said a crafted PAX record could
override `mode`. It cannot. nanotar builds attrs as
`{...globalExtendedHeader, ...nextExtendedHeader, mode, uid, ...}`, so the
header-derived values are assigned AFTER the spreads and win, and its writer
reads only those six fields, which makes any PAX-injected key inert. The
real exposure was narrower than I described: the tar header's own mode
field. Fixed either way.
mtime is deliberately unchanged. It is inert on extract, and pinning it
would alter the bytes of every package on the trusted publish path too.
Doing so would make republishes byte-identical, which is worth having for
reproducibility, but it is a separate change from this security fix.
Fixes an mtime bug found while implementing this. nanotar's parser returns mtime in SECONDS while its writer expects MILLISECONDS and divides by 1000, so passing parsed attrs straight back to the writer divided by 1000 twice: 1985-10-26 was written out as 1970-01-06. Every preview tarball published so far carries that mangled date. Entries now carry a fixed mtime of 499162500000ms (1985-10-26T08:15:00Z), which is what node-tar's `portable` mode stamps and therefore already the date on every entry `pnpm pack` produces. So repacking preserves the source date rather than inventing one, and an attacker-supplied mtime on the untrusted upload path is flattened to the same value. With mode, ownership and mtime all fixed, a rebuild is deterministic: identical content produces identical bytes. Verified that the gzip layer cooperates, since CompressionStream writes a zeroed MTIME into the gzip header rather than the current time. Republishing a commit therefore lands on the SAME content-addressed key instead of accumulating one object per run, which matters given the store is keyed by shasum. Correcting the previous commit's claim that pinning mtime would change the bytes of every package on the trusted publish path, and my reply on the review thread saying the same: the direction was right but the reason was not. The bytes change because the old code was mangling the date, not because pnpm pack disagreed with the constant. One consequence: republishing a commit published before this produces different bytes and a new CAS key. Harmless, since the packument advertises the new shasum and the old object expires with its ref, but it means an already-published version will not hash to its stored value.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1ac787d73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Cleanup pass from four parallel reviews (reuse, simplification, efficiency, altitude). No behavior change intended beyond the two noted below. Deduplication, which three of the four reviews flagged independently: - One definition of a tar entry path. `normalizeEntryName` moves into validateTarballPath.ts next to `assertSafeTarballPath` and `isUnderPackageRoot`, and both of those normalize internally. There were three spellings of "the same path" in the module whose job is to remove exactly that ambiguity. - One predicate for "is this the manifest". `isPackageManifest` replaces PACKAGE_JSON_NAMES plus a bolted-on normalized comparison; the archive policy no longer imports from the module it constrains. - One batch assertion. `resolveArchiveBatch` (was `batchFromArchives`) now collects manifests and calls `assertValidBatch` instead of keeping a second copy of the allowlist, duplicate-name and dependency rules, down to the error strings. PackageDir.dir becomes .label, since a tarball path is not a directory. - One request policy. `send()` holds the retry, auth-header and 401-invalidate handling that `uploadTarball` and `post` each had. Efficiency: - Memoize the JWKS and the imported CryptoKey per isolate. A publish run is ~23 authenticated requests sharing one unchanging key, each of which was doing a KV read, a JSON parse and an importKey. Keyed by modulus, not kid, so a rotated key reusing a kid cannot hit a stale entry. - Only prefetch one package ahead where reading is actually async. In upload mode `read` is readFileSync, so the prefetch returned an already-settled promise and just held a second archive resident through the heaviest allocation phase. Kept for publish mode, where packDirectory spawns a process and the overlap is real. - The two JWKS KV writes are independent; run them together. Simplification: verifyOidcToken returns only what callers read (repository, workflow_ref) instead of echoing back four values it just compared against constants; requirePublisher flattens to one guard chain; PackManifest.files duplicated packages[].file; dropped a dead mkdirSync import, an unused mintOidcToken export, and an identity wrapper in the tests. Two deliberate deviations from the review suggestions. The path helpers were proposed to REQUIRE pre-normalized input; they normalize internally instead, because a future caller who forgets must not silently get a weaker answer from a security check, and normalizeEntryName is idempotent so the extra pass is free. And the entry-level count/size bounds stay even though the raw record scan now reaches them first, so assertCanonicalEntries remains valid called on its own. Also replaced a 1.1s wall-clock sleep in the reproducibility test with a direct assertion that the gzip header MTIME is zeroed, which is what the sleep was indirectly probing.
Five findings, all valid. Verified each against nanotar's source and, for the two that turned on its exact behaviour, empirically. P1, tar size fields. The raw scanner stopped at the first space, while nanotar's _readNumber passes the whole field to parseInt, which skips leading whitespace. So ` 2000000000\0` read as 0 here and as 256MiB there: the scanner advanced one block into an attacker-controlled payload, hit a zero byte, treated it as end-of-archive, and left the rest of the archive unbounded, defeating the metadata-record bound added last commit. The field is now parsed exactly as nanotar parses it, with a separate strict check that it is octal digits plus space/NUL padding. P2, long paths. nanotar's writer encodes the name into the 100-byte ustar field and emits neither a prefix nor a long-name record, so a longer path is silently TRUNCATED on rebuild. Measured: `package/<100 a's>/index.js` comes back as 96 bytes, and a sibling sharing that prefix comes back as the SAME name, so two files become one and the duplicate-path invariant is defeated after validation passed. This is pre-existing and affects `publish` mode in production today, not just the new upload path, so the check lives in buildPreviewTarball where it covers both. Refusing is the only honest option while the writer cannot represent them: a truncated path is a broken package. If a real package ever hits this, the fix is a writer that emits PAX, not a higher limit. P1, compressed size. readFileSync materialized the whole tarball before gunzipBounded could bound the inflated size, so a multi-gigabyte pkg-0.tgz exhausted the runner first. Now bounded from the directory entry we already lstat, at 128MB against a ~23MB largest real package. P1, manifest retention. The per-file limit is sized for binaries, so a hostile artifact could ship several highly compressible 256MB package.json files, each parsed and retained for the whole batch scan. Bounded to 4MB and projected to just the name and dependency keys the batch check reads; buildPreviewTarball re-reads the manifest from the archive anyway. P2, package count. The artifact reader accepted unlimited pkg-<n>.tgz, so a modified build workflow could hand the trusted leg tens of thousands of small archives. Capped at 128 against a batch of ~11.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
voidzero-dev/pkg-pr-registry-bridge#86 is merged and deployed, so both pins move from that PR's branch head to the resulting main commit and the TODO comments go away. Worth noting the old pin was stale even against the branch: fd9ad0e predated the simplify pass and the second review round, so it would have run the action without the SR-6 hardening (tar size-field agreement, artifact size and count bounds, the long-path refusal). Verified action.yml and the bundled dist resolve at the new SHA.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
Splits preview publishing into an untrusted build leg and a trusted publish leg so external contributors' PRs can be installed and reviewed. Until now the bridge step was gated on the PR coming from this repo, because GitHub withholds secrets from fork pull_request runs. Fork PRs therefore got no preview build, no install instructions, and no Docker image, which is most of the value of the preview flow for exactly the contributions that need review most. publish-preview.yml keeps building, but now only packs (`mode: pack`, no network, no credentials) and uploads a workflow artifact. The new publish-preview-register.yml triggers on it completing, runs from main in base-repo context, and mints a GitHub Actions OIDC token that the bridge verifies against GitHub's JWKS. No bridge secret in this repo. The `authorize` job is the security boundary, not the label check in the build leg: on pull_request events GitHub runs the workflow file from the merge ref, so a PR author can edit that file to delete its own gate, or add a workflow with a matching name to trigger the trusted leg. So `authorize` re-resolves the PR from workflow_run.head_sha via the API, requires it to be open against this repo and currently labeled preview-build, and fails closed otherwise. It also supplies the PR url, which the bridge maps to the pr-<n> dist-tag; taking that from the artifact would let one PR retarget what VP_PR_VERSION installs for another. The job holding id-token does nothing but move bytes. The comment and Docker jobs are separate jobs with their own permissions, so the job that runs a preview package's install scripts cannot mint a publish token. The Docker preview stays same-repo only for now. It installs the preview package and pushes to ghcr.io/voidzero-dev/vite-plus, and before this change a fork could not reach it (fork runs get a read-only token, so the push failed). Running in base-repo context is what would make it succeed, so whether to put unreviewed fork code under the org's namespace is left as a deliberate decision: drop the is-fork condition to enable it. Fork builds get a warning banner naming the source fork above the install instructions, since those lines are official-looking and end in `curl | bash`. Requires on the bridge side (voidzero-dev/pkg-pr-registry-bridge#86): OIDC_AUDIENCE, OIDC_TRUSTED_WORKFLOWS, OIDC_TRUSTED_REPOSITORY_ID and OIDC_TRUSTED_OWNER_ID set on the production Worker. The action pin here points at that PR's head and must be repointed to a main SHA before this merges.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
voidzero-dev/pkg-pr-registry-bridge#86 is merged and deployed, so both pins move from that PR's branch head to the resulting main commit and the TODO comments go away. Worth noting the old pin was stale even against the branch: fd9ad0e predated the simplify pass and the second review round, so it would have run the action without the SR-6 hardening (tar size-field agreement, artifact size and count bounds, the long-path refusal). Verified action.yml and the bundled dist resolve at the new SHA.
fengmk2
added a commit
that referenced
this pull request
Aug 10, 2026
The four OIDC_* vars were declared in src/config.ts's Env interface but never in env.ts or .env.production, so they were absent at runtime and the OIDC path has been disabled since #86 deployed. They are not secrets: all four hold public identifiers and the verification key is GitHub's public JWKS, so per the README's configuration model they belong in env.ts plus the committed .env.production, not `void secret put`. Declared optional as a group. All four unset disables OIDC and leaves admin-token publishing untouched; setting only some is already rejected at request time with a 503 naming the missing var. Recorded alongside the values: `pnpm deploy:staging` runs the same `void deploy`, so staging inherits .env.production (which is also why PUBLIC_BASE_URL there points at prod) and shares this audience and allowlist. Not an escalation while both are identical, but staging cannot be given a looser allowlist for testing without that workflow also being able to publish to production.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
Splits preview publishing into an untrusted build leg and a trusted publish leg so external contributors' PRs can be installed and reviewed. Until now the bridge step was gated on the PR coming from this repo, because GitHub withholds secrets from fork pull_request runs. Fork PRs therefore got no preview build, no install instructions, and no Docker image, which is most of the value of the preview flow for exactly the contributions that need review most. publish-preview.yml keeps building, but now only packs (`mode: pack`, no network, no credentials) and uploads a workflow artifact. The new publish-preview-register.yml triggers on it completing, runs from main in base-repo context, and mints a GitHub Actions OIDC token that the bridge verifies against GitHub's JWKS. No bridge secret in this repo. The `authorize` job is the security boundary, not the label check in the build leg: on pull_request events GitHub runs the workflow file from the merge ref, so a PR author can edit that file to delete its own gate, or add a workflow with a matching name to trigger the trusted leg. So `authorize` re-resolves the PR from workflow_run.head_sha via the API, requires it to be open against this repo and currently labeled preview-build, and fails closed otherwise. It also supplies the PR url, which the bridge maps to the pr-<n> dist-tag; taking that from the artifact would let one PR retarget what VP_PR_VERSION installs for another. The job holding id-token does nothing but move bytes. The comment and Docker jobs are separate jobs with their own permissions, so the job that runs a preview package's install scripts cannot mint a publish token. The Docker preview stays same-repo only for now. It installs the preview package and pushes to ghcr.io/voidzero-dev/vite-plus, and before this change a fork could not reach it (fork runs get a read-only token, so the push failed). Running in base-repo context is what would make it succeed, so whether to put unreviewed fork code under the org's namespace is left as a deliberate decision: drop the is-fork condition to enable it. Fork builds get a warning banner naming the source fork above the install instructions, since those lines are official-looking and end in `curl | bash`. Requires on the bridge side (voidzero-dev/pkg-pr-registry-bridge#86): OIDC_AUDIENCE, OIDC_TRUSTED_WORKFLOWS, OIDC_TRUSTED_REPOSITORY_ID and OIDC_TRUSTED_OWNER_ID set on the production Worker. The action pin here points at that PR's head and must be repointed to a main SHA before this merges.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
voidzero-dev/pkg-pr-registry-bridge#86 is merged and deployed, so both pins move from that PR's branch head to the resulting main commit and the TODO comments go away. Worth noting the old pin was stale even against the branch: fd9ad0e predated the simplify pass and the second review round, so it would have run the action without the SR-6 hardening (tar size-field agreement, artifact size and count bounds, the long-path refusal). Verified action.yml and the bundled dist resolve at the new SHA.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
Splits preview publishing into an untrusted build leg and a trusted publish leg so external contributors' PRs can be installed and reviewed. Until now the bridge step was gated on the PR coming from this repo, because GitHub withholds secrets from fork pull_request runs. Fork PRs therefore got no preview build, no install instructions, and no Docker image, which is most of the value of the preview flow for exactly the contributions that need review most. publish-preview.yml keeps building, but now only packs (`mode: pack`, no network, no credentials) and uploads a workflow artifact. The new publish-preview-register.yml triggers on it completing, runs from main in base-repo context, and mints a GitHub Actions OIDC token that the bridge verifies against GitHub's JWKS. No bridge secret in this repo. The `authorize` job is the security boundary, not the label check in the build leg: on pull_request events GitHub runs the workflow file from the merge ref, so a PR author can edit that file to delete its own gate, or add a workflow with a matching name to trigger the trusted leg. So `authorize` re-resolves the PR from workflow_run.head_sha via the API, requires it to be open against this repo and currently labeled preview-build, and fails closed otherwise. It also supplies the PR url, which the bridge maps to the pr-<n> dist-tag; taking that from the artifact would let one PR retarget what VP_PR_VERSION installs for another. The job holding id-token does nothing but move bytes. The comment and Docker jobs are separate jobs with their own permissions, so the job that runs a preview package's install scripts cannot mint a publish token. The Docker preview stays same-repo only for now. It installs the preview package and pushes to ghcr.io/voidzero-dev/vite-plus, and before this change a fork could not reach it (fork runs get a read-only token, so the push failed). Running in base-repo context is what would make it succeed, so whether to put unreviewed fork code under the org's namespace is left as a deliberate decision: drop the is-fork condition to enable it. Fork builds get a warning banner naming the source fork above the install instructions, since those lines are official-looking and end in `curl | bash`. Requires on the bridge side (voidzero-dev/pkg-pr-registry-bridge#86): OIDC_AUDIENCE, OIDC_TRUSTED_WORKFLOWS, OIDC_TRUSTED_REPOSITORY_ID and OIDC_TRUSTED_OWNER_ID set on the production Worker. The action pin here points at that PR's head and must be repointed to a main SHA before this merges.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
voidzero-dev/pkg-pr-registry-bridge#86 is merged and deployed, so both pins move from that PR's branch head to the resulting main commit and the TODO comments go away. Worth noting the old pin was stale even against the branch: fd9ad0e predated the simplify pass and the second review round, so it would have run the action without the SR-6 hardening (tar size-field agreement, artifact size and count bounds, the long-path refusal). Verified action.yml and the bundled dist resolve at the new SHA.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
Splits preview publishing into an untrusted build leg and a trusted publish leg so external contributors' PRs can be installed and reviewed. Until now the bridge step was gated on the PR coming from this repo, because GitHub withholds secrets from fork pull_request runs. Fork PRs therefore got no preview build, no install instructions, and no Docker image, which is most of the value of the preview flow for exactly the contributions that need review most. publish-preview.yml keeps building, but now only packs (`mode: pack`, no network, no credentials) and uploads a workflow artifact. The new publish-preview-register.yml triggers on it completing, runs from main in base-repo context, and mints a GitHub Actions OIDC token that the bridge verifies against GitHub's JWKS. No bridge secret in this repo. The `authorize` job is the security boundary, not the label check in the build leg: on pull_request events GitHub runs the workflow file from the merge ref, so a PR author can edit that file to delete its own gate, or add a workflow with a matching name to trigger the trusted leg. So `authorize` re-resolves the PR from workflow_run.head_sha via the API, requires it to be open against this repo and currently labeled preview-build, and fails closed otherwise. It also supplies the PR url, which the bridge maps to the pr-<n> dist-tag; taking that from the artifact would let one PR retarget what VP_PR_VERSION installs for another. The job holding id-token does nothing but move bytes. The comment and Docker jobs are separate jobs with their own permissions, so the job that runs a preview package's install scripts cannot mint a publish token. The Docker preview stays same-repo only for now. It installs the preview package and pushes to ghcr.io/voidzero-dev/vite-plus, and before this change a fork could not reach it (fork runs get a read-only token, so the push failed). Running in base-repo context is what would make it succeed, so whether to put unreviewed fork code under the org's namespace is left as a deliberate decision: drop the is-fork condition to enable it. Fork builds get a warning banner naming the source fork above the install instructions, since those lines are official-looking and end in `curl | bash`. Requires on the bridge side (voidzero-dev/pkg-pr-registry-bridge#86): OIDC_AUDIENCE, OIDC_TRUSTED_WORKFLOWS, OIDC_TRUSTED_REPOSITORY_ID and OIDC_TRUSTED_OWNER_ID set on the production Worker. The action pin here points at that PR's head and must be repointed to a main SHA before this merges.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
voidzero-dev/pkg-pr-registry-bridge#86 is merged and deployed, so both pins move from that PR's branch head to the resulting main commit and the TODO comments go away. Worth noting the old pin was stale even against the branch: fd9ad0e predated the simplify pass and the second review round, so it would have run the action without the SR-6 hardening (tar size-field agreement, artifact size and count bounds, the long-path refusal). Verified action.yml and the bundled dist resolve at the new SHA.
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
1 participant
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.
RFC 0002 and its bridge-side and action-side implementation. Consumer wiring is voidzero-dev/vite-plus#2387, which depends on this.
Problem
The publish path requires
Authorization: Bearer <ADMIN_TOKEN>, and GitHub withholds secrets from forkpull_requestruns. vite-plus'spublish-preview.ymltherefore skips the bridge step for external contributors, so reviewers cannot install or smoke-test a third-party PR.Approach
Verify GitHub Actions OIDC tokens against GitHub's JWKS instead of a shared secret. Fork
pull_requestruns cannot mint OIDC tokens either, so publishing splits into an untrusted build leg (pull_request, no credentials, packs an artifact) and a trusted publish leg (workflow_run, base-repo context, mints the token). After rollout no bridge secret lives in the consumer repo;ADMIN_TOKENstays for/-/purge,warm, and ops.Contents
rfcs/0002-*.md, including the security section and accepted risks.src/security/oidc.tsplus wiring on the three publish endpoints./-/purgestays admin-only. RS256 is hardcoded on both key import and verify so the token header never selects an algorithm; key bykidfrom GitHub's JWKS (KV-cached, unknown-kidrefetch behind a cooldown); parse bounds before any crypto; identity anchored on the immutablerepository_id/repository_owner_idahead ofworkflow_ref.modeinput:publish(unchanged),pack(no network, no credentials),upload(validate, rebuild, publish).src/tarball/validateArchive.tsis the SR-6 canonical policy.admin-tokenis now optional; without it the action mints an OIDC token.ci-setup.mdrewritten for the two legs; README andself-hosting.mdupdated.177 tests (81 new), typecheck clean, no action-bundle drift.
Security requirements
Section 8 records seven requirements found while reviewing the design, plus what the
preview-buildlabel does and does not bound. Two are load-bearing rather than defense-in-depth:head_shaand re-check the label via the API. Onpull_requestevents GitHub runs the workflow file from the merge ref, so the build leg's own label check sits in a file the PR author can edit, andworkflow_runmatches on workflow name. Without SR-1 an unlabeled fork PR publishes to production. Implemented in #2387.algfrom the header. Analg: noneacceptance is a full bypass for anyone on the internet.It also fixes a hole that predates this RFC:
/-/registertookprUrlfrom the caller andgetConfiguredRefs.ts:78derivesprNumberfrom it, so any publisher could retarget another PR'spr-<n>dist-tag and change whatVP_PR_VERSION=<n>installs.Two corrections worth reading
SR-2 changed during implementation. The RFC first proposed rejecting a
prUrlwhose PR number was already bound to another commit. That would have broken every multi-commit PR, since a PR accumulates one ref per pushed commit sharing oneprUrl, and that is exactly howpr-<n>advances to the head build. Replaced with per-ref immutability, with a test for the legitimate case beside the hijack case (commite70e020).The pkg.pr.new comparison was wrong. I had described it as an App whose server pulls artifacts from the GitHub API and rejected it on CPU grounds. Their CLI sends no credential at all and uploads directly; authorization comes from a publish window their App opens on
workflow_runwebhooks. Rebuilt the rejection on accurate grounds (commitf9ec9e1). Their maintainers declined OIDC in stackblitz-labs/pkg.pr.new#535 precisely because it loses fork-PR support, which independently confirms the constraint driving the split.Deploying this is safe on its own
With the OIDC vars unset, the admin-token path is unchanged and a JWT is a plain 401; there are tests for that state, and the staging job runs under it. vite-plus pins the action to an older commit, so merging this does not change what it executes.
One hazard: the config resolves before the credential is routed, so setting the four vars partially returns 503 for every publish including admin-token ones. It fails loudly and names the missing var, but the four should go in as a single change. Also covered by a test.
Review asks
workflow_runworkflow?workflow_runonly fires for files already on the default branch, so SR-1 cannot get a live negative test until #2387 merges.