feat(secret-rotator): sync bot tokens into Woodpecker org secrets #363
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: konairius/homelab#363
Reference in New Issue
Block a user
Moves
review_bot_tokenandclaude_code_oauth_tokenoff hand-loading and into the secret-rotation toolchain, targeting org-level Woodpecker secrets (every repo underk.rennerinherits them — no per-repo copies).What it adds
secret-rotatorwoodpecker-secretmode (woodpecker.rs): upserts an OpenBao-held value into ak.rennerorg CI secret (create if absent, else patch value+events) via a dedicatedwoodpecker-botbearer token. Verified endpoints against the live server (org id lookup, 404=absent). Never logs the value; refuses to push a blank over a working secret.review-bot.yaml— one CronJob: init step mints review-bot's PAT (existinggitea-tokenmode, pull-verified against a private repo it collaborates on; push not verified — read collaborator, posts reviews via API), main step syncs it to org secretreview_bot_token(cron,manual,pull_request).claude-oauth.yaml— sync-only (interactivesetup-tokencan't be minted headlessly): daily push of an owner-seeded OpenBao value → org secretclaude_code_oauth_token.rotator-{review-bot,claude-oauth}.hcl(least-privilege) + netpol (rotator-*-egress,gitea-allow-review-bot-rotator, newwoodpecker-allow-rotators).Validation
cargo testgreen (15 tests incl. the new module);kubectl kustomizebuilds clean for the rotation dir, woodpecker app, and network-policies.Deploy prerequisites (can't come from git — do before/at rollout)
secret-rotatorimage includingwoodpecker-secretmode, then pin@sha256inreview-bot.yaml+claude-oauth.yaml(they're on:latest; the digest the other rotators pin predates this mode and 404s with "unknown mode")..hclheaders).secret-rotators-bootstrap/woodpecker-bot#token(a dedicated woodpecker-bot API token, org-secret write),secret-rotators-bootstrap/review-bot(review-bot's password),infrastructure/review-bot#token(placeholder),infrastructure/claude-code-oauth#token(yoursetup-tokenvalue).kubectl -n openbao create job --from=cronjob/...), confirm the org secrets appear, then drop homelab's repo-levelclaude_code_oauth_token/review_bot_tokenso the org versions are the single source.Depends on / completes the PR-event review-bot cutover (k.renner/homelab#362 + the shelly/authentik/HA PRs).
🤖 Automated review — model
claude-opus-4-8(complexity: complex), commit121a07d496e244adc9abad742602db375b536f8f.Summary
Adds a
woodpecker-secretmode tosecret-rotator(newwoodpecker.rs) thatupserts an OpenBao-held value into a Woodpecker org-level CI secret, plus two
CronJobs (
review-bot.yaml,claude-oauth.yaml), two least-privilege OpenBaopolicies, and the Cilium ingress/egress halves to let the openbao-ns rotators
reach gitea
:3000and the woodpecker server:8000. The change is careful andwell-documented; I found no blocking issues — the notes below are
robustness/clarity suggestions.
I verified: the
woodpecker-serverpod carries the labels the newwoodpecker-allow-rotatorsingress policy selects (server.yaml:7-8, port 8000at
server.yaml:94); DNS/apiserver/world egress for the new rotator pods isalready granted by the openbao
default-deny-ingressendpointSelector: {}policy (
openbao-ns.yaml:37-84), so the new egress policies only need to add thein-cluster targets — matching the existing
rotator-gitea-flux-egresspattern;the apps ingress model is additive (
allow-ingress.yaml,default-deny.yaml),so adding the rotator sources does not restrict Traefik/agent access; and the
OpenBao policy paths (
homelab/data/infrastructure/...) match thedata/KV-v2 path
bao.rsconstructs.🔴 Blocking
None.
🟡 Suggestions
is_success()branches are effectively unreachable insecret-rotator/src/woodpecker.rsupsert_org_secret(the PATCH block and thePOST block).
ureq3 treats non-2xx asErrby default — the codebase relieson exactly this in
gitea.rs:82-96(Err(ureq::Error::StatusCode(403))). So.send_json(&body)?propagates a rawStatusCodeerror on failure and theelse { err(format!("... -> {}", resp.status())) }arms never run. Theintended, more useful message (
create/patch org secret k.renner/... -> 500)will therefore never be emitted. Consider either matching on
Err(ureq::Error::StatusCode(code))likegitea.rsdoes, or dropping the deadelseand letting the ureq error propagate — the current form reads as if ithandles the failure path but doesn't.
PATCH omits
images(woodpecker.rs, theexists()==truebranch sends{ "value": value, "events": events }). If Woodpecker's PATCH handler resetsunspecified fields to defaults, an existing secret's
imagesrestriction wouldbe widened to "all images" on the next sync — a small trust-boundary change for
review_bot_token/claude_code_oauth_token. I could not confirm Woodpecker'spatch-merge semantics from this repo. One check settles it: PATCH an org secret
that has a non-empty
imageslist and confirm the list survives; if it doesn't,send the existing
imagesback in the body.CronJobs ship
suspend: falseon a:latestimage that does not yet containthis mode.
claude-oauth.yaml:295runs daily (schedule: "20 4 * * *") andreview-bot.yamlquarterly, both onsecret-rotator:latest(claude-oauth.yaml:325,review-bot.yaml:451/493).Per the PR body the current pinned digest 404s with "unknown mode", so once
Flux applies these, the claude-oauth Job will fail daily until the image is
rebuilt and
@sha256-pinned. It fails safely (errors before any API write, oldorg secret untouched) and
failedJobsHistoryLimit: 5caps the litter, but itwill generate recurring failed Jobs / alert noise. Consider shipping with
suspend: trueand flipping it in the same change that pins the digest, so themerge order can't produce failing Jobs.
🟢 Nits
review-bot.yamlusesapp.kubernetes.io/name: woodpecker(nocomponent) inthe egress
toEndpoints(viarotator-review-bot-egress), which also matchesthe agent pod. Harmless (traffic targets the server Service), but scoping to
component: serverwould mirror the ingress side's tighter selector.Tests
Consistent with the repo's convention:
woodpecker.rsunit-tests only the purebearer()helper, exactly asgitea.rstests onlybasic()— the HTTP paths arenot mocked anywhere in this crate. The create-vs-patch branching and
org_idJSON parsing are therefore exercised only against the live server (the PR notes a
manual endpoint verification). That's acceptable here, but the
org_idparse(
v["id"].as_i64()) and the events-splitting insync_woodpecker_secretare pureand could be cheaply unit-tested if you want regression coverage without a server.
Recommendation
Looks reasonable — no blocking issues; comments above are optional.
121a07d496to581f25d72c581f25d72cto16013a4ca0🤖 Automated review — model
claude-opus-4-8(complexity: complex), commit16013a4ca051697d33b209b4ec4e47c560fd6f99.Summary
Adds a
woodpecker-secretmode tosecret-rotatorthat upserts an OpenBao-heldvalue into a
k.rennerorg-level Woodpecker CI secret, plus two CronJobs(
review-bot,claude-oauth), OpenBao policies, and the netpol halves to wirethem up. The Rust is clean, well-tested for its unit surface, and consistent
with the existing rotators; the design (org-level, sync-not-mint, value never
logged) is sound. No blocking defects found — the notes below are verification
and sequencing items the author has largely pre-flagged.
🟡 Suggestions
1. The pull-only verify gate depends on
shelly-managerbeing private —please confirm.
review-bot.yaml:473-482setsGITEA_VERIFY_REPO: k.renner/shelly-managerwithGITEA_REQUIRE_PUSH: "false". The existingrotate_gitea_tokencode warns exactly about this atsecret-rotator/src/main.rs:285-289:for a public verify repo,
git-upload-packreturns 200 for any valid tokenregardless of scope, so a pull-only check degrades to "is this any valid token"
— the safety gate that keeps a broken PAT from overwriting a working one stops
being load-bearing. The manifest comment asserts the repo is private, but that
can't be verified from this checkout (the homelab repo itself is public per
pr.json). One check settles it:curl -so /dev/null -w '%{http_code}' https://gitea.renner.uno/k.renner/shelly-manager(302→login = private, 200 =public). If it's public, either flip
GITEA_REQUIRE_PUSHback on against a reporeview-bot can push, or point
GITEA_VERIFY_REPOat a private repo.2.
:latestwill fail nightly until the image is rebuilt and pinned.claude-oauth.yaml:325andreview-bot.yaml:451,493runsecret-rotator:latestwithargs: ["woodpecker-secret"]. The current:latestpredates this mode, so it exits with
unknown mode 'woodpecker-secret'(
main.rs:41). The claude-oauth CronJob issuspend: falseon a daily20 4 * * *schedule, so once Flux applies this it fails every night at 04:20 until thenew image is built and the
@sha256pinned. This is non-destructive (a failedsync leaves the org secret untouched) and the PR body lists it as a rollout
prerequisite — just calling out that the failure window opens at merge, not at
your convenience. Consider
suspend: trueon both CronJobs in this PR andflipping them on in the image-pin follow-up, so a merge doesn't start a failing
schedule.
3. Make-before-break has a narrow reaping window if a sync fails.
review-bot.yaml: the init step stores the new PAT to OpenBao and reaps oldtokens keeping the two newest (
main.rs:325-345), independent of whether themain
sync-woodpeckerstep then succeeds. If a quarter's sync step fails,Woodpecker keeps referencing token T1 while OpenBao holds T2. The next
quarter mints T3 and reaps down to {T3, T2} — deleting T1, which Woodpecker is
still using, before T3's sync lands. Quarterly cadence +
backoffLimit: 1makesthis unlikely, but the "if the sync step fails the pipeline keeps working on the
prior value" claim in the header comment holds only for one cycle. Not worth
restructuring; worth a one-line caveat in the comment.
🟢 Nits
woodpecker.rs:715-719/728-732: theelse { err(...) }arms aftersend_json(&body)?are effectively unreachable — ureq 3 hashttp_status_as_erroron by default, so a non-2xx already returnsErrfrom?. Harmless and it mirrors the same defensive pattern inbao.rs:135, sofine to leave; just noting it isn't doing what the branch implies.
Tests
cargo testis reported green (15 tests). The new module ships a unit test forbearer()(woodpecker.rs:736-743), consistent with howgitea.rs/bao.rstest their pure helpers (
basic(),mount()/sub()). The HTTP-driven logic(
org_id,exists's 404-means-absent branch, create-vs-patch selection) is notunit-tested — matching the repo's existing convention of not mocking ureq — and
the author states it was verified against the live server. That's the right call
for this codebase; the one behavior I'd most want asserted is
exists()mapping only 404 to
falseand propagating every other status, since that's theguard that stops a blind create/patch.
Recommendation
Looks reasonable — no blocking issues; comments above are optional (item 1 is a
verification worth doing before rollout).
🤖 Automated review — model
claude-opus-4-8(complexity: complex), commit0f41efee359af868811ff90e926a23f0513b5073.Summary
Adds a
woodpecker-secretmode to thesecret-rotatorbinary (newwoodpecker.rs)that upserts an OpenBao-held value into a
k.rennerorg-level Woodpecker CIsecret, plus two suspended CronJobs (
review-bot.yaml,claude-oauth.yaml),their OpenBao policies, and the Cilium ingress/egress halves. The change is
well-scoped, closely mirrors the existing flux-git rotator conventions, and both
CronJobs ship
suspend: trueso nothing runs at merge. No blocking issues found.🟡 Suggestions
Org-scope widens the secret's blast radius vs. repo-scope —
review-bot.yaml:534/claude-oauth.yaml:351pushreview_bot_token(a
write:repositoryGitea PAT) andclaude_code_oauth_tokento ak.rennerorg secret, inherited by every repo in the org, with
pull_requestin theevent set.
homelabitself is a public repo (pr.json→"private":false).Worth confirming Woodpecker does not hand these secrets to pipelines triggered
by fork/untrusted
pull_requestevents on any org repo before un-suspending —moving from repo- to org-scope only enlarges the set of pipelines that can read
a write-scoped credential. This is the stated design intent, so just a
double-check, not a defect.
GITEA_VERIFY_REPOvisibility is load-bearing but external to this repo —review-bot.yaml:492points the pull-verify atk.renner/shelly-managerandthe comment itself flags that if that repo ever goes public the check silently
degrades to "is this any valid token" (
main.rs:285-289). The safeguard is ahand-maintained invariant with no automated guard; the inline
IMPORTANTnoteis the right mitigation, but it's the kind of thing that rots. No action needed
now.
read_fieldis the only guard against pushing a blank —main.rs:611-613relies onbao::read_fielderroring on missing/empty(confirmed at
bao.rs:76-79), which is correct. Unlikerotate_gitea_token,the woodpecker-secret path has no
assert_fields_exist— fine here since itnever patches OpenBao, but note the "refuse blank over working secret" claim
depends entirely on
read_field's empty-check, so keep that invariant ifread_fieldis ever refactored.🟢 Nits
Dead
elsebranches inupsert_org_secret—woodpecker.rs:733-737and746-750checkresp.status().is_success()and build a custom"patch/create org secret … -> {status}"error otherwise. With ureq 3(
Cargo.toml:11,http_status_as_errordefaults totrue, and the rest ofthe codebase relies on this — see
gitea.rs:90-94),send_jsonalready returnsErr(StatusCode(..))for any non-2xx, so thoseelsearms are unreachable andthe nicer message never fires; the raw ureq error propagates instead. Harmless,
but inconsistent with
exists()right above it, which correctly matches onErr(StatusCode(404)).rotator-review-bot-egresswoodpecker toEndpoints isn't scoped tocomponent: server—
openbao-ns.yaml:166-173(and the claude-oauth egress at189-196) allowegress to any
app.kubernetes.io/name: woodpeckerpod on :8000, whereas theapps-side ingress (
woodpecker/networkpolicy.yaml:72-74) correctly narrows tocomponent: server. Egress-side over-breadth is harmless (agents don't listenon 8000), just slightly looser than necessary.
Tests
Consistent with the repo's convention: the only unit test is
bearer_header_prefixes_token(woodpecker.rs:758-761). The create-vs-patchbranching in
upsert_org_secret, theorg_idlookup, and the 404→absent logicin
existsare unverified by tests — the same asgitea.rs/cloudflare.rs,which only test pure helpers, so this matches the established pattern rather than
introducing a gap. The PR body reports these endpoints were verified against the
live server. If a test were added, the highest-value one would cover
existsreturningfalseon 404 vs. propagating other status codes (thatbehavior is what keeps a create/patch from firing blind).
Recommendation
Looks reasonable — no blocking issues; comments above are optional.