Skip to content

Dispatch PR previews to paired Web Simulator - #440

Open
schnuartz-ai wants to merge 9 commits into
cryptoadvance:masterfrom
schnuartz-ai:codex/paired-preview-dispatch
Open

schnuartz-ai wants to merge 9 commits into
cryptoadvance:masterfrom
schnuartz-ai:codex/paired-preview-dispatch

Conversation

@schnuartz-ai

@schnuartz-ai schnuartz-ai commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Move browser-preview execution and hosting into the paired [Specter DIY Web Simulator PR #8](cryptoadvance/specter-diy-web-simulator#8).

This PR introduces a small, trusted GitHub Actions dispatcher in Specter DIY. It validates the live pull request, dispatches its exact metadata to the Web Simulator, and exits immediately.

There is no polling, long-running privileged workflow, or PR-comment write access on the Specter DIY side.

Architecture

  1. A Specter DIY pull request triggers the pull_request_target workflow.
  2. The dispatcher checks out only trusted default-branch code.
  3. It validates the live PR and exact head SHA, base SHA, repositories, branches and request metadata.
  4. It starts the paired Web Simulator using workflow_dispatch and a unique request ID.
  5. The Specter workflow exits immediately.
  6. The Web Simulator independently builds and verifies the preview, publishes GitHub Pages and updates the PR through a dedicated machine-user account.

The Web Simulator maintains a single persistent PR comment. Existing comments are updated in place instead of deleted and recreated.

The comment includes build status, collapsible provenance, simulator URL, matching firmware artifact, build logs and the experimental-build warning.

Closing a PR dispatches a cleanup request for previews, firmware artifacts and the machine user's own marked comment.

Security

  • The dispatcher never checks out or executes untrusted PR-controlled code.
  • The Specter workflow uses only contents: read and pull-requests: read.
  • WEB_SIMULATOR_DISPATCH_TOKEN requires only Actions: Read and write access to the paired Web Simulator repository.
  • The dispatcher has no Issues or Pull Requests write permissions.
  • PR title/body edits do not trigger additional preview builds.
  • Exact SHA and live-PR validation protect against stale or mismatched requests.
  • The Web Simulator independently revalidates requests before building, publishing and reporting.
  • The machine-user token exists only in the trusted Web Simulator finalizer, never in untrusted build jobs.

The dedicated account specter-preview-bot must have no repository write permissions.

GitHub enforces comment ownership for this account. It may update/delete its own comments but cannot modify third-party comments or PR metadata without additional repository permissions.

The classic PAT uses public_repo, which is not a comment-only permission scope. The account must therefore remain dedicated to this service and without elevated repository roles.

Configuration

Before production activation:

  1. Merge and configure [Web Simulator PR #8](Move PR preview builds and hosting into Web Simulator specter-diy-web-simulator#8).
  2. Configure WEB_SIMULATOR_DISPATCH_TOKEN in cryptoadvance/specter-diy, scoped exclusively to the paired Web Simulator repository with Actions read/write permission.
  3. Verify the organization's pull_request_target policy.
  4. Configure GitHub Pages and protect the Web Simulator main branch.
  5. Configure the production Web Simulator Actions secret SPECTER_PREVIEW_BOT_TOKEN and variable SPECTER_PREVIEW_BOT_LOGIN.
  6. Run a controlled production end-to-end test.

The default paired repository is <owner>/specter-diy-web-simulator. The optional WEB_SIMULATOR_REPOSITORY variable can override the repository name while preserving the paired-owner restriction.

Validation

Passed:

  • Specter DIY CI, native tests and firmware build.
  • Automatic dispatcher-to-Web-Simulator workflow dispatch in the forks.
  • Exact source-SHA and live-PR validation.
  • Firmware and browser simulator builds.
  • Browser compatibility, network-policy, Virtual Host and USB transport tests.
  • GitHub Pages publication.
  • Machine-user comment creation and in-place updates.
  • Compact comment layout and verified provenance links.
  • Simulator footer showing firmware version and linked exact source commit.
  • PR-close preview cleanup.

Relevant runs:

Still pending: Production bot-token configuration and a controlled end-to-end test against the upstream repositories.

Merge dependency

Merge and configure Web Simulator PR #8 first. Verify the production machine-user reporter and then merge this dispatcher PR.

See [the browser preview documentation](docs/browser-simulator.md) for additional details.

@netlify

netlify Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for specter-diy-docs ready!

Name Link
🔨 Latest commit 7da453c
🔍 Latest deploy log https://app.netlify.com/projects/specter-diy-docs/deploys/6ac7a71ac21f8200082bdbad
😎 Deploy Preview https://deploy-preview-440--specter-diy-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@Schnuartz
Schnuartz marked this pull request as draft October 2, 2026 08:36
@Schnuartz

Copy link
Copy Markdown
Contributor

To Do:
Allow the pull_request_target event for .github/workflows/browser-preview.yml in cryptoadvance/specter-diy, preferably via a workflow-specific GitHub Actions policy rather than enabling it organization-wide.

@al-munazzim al-munazzim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for splitting the preview build out of this repository and keeping the pull_request_target side small. The trust boundary is generally much better than building PR-controlled code directly here, and the paired simulator PR does a lot of validation/provenance checking.

I would still keep this draft until a couple of pull_request_target concerns are addressed:

  1. edited can trigger expensive privileged preview runs. The workflow listens to edited, maps every non-closed event to ACTION=build, dispatches the paired simulator via WEB_SIMULATOR_DISPATCH_TOKEN, then polls for up to 330 minutes inside a 360-minute job. A fork PR author can edit metadata such as title/body without changing code and repeatedly cause a privileged dispatcher run plus a paired simulator build. If edited is only meant to catch base-branch retargeting, please restrict it to that case in the dispatcher before dispatching, or remove edited and use a maintainer/manual revalidation path.

  2. The pull_request_target token has an unused write scope. The workflow grants pull-requests: write, but the dispatcher only reads PR metadata and writes/patches/deletes issue comments. The write path uses issue-comment endpoints, which are covered by issues: write; I do not see a PR write endpoint being used. Please drop pull-requests: write (or reduce it to read only if GitHub requires it for the PR GET) to keep this new privileged workflow minimal.

Separately, this PR should probably stay blocked on the paired specter-diy-web-simulator workflow being merged/configured and one live end-to-end dispatch proving: dispatch → remote build → Pages status JSON → PR comment update. The current unit/actionlint coverage is good, but the PR body says the hosted path has not been exercised yet.

@al-munazzim

Copy link
Copy Markdown
Contributor

One architectural concern I should make explicit: this currently makes the Specter-side pull_request_target workflow synchronously wait for the paired simulator by polling Pages JSON for up to 330 minutes. That couples the base repo's privileged workflow lifetime to the remote build and can leave stale building comments if the PR/base state changes mid-run.

A cleaner pattern would be async: the Specter workflow dispatches the remote build and posts a short started comment with the remote run link/request id, then the simulator finalizer publishes status and/or updates the PR comment when the result is ready, using a narrowly scoped credential or GitHub App. That would avoid long-lived privileged polling in the base repo and make stale/cancelled remote builds easier to report accurately.

@Schnuartz

Copy link
Copy Markdown
Contributor

al-munazzim

@al-munazzim Which architecture would you prefer here?

  1. Keep the current polling approach – no additional credentials or setup are required, but the privileged Specter workflow stays alive while waiting for the remote build, so this is the least clean architecture.

  2. Use a fine-grained token scoped only to cryptoadvance/specter-diy – it would need only Pull requests: read and Issues: write; it could not modify code, branches, workflows, secrets, or repository settings, but Issues: write can technically also edit/delete other users’ issue/PR comments. Setup is fairly simple, but an organization owner may need to approve the token and someone with admin access to the Web Simulator repo would need to add it as an Actions secret.

  3. Use a GitHub App scoped only to cryptoadvance/specter-diy – it would need the same Pull requests: read and Issues: write permissions, with no code/admin access; Issues: write is still broad enough to modify comments, but the app uses short-lived installation tokens and is not tied to a personal account. Setup is a bit more involved and would require an organization owner or GitHub App manager to create/install it.

My main concern with 2 and 3 is introducing a credential in the Web Simulator that can write back to specter-diy. Because of that, I currently lean towards 1, despite the less clean polling architecture. Do you think the async architecture is worth introducing that additional trust boundary?

@Schnuartz
Schnuartz requested a review from al-munazzim October 6, 2026 09:20
@Schnuartz Schnuartz self-assigned this Oct 6, 2026
@Schnuartz
Schnuartz marked this pull request as ready for review October 6, 2026 09:32
@Schnuartz

Schnuartz commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

CI update for 4be723a: The Specter checks Build Firmware, native-tests, and tests passed (Specter Actions run).

Forked browser E2E: I ran the simulator workflow in my forks using helper PR #4 in schnuartz-ai/specter-diy against base b9c85b9, with the same head commit 4be723a. Validation, trusted runtime build, firmware/browser build, browser and network-policy tests, and Pages deployment all passed (Simulator Actions run). Open the browser preview.

This verifies the build and preview path in the forks. It does not verify the automatic dispatcher credentials or result-comment update on upstream PR #440.

schnuartz-ai added a commit to schnuartz-ai/specter-diy that referenced this pull request Oct 6, 2026
Promote the final browser-preview dispatcher workflow from PR cryptoadvance#440 to the test fork default branch so a separate test PR can exercise the updated pull_request_target workflow.
@schnuartz-ai

schnuartz-ai commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Fork E2E test results

The automatic browser preview flow passed end to end in the schnuartz-ai forks using the PR #440 implementation at head d928540e200bb63123c864dda9b550daf587d2a2.

  • The pull_request_target dispatcher ran automatically, dispatched the simulator workflow, and updated the test PR comment: dispatcher run.
  • The simulator run passed the firmware and browser builds, browser/network/compatibility/USB tests, and Pages deployment: full simulator E2E run.
  • For the stale-result race, commit A (3649415) was cancelled after newer commit B (320cd5b) arrived. B completed successfully and became the published status/comment result.
  • A title-only edit triggered no preview workflow.
  • Closing test PR Switch to libwally & secp256k1, large refactor #6 ran the cleanup successfully: its preview was removed, the status was marked deleted, and the existing bot comment was updated: dispatcher cleanup run, simulator cleanup run.

These checks were run in the schnuartz-ai forks; cryptoadvance organization-specific Actions permissions and secrets were not exercised.

Least-privilege permission proof

I removed issues: write from the fork's default-branch workflow (commit baae93c4), leaving contents: read and pull-requests: write. The reopened E2E mirror PR #4 used that workflow. Its token log shows Contents: read, Metadata: read, and PullRequests: write, with no Issues: write grant.

The dispatcher successfully updated the existing PR comment, and the paired simulator completed the build and preview verification:

PR #440 currently has the same reduced permissions at head 5771bba5. This confirms the permission behavior in the fork; upstream organization-level Actions settings were not exercised.

@al-munazzim

Copy link
Copy Markdown
Contributor

Thanks for the fork E2E follow-up. My preference remains:

  • Long term, the async/GitHub App shape is cleaner because the privileged Specter workflow does not need to stay alive while untrusted-source builds run elsewhere.
  • For this PR, I would not block on adding a new write-back credential into the Web Simulator. Given the current constraints, the polling approach is acceptable if it stays tightly scoped and the stale-result protections keep working.

The latest changes address the two concrete blockers I raised: edited no longer triggers builds, and the broad extra token scope was reduced. The fork E2E evidence also covers the dispatcher → simulator → Pages status → PR comment path, plus stale-result and close cleanup behavior. Remaining upstream merge/configuration checklist from my side is therefore operational rather than code-architecture:

  1. enable Actions/Pages for cryptoadvance/specter-diy-web-simulator;
  2. add WEB_SIMULATOR_DISPATCH_TOKEN with Actions write permission only to the paired simulator repo;
  3. run one upstream test PR after those settings are in place to confirm the org-level Actions policy allows this pull_request_target workflow and that the published status/comment path works.

I do not see a reason to introduce option 2/3 write-back credentials for this PR unless the long polling becomes a practical problem after deployment.

@schnuartz-ai

Copy link
Copy Markdown
Contributor Author

Paired preview E2E result

The simulator E2E run passed for Specter 2c2d8a63d83e9418b2eac21f2c885e5680071a58 paired with simulator e9ff8c8099c9422a2f495b024d95f961a2c17fb9. It covered the clean-runner browser flow, compatibility and network-policy checks, Virtual Host and USB transport, and publication of the immutable Pages preview. The workflow run is here, and the resulting PR #4 preview returned HTTP 200.

The fork did not have the GitHub App credentials required for the workflow-managed PR-comment reporter, so that reporter step was skipped and is not covered by this run.

@schnuartz-ai
schnuartz-ai force-pushed the codex/paired-preview-dispatch branch from 0c7e08d to 9d7e60d Compare October 8, 2026 07:51
Introduce the PR event dispatcher, workflow, and paired-fork documentation.

Preserves the initial architecture from 519a367, before subsequent security hardening.
Harden the PR dispatcher, restrict fork approval handling, bind requests to validated head/base commits, handle retargeting, and add dispatcher tests.

Consolidates original security fixes c7ff174, 26519c9, b4e8005, and 7f2f418.
Use narrowly scoped credentials for cross-repository actions and handle GitHub API errors without leaking secrets.

Consolidates original changes 5414c5d, 7e1d23d and 6f75399. Unrelated documentation fixes are excluded.
Limit untrusted event triggers and use the minimum necessary permissions, including review-driven corrections for PR timeline access.

Consolidates original revisions 9386dd8, 4388e64, 4be723a, d928540 and 5771bba.
Dispatch short-lived validated requests and let the paired Web Simulator own building, browser previews, firmware artifacts, Pages publication and PR status comments. Closing a PR dispatches remote cleanup only.

Preserves the focused architecture of original 2c2d8a6.
Clarify that the existing Specter DIY firmware build is untouched and that all preview-specific firmware artifacts, cleanup, and Pages retention live only in the Web Simulator.

Excludes original firmware-build naming/cleanup commits 5fef3bb and 0c7e08d while incorporating the final preview retention clarification from 47e6588.
@Schnuartz

Copy link
Copy Markdown
Contributor

Update: Fork implementation and E2E tests completed successfully.

We replaced the GitHub App with a dedicated machine user (specter-preview-bot) without repository write access. The bot now updates one persistent PR comment instead of creating duplicates, including collapsible build provenance, firmware links and the security warning.

The simulator footer also shows the firmware version and exact linked source commit (v1.10.5 · <commit>).

Successful tests:

What needs to be done by a organisation admin now like @k9ert:

  1. Protect Web Simulator main (required reviews and CI, restrict direct/force pushes) and verify GitHub Pages uses GitHub Actions.
  2. Configure SPECTER_PREVIEW_BOT_TOKEN (new production PAT, classic, public_repo) and SPECTER_PREVIEW_BOT_LOGIN=specter-preview-bot in Web Simulator Actions settings. The bot must not have repository write access.
  3. Configure WEB_SIMULATOR_DISPATCH_TOKEN in Specter DIY, scoped to Actions read/write on the Web Simulator repository. Verify pull_request_target is allowed.
  4. Merge Web Simulator PR Mix in external entropy sources #8 first, test production bot reporting, then merge Specter DIY PR Dispatch PR previews to paired Web Simulator #440 and verify the full upstream workflow.

The upstream production E2E remains pending. The fork tests passed, and neither PR has been merged.

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.

3 participants