Repository navigation
feat(decdn_node): allow a host-provisioned /etc/decdn/decdn.env (#39) - #41
Merged
Merged
Conversation
AGENTS.md 1 says secrets are generated on -- or operator-provisioned to --
the target host. The host-generated wallet material honoured that; the one
operator-supplied secret did not. decdn_rpc_url (may embed a provider API key)
and decdn_extra_env (AWS credentials behind an S3 origin) sat in plaintext in
a git-ignored host_vars/<node>/secret.yml on every operator's control machine
and were pushed over SSH on each converge, so one compromised laptop yielded
the whole fleet's RPC keys.
The env file may now be written by the operator on the target instead. The
path is inferred from host state:
* decdn_rpc_url set -> the role authors the file from inventory, as before
* decdn_rpc_url empty -> the operator owns the file; the role never reads its
contents back, enforcing only 0600 decdn:decdn and
grepping for a DECDN_RPC_URL= line with a real value
* neither -> fail loud, with the provisioning commands
decdn_extra_env against a host-provisioned file is rejected rather than
silently dropped: the file is authored all-or-nothing, and a dropped AWS key
surfaces hours later as an S3 403 behind a green deploy.
Because stat.exists cannot tell an operator's file from one the role wrote
last week, decdn_env_checksum_file (/etc/decdn/.decdn.env.sha256, 0600 root)
records the hash after each successful converge and serves as a provenance
marker. It drives three behaviours:
* an out-of-band edit restarts the unit on the next converge. The record is
written after the daemon is started, not next to the comparison -- the
handler does not flush until after the config-validate gate, so a record
written earlier would say "already handled" after any aborted run and
permanently burn the one signal that the secret had been rotated
* an empty decdn_rpc_url against a file the role itself wrote fails loud. A
missing secret.yml (fresh clone, CI runner, undecrypted vault) is not the
same as handing the file to the host, and adopting it would deploy green
while keeping a stale endpoint
* an inventory decdn_rpc_url against a file someone else wrote fails loud
rather than discarding it; decdn_env_overwrite_host_file: true confirms.
With no record (a pre-#39 host) provenance is unknowable, so the role warns
and starts tracking -- and that first converge deliberately does not restart
The env file is also required to be a regular file. The role chowns and chmods
that path as root inside a decdn-owned 0750 directory, so following a symlink
would apply 0600 decdn ownership to whatever it points at.
config validate now exits 3 when it cannot source the env file, so a quoting
typo in decdn.env no longer reports as a node.toml schema mismatch.
Tests: new host-env molecule scenario (host-provisioned file, S3 default-chain
origin, no decdn_rpc_url) asserting the file is byte-identical to what the
operator wrote, locked to 0600 decdn:decdn, that node.toml leaks no env value,
and -- via a side_effect rotation and a second converge -- that the daemon is
actually restarted. idempotence pins the converse. Three negative cases in the
validation scenario (no endpoint anywhere, extra_env against a host file, a
host file with no usable DECDN_RPC_URL). default keeps the inventory path and
now also covers the decdn_extra_env accept branch and its quote/backslash
escaping, the sidecar's hash, and EnvironmentFile= in the unit.
build_ignore is dropped from galaxy.yml: "*.example" excluded nothing real
(build.sh stages only roles/ plus the overlay) while silently dropping
roles/decdn_node/files/decdn.env.example, which external operators need.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WaE3ZP2xQdjd9juzRYKLwi
There was a problem hiding this comment.
🟡 Changes recommended
The new host-provisioning failure instructions should be aligned with the repo’s own umask 077 guidance to avoid briefly creating secret files with overly permissive permissions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the decdn_node Ansible role to support a safer secrets workflow by allowing operators to provision /etc/decdn/decdn.env directly on the target host (so secrets don’t live on, or transit, the control machine), while preserving the existing inventory-authored path as an explicit opt-in.
Changes:
- Add host-provisioned secret env support with provenance tracking via a role-managed sha256 record and restart-on-out-of-band-edit semantics.
- Update documentation across the role and repo to describe the two secret authoring paths and the new provenance/overwrite rules.
- Add/extend Molecule coverage (new
host-envscenario + newvalidationcases) to verify no-rewrite behavior, restart behavior, and negative gates.
File summaries
| File | Description |
|---|---|
| ansible/roles/decdn_node/tasks/main.yml | Implements host-provisioned env gating, provenance detection, overwrite protections, restart-on-edit trigger, and checksum recording. |
| ansible/roles/decdn_node/defaults/main.yml | Adds defaults for checksum record path and overwrite confirmation flag; documents the two secret paths. |
| ansible/roles/decdn_node/templates/node.toml.j2 | Updates comments to reflect that S3 creds can come from host env or inventory-authored env via decdn_extra_env. |
| ansible/roles/decdn_node/README.md | Adds a dedicated “Secrets” section documenting host vs inventory secret authoring and the provenance record behavior. |
| ansible/roles/decdn_node/files/decdn.env.example | New host-side template for /etc/decdn/decdn.env and guidance for safe provisioning/editing. |
| ansible/README.md | Updates top-level setup docs to prefer host-provisioned secrets and reference the canonical role docs. |
| ansible/molecule/host-env/molecule.yml | New Molecule scenario exercising host-provisioned env behavior and restart/idempotence sequencing. |
| ansible/molecule/host-env/prepare.yml | Stages operator-provisioned env + wallet fixtures for the host-provisioned scenario. |
| ansible/molecule/host-env/converge.yml | Converges with decdn_rpc_url unset to force the host-provisioned path. |
| ansible/molecule/host-env/side_effect.yml | Performs out-of-band env edit and records PID to verify restart on next converge. |
| ansible/molecule/host-env/verify.yml | Verifies byte-identical env preservation, permissions, checksum record correctness, unit EnvironmentFile, and restart behavior. |
| ansible/molecule/default/converge.yml | Extends default scenario to cover the accepted decdn_extra_env branch and escaping behavior. |
| ansible/molecule/default/verify.yml | Adds assertions for checksum record presence/value match and EnvironmentFile presence on the unit. |
| ansible/molecule/validation/converge.yml | Adds negative test cases for “no endpoint anywhere”, “extra_env with host file”, and “host file missing DECDN_RPC_URL”. |
| ansible/inventory/host_vars/decdn-node-1/secret.yml.example | Reframes inventory secrets as optional and documents overwrite implications. |
| ansible/inventory/host_vars/decdn-node-1/main.yml | Updates commentary to reflect host-provisioned vs inventory-carried secret options. |
| ansible/inventory/group_vars/decdn_nodes.yml | Updates group-vars commentary about the secret’s source options. |
| ansible/galaxy/galaxy.yml | Removes build_ignore to avoid excluding needed *.example templates from the collection build. |
| ansible/galaxy/CHANGELOG.md | Documents the new host-provisioned secrets feature, provenance record, and upgrade behavior. |
| AGENTS.md | Updates repo guidance to explicitly mention operator-provisionable secret env file behavior. |
| .github/workflows/molecule.yml | Updates workflow comments to include the new host-env scenario in the tested set. |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…cord explicit
Two bugs, both found by the tests added in the previous commit.
1. The env file's escaping was inverted, and had been since before this branch.
Inside a YAML literal block Jinja reads '\\' as TWO literal backslashes, so
the role's replace('\\', '\\\\') | replace('"', '\\"') chain never matched a
lone backslash and rendered a double quote as \\" -- an escaped backslash
followed by a CLOSING quote. Any value containing a quote was truncated
there, any backslash passed through unescaped, and systemd then skipped the
line it could not parse, leaving the variable absent at runtime. An S3
secret key containing either character would 403 on the first cache miss
with nothing in the deploy output to explain it.
Both decdn_extra_env values and DECDN_RPC_URL (previously interpolated raw
and unquoted) now render through to_json(ensure_ascii=false), which emits
exactly the double-quoted, C-escaped form EnvironmentFile expects.
ensure_ascii=false matters: systemd understands \xNN, not \uNNNN. Verified
round-tripping quotes, backslashes, spaces, '#' and UTF-8.
This was invisible because no molecule scenario had ever set
decdn_extra_env; the default scenario now does, with a value carrying both a
quote and a backslash.
2. The provenance record could not actually establish provenance. It held only
a hash, and it is written on BOTH authoring paths -- so "record matches
file" means "unchanged since the last converge", not "this role authored
it". The new adopt guard therefore fired on the second converge of a
legitimately host-provisioned node: exactly the configuration it exists to
protect. host-env's idempotence step caught it.
The record is now "<source> <sha256>", source being host or inventory, which
makes both guards precise: an empty decdn_rpc_url against a file recorded as
inventory and unchanged since means the value went missing from the control
machine, and a set decdn_rpc_url against a file recorded as host (or one
whose hash has moved) means an operator edit is about to be discarded. Both
verifies now assert the source token, not just the hash.
Also fixes Jinja filter precedence in the record's ternary -- filters bind
tighter than comparisons, so an unbracketed `decdn_rpc_url | length > 0 |
ternary(...)` parses as (decdn_rpc_url | length) > (0 | ternary(...)) and would
have written a nonsense source token.
molecule test --all: all six scenarios pass, failed=0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WaE3ZP2xQdjd9juzRYKLwi
…assert honestly Two review findings on #41. `sudo tee` followed by `chmod 600` leaves a window in which a secret-bearing file carries the caller's umask, so every copy of the provisioning recipe now sets `umask 077` first (fail_msg, role README, decdn.env.example). The trailing chmod stays as belt-and-braces: sudo may apply its own umask, so the umask alone is not a guarantee of the end state. "Refuse to overwrite an env file this role did not write" printed on every run of a task that conditionally PERMITS the overwrite when decdn_env_overwrite_host_file is true. Renamed to "Require confirmation to overwrite ..." so the play output is accurate in both states. No molecule rescue matches that name, so no test changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WaE3ZP2xQdjd9juzRYKLwi
thiras
added a commit
that referenced
this pull request
Oct 7, 2026
…ing, lifecycle, Compose gate page) (#102) * feat(sponsord): refuse restarts while a pool top-up is held, add sponsord_log_level A restart forgets sponsord's hold on an unconfirmed top-up (upstream #41), and the pool can then be refilled twice. The role now reads /metrics before it flushes a pending restart and fails while the hold is on, leaving the restart for the next run; sponsord_restart_ignore_topup_hold overrides it. Also renders sponsord_log_level as RUST_LOG. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(ansible): back up and decommission sponsord hosts backup.yml archives sponsord's credentials (treasury keystore and password, API token, Turnstile secret), encrypted on the host. decommission.yml checks every sponsord host for a held top-up, then removes the node, the onramp (and the role's Caddy) and sponsord, with one typed confirmation per host, and tears down Alloy on either kind of host. Covered by molecule/sponsord-onramp-lifecycle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(monitoring): sponsord dashboard and alert rules files/monitoring/sponsord/ holds a dashboard (uid decdn-sponsord) and rules for keeper failures, a stale pool read, a held top-up, an empty pool and request errors, with promtool unit tests in lint-helm. The chart does not render them: sponsord has no Kubernetes path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(compose): custom onramp gate page, top-up hold check before restarts The onramp mounts /etc/sponsord/onramp-gate/ read-only for ONRAMP_GATE_TEMPLATE (a fixed directory, so no setting can mount a credential into the page server). The README adds the manual top-up hold check before a restart, a backup command, and notes the onramp ignores RUST_LOG. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: close the review findings on sponsord ops coverage - sponsord: the restart-inputs comparison is the only restart trigger, and the top-up hold guard runs before it queues anything, so a write landing back on the recorded content can no longer restart past the guard (nor can --force-handlers). The guard probes the address the running process listens on, passes a daemon that has not bound yet, requires sponsord's keeper series, accepts a labelled gauge, retries briefly, and words its refusal for a restart or a stop. The documented override is the JSON form the boolean check accepts. - decommission: check the hold again right before sponsord's stop (the prompt can wait minutes); each prompt lists every service the run stops on its hosts. - backup: the Turnstile secret at the onramp role's path, required on onramp hosts. - sponsord_onramp_log_level: the onramp does read RUST_LOG (as Targets: a level or target=level pairs); correct the docs that said it ignored it. - alerts: SponsordRequestFailures fires on a code's first appearance (upstream exports it from its first error, already non-zero); a pool never read since start is SponsordPoolNeverChecked (3h), not "stale" after 15m. Tests model the exporter as it is, with internal-code and resolve cases. - tests: a revert during a hold, an unreadable /metrics, backup and log-level negatives, the decommission play order, a second decommission run, no confirmation left behind by a refusal. - docs: co-located caveats in lifecycle.md, the gate page's path rule, safer manual check and backup in the Compose README, stale scenario and galaxy text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: address the PR review on decommission, the gate page, backup and logging - decommission.yml: the first play enforces decdn_decommission_max_hosts against every host the run touches (each role checks only its own play, so a node-only and a sponsord-only host both passed), then checks for a held top-up. - compose: the onramp starts through a pinned /bin/sh check that refuses an ONRAMP_GATE_TEMPLATE resolving outside /etc/sponsord/onramp-gate/; upstream would serve any file, and /run/secrets holds the daemon token and the Turnstile secret. invariants.jq pins the script byte for byte (no flags). - backup: every archived credential must be a regular file; tar stores a symlink as the link, not the key. - log levels: each comma-separated part must be a level or target=level; a misspelled level or a bare non-level word silences the onramp (and reads as a target for sponsord). - render-test.sh: the local promtool test step honours PROMTOOL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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
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.
Closes #39.
Problem
AGENTS.md§1 states the repo's posture: secrets are generated on — or operator-provisioned to — the target host. The host-generated wallet material (keystore.json,node.secret, the keystore password) honours that exactly. The one operator-supplied secret did not:decdn_rpc_url(may embed a provider API key) anddecdn_extra_env(the AWS credentials behind an S3 cache origin) lived in plaintext in a git-ignoredhost_vars/<node>/secret.ymlon every operator's control machine, and transited it on every converge. One compromised control machine yielded every node's RPC key, fleet-wide.Two things made the host-provisioned path impossible rather than merely undocumented:
decdn_rpc_urlwas unconditionally asserted, and the env-filecopywas unconditional, so it would clobber a correct host-provisioned file with control-machine values.What changed
/etc/decdn/decdn.envcan now be provisioned by the operator on the target. The path is inferred from host state:decdn_rpc_url/etc/decdn/decdn.env0600 decdn:decdn, greps for a usableDECDN_RPC_URL=linedecdn_extra_envagainst a host-provisioned file is rejected, not silently dropped — the file is authored all-or-nothing, and a dropped AWS key surfaces hours later as an S3 403 behind a green deploy.The provenance record
stat.existscannot tell an operator's file from one the role wrote last week, and that ambiguity is dangerous in both directions.decdn_env_checksum_file(/etc/decdn/.decdn.env.sha256,0600 root) records the hash after each converge that gets the daemon running, and gives three behaviours:config validategate, so a record written earlier would say "already handled" on the next converge after any aborted run — permanently burning the one signal that the secret had been rotated.secret.ymlis not "the host owns this now." An emptydecdn_rpc_urlagainst a file the role itself wrote fails loud instead of adopting it and deploying green on a stale endpoint.sudo rmthe record to hand a node over deliberately.decdn_rpc_urlagainst a foreign file fails loud rather than discarding it;decdn_env_overwrite_host_file: trueconfirms. With no record (a host predating this) provenance is unknowable, so the role warns and starts tracking — and that first converge deliberately does not restart, so the upgrade does not bounce the fleet.Also
decdn-owned0750directory, so following a symlink would apply0600 decdnownership to whatever it points at.decdn config validateexits 3 when it cannot source the env file, so a quoting typo indecdn.envno longer reports as anode.tomlschema mismatch.build_ignoreis dropped fromgalaxy.yml."*.example"excluded nothing real —build.shstages onlyroles/plus the overlay — while silently droppingroles/decdn_node/files/decdn.env.example, which external collection users need.Backward compatibility
Non-breaking. An inventory with
decdn_rpc_urlset behaves as before. The first converge seeds the provenance record without restarting.Test plan
host-envmolecule scenario — host-provisioned file, S3default-chainorigin, nodecdn_rpc_url. Asserts the file is byte-identical to what the operator wrote (it carries a key the role has no inventory var for, so a rewrite could not reproduce it), locked to0600 decdn:decdn, that no env value leaked intonode.toml, and thatEnvironmentFile=is on the unit. Aside_effectstep rotates the secret out of band and a second converge must restart the daemon (PID comparison) — the headline behaviour, which every checksum assertion alone would leave unproven.idempotencepins the converse.validation: no endpoint anywhere,decdn_extra_envagainst a host file, and a host file with no usableDECDN_RPC_URL. Each owns its fixture viaalways:.defaultkeeps the inventory path and now also covers thedecdn_extra_envaccept branch with a quote-and-backslash value (exercising the template escaping, whose failure mode is a silently-skipped systemd line), the sidecar's hash value, andEnvironmentFile=.pre-commit,ansible-lint(production profile), KICS (0 high), andmake buildall pass locally. CI runsmolecule test --all.🤖 Generated with Claude Code
https://claude.ai/code/session_01WaE3ZP2xQdjd9juzRYKLwi