Set the OIDC publishing vars for production - #90
Merged
Merged
Conversation
These were declared in src/config.ts's Env interface but never in env.ts or .env.production, so they were simply absent at runtime and the OIDC path has been disabled since deploy. The post-deploy check I ran (a JWT-shaped token returning 401) was consistent with that and I read it as "correctly unconfigured" rather than "not configured at all". They are NOT secrets, so they do not need `void secret put` and they do not need anyone's Void auth: all four hold public identifiers, and the verification key is GitHub's public JWKS. Per README, non-secret values are declared in env.ts and set in the committed .env files. This should have been part of #86. Declared optional as a group: all four unset disables OIDC and leaves admin-token publishing untouched, which is the state production is in today. Setting only some is already rejected at request time. One wrinkle recorded next to 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). Staging therefore shares this audience and allowlist. Not an escalation while both are identical, but it does mean staging cannot be given a looser allowlist for an OIDC smoke test without that workflow also being able to publish to production.
fengmk2
added a commit
that referenced
this pull request
Aug 10, 2026
Found by probing production after #90 turned OIDC on: `aaa.bbb.ccc` answered 500 Internal error instead of 401. "aaa" is a legal base64url segment, but it decodes to 0x69 0xa6, which is invalid UTF-8, and TextDecoder({fatal: true}) throws a plain TypeError. That escaped decodeJsonSegment (only JSON.parse was guarded), missed the HttpError branch in the router, and became a 500. The base64url alphabet says nothing about whether the decoded bytes are valid UTF-8, so this is reachable from any well-formed-looking token. No security impact, the token is rejected either way, but an unauthenticated caller could produce 500s at will, which is noise in error monitoring and the wrong signal about which layer refused. The tests missed it because every fixture segment held valid UTF-8. Added one that does not. Also guards atob, though that path is not reachable today; marked defensive rather than covered by a test asserting a case that cannot occur.
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.
Fixes an omission from #86: the four
OIDC_*vars were declared insrc/config.ts'sEnvinterface but never inenv.tsor.env.production, so they are absent at runtime and the OIDC path has been disabled since deploy.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.tsplus the committed.env.production, notvoid secret put. I had been describing this as needing Void auth, which was wrong; it is a code change and should have shipped with #86.Declared optional as a group. All four unset disables OIDC and leaves admin-token publishing untouched, which is production's current state. Setting only some is already rejected at request time with a 503 naming the missing var.
The numeric ids are what anchor trust, since
OIDC_TRUSTED_WORKFLOWSembeds a repository name and names can be renamed, transferred, or reclaimed. Read back fromgh api repos/voidzero-dev/vite-plus.One wrinkle worth knowing
pnpm deploy:stagingruns the samevoid deploy, so staging inherits.env.production— which is also whyPUBLIC_BASE_URLthere already points at prod. Staging will therefore share this audience and allowlist.That is not an escalation while the two are identical: only the vite-plus register workflow can mint an acceptable token either way. But it does mean staging cannot be given a looser allowlist to smoke-test OIDC without that workflow also being able to publish to production, so the "test OIDC against staging first" plan I suggested does not work as described. Recorded in a comment next to the values rather than left to be rediscovered.
Effect of merging
Deploying this turns OIDC on in production. Nothing changes for existing publishes: vite-plus
mainstill uses the admin-token path, and the admin path is unaffected either way. It simply makes voidzero-dev/vite-plus#2387 able to work once merged.205 tests pass, typecheck clean.