Vault-manage staging's Stripe test key (scoped fix) - #68
Open
rdhyee wants to merge 2 commits into
Open
Conversation
Staging's Stripe test secret key went stale: it was rolled in the Stripe dashboard at some point, and the old value on test.unglue.it (a static, unmanaged hand-set file) started failing "Expired API Key provided" on any real charge attempt. group_vars/test/vault.yml (new, encrypted) plus two lines in vars.yml wire up vault_stripe_pk/vault_stripe_sk, sourced from a new 1Password item (EbookFoundation/"unglue.it staging Stripe"). setup-test.yml gets two new stripe-tagged tasks that surgically rewrite just the two Stripe lines in the already-deployed keys file on the box, via a scoped regex replace rather than a full-file re-render. Deliberately scoped, not full parity with prod/dev/ondeck: re-rendering the whole keys file would require vaulting every other secret currently on the box (AWS keys, secret key, email password, OAuth, Turnstile). Those stay exactly as they are. Rotating the key going forward: update the 1Password item, rebuild the vault entry, then ansible-playbook -i hosts setup-test.yml --tags stripe Live-verified against the real Stripe API post-deploy: new key returns a valid test token, no expiry error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014k6DLV3ytoo5QNRFMSy29W
Codex reviewed c6a3c12 and flagged four real issues (kept, disagreed with none): 1. A plain `lineinfile` fails open if the file's exact quote/spacing ever drifts from what the regex expects (misses -> appends a duplicate at EOF instead of erroring). Added two preflight tasks: exact-count grep for each of STRIPE_PK/STRIPE_SK, then an assert that fails loudly if the count isn't exactly 1. Broadened the replace regexp itself since the preflight now guards existence. 2. The "we do NOT re-render the whole file" comment overclaimed: true only for `--tags stripe`. An untagged run (or `--tags config`) still executes the role's full render, which test's group_vars aren't configured for. Reworded both comments to be precise about scope. 3. No validation that the vault-supplied values are actually non-empty test-mode keys. Added a no_log assert requiring pk_test_/sk_test_ prefixes and non-empty values before either edit runs. 4. `become: yes` was unnecessary — the role's own equivalent template task runs unprivileged. Removed it. Also added `validate: python3 -m py_compile %s` to both lineinfile tasks (catches any templating mistake that would produce invalid Python before it's written live), and fixed vars.yml's "sourced from" wording to describe the actual manual 1Password -> vault-edit workflow, with the three commands to run on the next rotation. Did not add `backup: true` (Codex's own call, agreed): a backup would copy every other still-static secret in the same file into a second plaintext copy with no retention policy — worse than the problem it solves. The preflight + validate + the existing live Stripe-API smoke test are the safety net instead. Re-verified idempotent in --check --diff against test.unglue.it: all preflight assertions pass, both rotate tasks report no change (the values already match from the earlier real run). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014k6DLV3ytoo5QNRFMSy29W
4 tasks
rdhyee
added a commit
that referenced
this pull request
Sep 3, 2026
…e, reorder preflight Codex round-1 findings on PR #69, all reproduced independently before fixing (fetched roles/regluit_prod/templates/prod.wsgi.j2 and its rendering task directly; empirically tested ansible-vault edit against a nonexistent file): - BLOCKING: the scoped production rotation updated a fallback Apache does not use. deploy/prod.wsgi (rendered by prod.wsgi.j2, confirmed live via apache.conf.j2's WSGIScriptAlias) sets os.environ[NAME] = <value> directly at WSGI worker startup, which wins over keys/host.py's os.environ.get(NAME, <fallback>). The original PR only edited keys/host.py -- a --tags google-oauth rotation could complete successfully while production kept authenticating against the OLD client. Fixed: added matching preflight + lineinfile tasks for deploy/prod.wsgi in both setup-prod.yml (unconditional -- prod's file is confirmed to exist) and setup-test.yml (guarded by a stat check first -- test's copy of this file, if any, was not independently confirmed). - BLOCKING: the recommended full --check --diff can print the OAuth secret. The role's "Create WSGI Script" task has no no_log (open PR #58 in this repo fixes that, unmerged). Documented in both playbooks' trailing comments: only --tags google-oauth (whose tasks are all no_log: true) is safe to --diff; a full untagged --check --diff is not, until #58 merges. - The non-empty assertion ran too late for a full run: positioned after import_role, an untagged production run would render an accidental empty vault value into both files before the assertion ever caught it. Moved the assertion before import_role, guarded with `is defined` so it is a no-op whenever these two vars aren't in play, and strengthened to `| trim | length > 0` so whitespace-only values are caught too. - The duplicated production variables (top-level vars.yml entries alongside the existing host_keys nesting) were flagged as an unnecessary precedence-divergence risk. Removed entirely -- group_vars/production/ vars.yml now has zero diff versus master; setup-prod.yml's tasks reference host_keys.social_auth_google_oauth2_key/_secret directly, the canonical values already backing the live client. - group_vars/test/vars.yml's TODO comment said `ansible-vault edit` to create group_vars/test/vault.yml -- verified empirically that edit refuses a nonexistent file ("Unable to read source file"); only `ansible-vault create` does. Comment corrected to cover both cases (create for a first-time file, edit once PR #68 or a prior run of these instructions has made one). Verified: `ansible-playbook --syntax-check` passes for both playbooks; both modified group_vars/*/vars.yml files parse as valid YAML; the new lineinfile regexes were tested directly against sample lines shaped like what host.py.j2/prod.wsgi.j2 actually produce (both match correctly, neither cross-matches the other setting's line). Not addressed (Codex flagged as non-blocking / a judgment call, not fixed here): a syntactically-valid-but-wrong credential still passes these assertions -- only a real login (RUNBOOK_1246.md's verification step) can catch that; py_compile checks Python syntax only, not credential validity. -- rdhyee + Claude
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.
Why
Staging's (test.unglue.it) Stripe test-mode secret key went stale: it was rolled in the Stripe dashboard at some point, and the old value — a static, hand-set file with nothing managing it — started failing
Expired API Key providedon any real charge attempt. Found and fixed live 2026-08-28.What this does
group_vars/test/vault.yml(ansible-vault encrypted), holding justvault_stripe_pk/vault_stripe_sk, sourced from a new 1Password item (EbookFoundation/unglue.it staging Stripe).stripe-tagged tasks insetup-test.ymlthat surgically rewrite just theSTRIPE_PK/STRIPE_SKlines in the already-deployedsettings/keys/common.py— not a full-file re-render.pk_test_/sk_test_prefix required) before either line is touched, plus a Python syntax validation pass, added after a Codex review round.Rotating the key going forward: update the 1Password item,
ansible-vault edit --encrypt-vault-id fef group_vars/test/vault.yml, thenansible-playbook -i hosts setup-test.yml --tags stripe.Deliberately out of scope
This is a scoped fix, not a full-parity vault migration. Staging's
settings/keys/common.py/keys/host.pycarry ~22 other secrets (AWS keys, DjangoSECRET_KEY, email password, Google OAuth, Cloudflare Turnstile, Mailchimp/Booxtream/KU/Dropbox) — all untouched by this PR, all still static/hand-managed.Related history, for context:
SECRET_KEY— both HIGH severity, both still unresolved). That issue's item 1 (Stripe keys) was fixed at the time via #32 — but that PR merged into an abandoned branch (feature/parameterize-python-version) that was never merged tomaster. The fix was deployed directly to the boxes and worked, but master's tracked config never captured it, which is part of why today's regression went unnoticed.SECRET_KEYitems is being tracked as a separate, prioritized piece of work (owner: Raymond), not bundled here.Verification
ansible-playbook --syntax-checkclean.--check --diffdry run againsttest.unglue.it: preflight assertions pass, both edits apply as expected (no_logconfirmed to suppress secret content from--diff).tok_0U9PdYVtOLnXMBHClqGNZ3Jk), no expiry error.lineinfile, corrected scope-of-comment wording, added value-shape validation, removed an unnecessarybecome: yes.🤖 Generated with Claude Code