fix(site): write supporter register to staging branch - #319
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Keep bot writes off the protected default branch by targeting the configured register branch, and include the bot DCO trailer required by repository policy. Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> Assisted-by: AI
Run the allowlist-only register publisher from this repository so the private token is used only to read the supporters branch and public publication uses GITHUB_TOKEN. Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> Assisted-by: AI
Retain the local handoff report in the worktree without shipping it in the pull request. Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> Assisted-by: AI
Collect consented public entries in one pass so the register publisher satisfies the React Doctor performance check without changing its output. Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> Assisted-by: AI
The publisher now resets its branch from the chosen base, opens or updates a fixed-title PR, and requests squash auto-merge instead of pushing to the protected default branch. Runtime provider tests exercise the GitHub branch paths and docs name the publisher's private read scope. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
tnunamak
force-pushed
the
fix/register-bot-branch
branch
from
September 3, 2026 22:27
f419a58 to
a6ad599
Compare
State that github-actions[bot] reads only the private signatures checkout through the publication token, so the register access model matches the workflow and access table. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Keep generated publication on a fixed branch, require manual squash merge, document the private token authority, and make the provider regression tests satisfy the site verification gate. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
tnunamak
added a commit
that referenced
this pull request
Sep 4, 2026
Six corrections from an independent review of the oracle added in the previous commit. Two of them are the difference between an assertion that proves something and one that does not. The withdrawal step could pass on a commit from an earlier run. `waitForCommit` matched on a commit subject with no recency filter at all. Two of the three subjects it waits for embed a per-run uuid and so were run-unique by accident of how the app names them, but the third, "Record a withdrawal", is a constant the app writes for every withdrawal by anybody. Any earlier withdrawal sitting in recent history satisfied it on the first poll, so step 6 could report a pass having proven nothing about this run -- which is precisely the vacuous pass the step exists to rule out. Every wait now takes the `since` timestamp already computed for the mailbox filter, applied both as a `since` parameter to the API and as a per-candidate committer-date check, so a stale entry cannot slip through even if the parameter were ignored. A commit with no committer date is rejected rather than assumed recent. The Signed-off-by assertion was documented backwards. The header said the trailer came from the private repo's "require sign-off on web-based commits" setting and that asserting it here was the only check on that policy. It is added by the site, in `botCommitMessage`, and every write goes through it -- which landed on main in #319, after this branch was cut. The assertion still catches the trailer going missing, but it checks app code this repo owns, not an out-of-band setting, and a maintainer told otherwise would look in the wrong place. The comment now says what it checks, and says plainly that nothing here checks the repo policy. This commit is rebased onto main so the claim matches the tree it is made against. Four smaller corrections. Exit code 2 was undocumented and was the one path that wrote no receipt, so a job keying off the receipt could not tell a missing environment variable from a crash; that check moved inside the run and now records a named `setup` failure and exits 1 like everything else. The stored address was compared against the raw SIGNING_TEST_EMAIL, but the schema lowercases it, so any operator whose address has a capital in it saw a correct write reported as a broken one -- the oracle now normalises the same way the schema does. Step 4 built the signatory file path from the wall clock while the app files it under the year of the confirmation, which disagree for one second a year across a UTC new year; the path is now read out of the commit that wrote it, which also makes the step prove the commit added a file for this signatory rather than assuming it. And a mailbox failure wrote `execFile`'s error message into the receipt, which contains the entire command line: the reader is operator-supplied and documented as swappable, so a replacement invoked with a credential in argv would put it on disk. Only the program name and the exit status now cross into the receipt. The receipt also records the confirm and withdraw links, redacted to origin, path and an eight-character token fingerprint. Under `--keep` the entry is deliberately left live and withdrawable only from the email, so the receipt is the natural second copy of that link; the tokens themselves stay off disk because they are live single-use credentials. Two documentation gaps the review found are closed in the header: the site allows five submissions per IP per hour, which matters for a script whose purpose is repeated runs from one address, and the 429 message now says how long to wait; and `sanitizeUrl` is noted as not origin-safe on its own, since what rejects a lookalike host is `extractLinks` searching for the origin as a literal prefix. Sixteen tests added, 28 passing. Each was written failing first: the recency filter is tested against a commit that predates the run, the path derivation against a record confirmed in one year and read in the next, and the redaction helpers against a reader invoked with a password in its arguments. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
tnunamak
added a commit
that referenced
this pull request
Sep 4, 2026
) * test: walk the Supporter signing journey against a real deployment The Supporter signing flow (the form on /principles that records someone as having signed the PDPP Principles) crosses four systems that no unit test can join up: a Next.js route handler, a key-value store holding the unconfirmed submission, a transactional mail provider, and a write to a private GitHub repository that holds the signatory records. Each seam is mocked in isolation, and mocks encode the same assumptions as the code that calls them. Until now nothing exercised the seams between them, which the signing module itself says out loud: "Nothing in this file is exercised by a test or a live run yet." The failures that class of gap hides are the ones that matter here. A confirmation email whose links point at a different deployment still looks correct. A token that verifies but whose pending record was never stored still looks correct. A signatory file that lands on a branch nobody publishes from still looks correct. Every one passes a green unit suite and breaks the real journey for a real person trying to sign. So this adds scripts/signing-journey-oracle.mjs, which walks the journey the way a supporter does, against a deployment you name, and asserts the observable outcome of each step: submit and expect a 303 redirect carrying signed=pending; poll a mailbox for the confirmation; follow the confirm link and expect signed=confirmed; check that GitHub received an "Add signatory <id>" commit whose stored record matches what the form sent; follow the confirm link a second time and expect signed=invalid, proving the link is single use; then withdraw, and require the commits that delete the record and log the withdrawal. It writes a JSON receipt naming every step's status and exits non-zero on any failure, so a failed run says how far the journey got rather than only that it broke. Two details are worth knowing because they are not visible in the diff. The mailbox is read through a swappable command template rather than a Gmail library, because the default reader is one maintainer's authenticated `gog` CLI and anyone else has a different one. And the default reader is `gog gmail messages search` rather than `gog gmail search`: the latter returns thread identifiers, Gmail threads every confirmation together under one subject, and fetching a thread yields its oldest message — so a thread-level reader hands the oracle a stale email from an earlier run every time. That was observed during development, not theorised. Every extracted link is filtered on an origin equal to the deployment under test. Preview deployments share one mailbox, so without that filter this could pass while testing a build nobody asked for. The accompanying test covers the link sanitiser and the receipt shape — the two pieces that can be checked without a live deployment. The sanitiser is worth isolating because all its failure modes are silent: a token corrupted by a mail transport that hard-wraps a long line still looks like a link, and fails later as an "invalid" outcome that would be misreported as a broken signing flow rather than a broken parse. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> * test: bound the signing oracle's commit assertions to the run under test Six corrections from an independent review of the oracle added in the previous commit. Two of them are the difference between an assertion that proves something and one that does not. The withdrawal step could pass on a commit from an earlier run. `waitForCommit` matched on a commit subject with no recency filter at all. Two of the three subjects it waits for embed a per-run uuid and so were run-unique by accident of how the app names them, but the third, "Record a withdrawal", is a constant the app writes for every withdrawal by anybody. Any earlier withdrawal sitting in recent history satisfied it on the first poll, so step 6 could report a pass having proven nothing about this run -- which is precisely the vacuous pass the step exists to rule out. Every wait now takes the `since` timestamp already computed for the mailbox filter, applied both as a `since` parameter to the API and as a per-candidate committer-date check, so a stale entry cannot slip through even if the parameter were ignored. A commit with no committer date is rejected rather than assumed recent. The Signed-off-by assertion was documented backwards. The header said the trailer came from the private repo's "require sign-off on web-based commits" setting and that asserting it here was the only check on that policy. It is added by the site, in `botCommitMessage`, and every write goes through it -- which landed on main in #319, after this branch was cut. The assertion still catches the trailer going missing, but it checks app code this repo owns, not an out-of-band setting, and a maintainer told otherwise would look in the wrong place. The comment now says what it checks, and says plainly that nothing here checks the repo policy. This commit is rebased onto main so the claim matches the tree it is made against. Four smaller corrections. Exit code 2 was undocumented and was the one path that wrote no receipt, so a job keying off the receipt could not tell a missing environment variable from a crash; that check moved inside the run and now records a named `setup` failure and exits 1 like everything else. The stored address was compared against the raw SIGNING_TEST_EMAIL, but the schema lowercases it, so any operator whose address has a capital in it saw a correct write reported as a broken one -- the oracle now normalises the same way the schema does. Step 4 built the signatory file path from the wall clock while the app files it under the year of the confirmation, which disagree for one second a year across a UTC new year; the path is now read out of the commit that wrote it, which also makes the step prove the commit added a file for this signatory rather than assuming it. And a mailbox failure wrote `execFile`'s error message into the receipt, which contains the entire command line: the reader is operator-supplied and documented as swappable, so a replacement invoked with a credential in argv would put it on disk. Only the program name and the exit status now cross into the receipt. The receipt also records the confirm and withdraw links, redacted to origin, path and an eight-character token fingerprint. Under `--keep` the entry is deliberately left live and withdrawable only from the email, so the receipt is the natural second copy of that link; the tokens themselves stay off disk because they are live single-use credentials. Two documentation gaps the review found are closed in the header: the site allows five submissions per IP per hour, which matters for a script whose purpose is repeated runs from one address, and the 429 message now says how long to wait; and `sanitizeUrl` is noted as not origin-safe on its own, since what rejects a lookalike host is `extractLinks` searching for the origin as a literal prefix. Sixteen tests added, 28 passing. Each was written failing first: the recency filter is tested against a commit that predates the run, the path derivation against a record confirmed in one year and read in the next, and the redaction helpers against a reader invoked with a password in its arguments. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> --------- Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
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.
A supporter who opens the website’s emailed confirmation link, the link that finalizes a pending supporter signature by creating its private record, could receive a failure if the private GitHub repository that stores supporter records uses its default Git branch. The daily GitHub Actions job, GitHub’s hosted workflow runner for repository automation, that copies safe fields into the public supporter list could also be blocked if it wrote directly to the protected default branch,
main.The website now sends every private repository file mutation through GitHub’s Contents API, the endpoint used here to create or delete files, to
PDPP_PRIVATE_REPO_BRANCH, the deployment environment variable that names the target private-repository branch; it defaults tosignatures. A failed private write becomesSigningUnavailableError, the website error type that returns a safe unavailable-service response instead of exposing the GitHub failure. The public publisher, the GitHub Actions job that copies onlycountry,principlesVersion,publicName,signedOn, andtype, checks out the privatesignaturesbranch and opens a pull request that updates only the generated public supporter register rather than writingmain.Round 2
The workflow resets its generated-output branch from the selected base branch, creates a signed bot commit, and force-pushes only that branch. Provider tests, the tests for the website module that writes supporter records to GitHub, mock GitHub API responses. They prove that the Contents API PUT request carries the configured branch and Developer Certificate of Origin (DCO) sign-off trailer, that an unset branch chooses
signatures, and that withdrawal, the operation that removes a previously confirmed supporter record, does not search possible record-file paths after the configured branch lookup fails. The public-register test fixture, a sample private supporter record, includes an IP address and nested private data but asserts that neither reaches the five named public fields.Round 3
The site verification command,
pnpm --filter pdpp-site verify, runs type checking, all site tests, source-style checks, generated-artifact validation, and the production build. It previously failed because the new provider tests violated five source-style rules: the repository’s automated checks for import order, destructuring, top-level regular expressions, formatter output, and related formatting constraints. Those tests retain the same branch-failure scenarios and now satisfy those rules; the full command passes.publish/supporters, the fixed branch that holds generated public-register changes, is no longer a manual-dispatch input. Before checkout, the workflow compares that publisher branch,publish/supporters, with the repository default branch, the branch GitHub treats as its main merge target, and fails with a clear error if they are equal; publication therefore cannot be directed tomainor another arbitrary branch. The publisher only opens or updates its fixed-title pull request, the single reusable PR used for generated public-register updates. After the repository’s required pull-request checks pass, a maintainer uses GitHub’s Squash and merge control to merge that pull request as one commit. The workflow makes no auto-merge request because the repository disables auto-merge.The credential the workflow uses to read the private supporter repository is
PDPP_PRIVATE_REPO_TOKEN: a fine-grained personal access token, a GitHub token restricted to selected repository permissions, owned by maintainer accounttnunamak. It is scoped toPDP-Connect/supporters-privatewith Contents read and write and is stored as thepdpprepository Actions secret. The token itself is neither branch-limited nor read-only; the workflow limits its own checkout tosignatures, does not persist its credential, and has no private-repository write step.actionlint, the GitHub Actions workflow validator, accepted the workflow. The branch guard acceptedpublish/supportersand rejectedmain. PR #319's checks are green.Assisted-by: AI