Add CI: test on every PR, deploy on merge to master - #4
Merged
Conversation
Adds two GitHub Actions workflows and the scripts/docs behind them: - .github/workflows/test.yml runs on GitHub-hosted runners for any pull request (forks included) and on push to master: gofmt, go vet, go test, go build for both the relay module and keys-panel, plus bash -n on every deploy script. - .github/workflows/deploy.yml runs deploy/ci-deploy.sh on a self-hosted runner on the production host, triggered only by push to master. This repository is public, and self-hosted runners are a well-known way for a public repo to turn "anyone can open a PR" into "anyone can execute code on whatever the runner sits on" - GitHub's own docs warn against exactly this combination. deploy/CI.md is the trust model for why this deployment doesn't have that problem: the deploy workflow only ever triggers on a push to master, master is branch-protected (PR + collaborator-only merge, enforced for the owner too), so nothing an outside PR does can produce a push event against this repository's master - not the "require approval" setting, the trigger topology itself. test.yml intentionally stays on GitHub-hosted runners so external PRs still get real CI signal without ever reaching production. Also, in support of that: - deploy/ci-deploy.sh: the one thing the self-hosted runner's unprivileged service account can run as root (via one exact, argument-free sudoers rule) - calls the existing deploy/update-relay.sh and the new keys-panel/deploy/update-keys-panel.sh, both test-build-validate-install -with-rollback, same shape as update-relay.sh already documented in README.md. Deliberately not redeployed by the pipeline it gates, so a bad merge can't widen its own privileges on the next run - see deploy/CI.md for the installation step this implies. - keys-panel/deploy/update-keys-panel.sh: the same safe-update pattern for tproxy-keys, which had no equivalent before this. Runner installation itself (creating the ghrunner account, registering the runner, the sudoers grant) is a manual step on the production host - not something this PR does, since it requires host access this repository's CI can't have yet. Documented in deploy/CI.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![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: 02f546a244
ℹ️ 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".
Pinned down the actual endpoint and parameter values by hand (the docs returned by an assistant web fetch pointed at the wrong one) and applied it to the repository: fork-pr-contributor-approval, policy all_external_contributors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The keys-panel job only ran vet and build, so a future regression test added to that module could pass CI green without ever actually running - the relay job's go test ./... doesn't descend into keys-panel, since it's a separate Go module. No test files exist there yet, so this is a no-op today (`go test ./...` just reports "no test files"), but it's the correct steady state to be in when tests do get added. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
Fixed in 08fd3fb — added |
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.
Summary
.github/workflows/test.yml: gofmt/vet/test/build for both Go modules +bash -non every deploy script, on GitHub-hosted runners, for any PR (forks included) and on push to master..github/workflows/deploy.yml: runsdeploy/ci-deploy.shon a self-hosted runner on the production host, triggered only by push to master.deploy/CI.md: the trust model — why a self-hosted runner is safe here despite this repo being public. Short version: the deploy workflow only triggers onpushtomaster, andmasteris branch-protected (PR + collaborator-only merge, enforced for the owner too), so no outside PR can ever produce that event against this repository. That's the actual safety boundary, not the separate "require approval for outside collaborators" setting (also enabled, as defense in depth).deploy/ci-deploy.sh: the one thing the runner's unprivileged service account can run as root, via one exact argument-free sudoers rule. Calls the existingdeploy/update-relay.shand the newkeys-panel/deploy/update-keys-panel.sh(same test → build → validate → atomic install → health-check → rollback-on-failure shape). Not redeployed by the pipeline it gates — installing it and its sudoers grant is a manual step, documented indeploy/CI.md, precisely so a bad merge can't widen its own privileges on the next run.Test plan
bash -non all changed/new shell scriptsact/live GitHub Actions available before this PR exists)gofmt -l .,go vet ./...,go test ./...,go build ./...clean on the relay module;gofmt -l .,go vet ./...,go build ./...clean onkeys-panel/— all re-verified on a real host on this exact treetest.ymlrunning for real on this PR, on the hosted runner — will check after openingdeploy.ymlon the self-hosted runner — can't be exercised before merge by design (it only triggers on push to master); runner isn't installed on the host yet either. Planned right after this merges: install the runner perdeploy/CI.md, then merge a trivial follow-up to master and watch the first real deploy run end to end.🤖 Generated with Claude Code