operations: rotate the deletion tokens in place, and prove it took - #188
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new script can exit prematurely on curl errors due to set -e (breaking verification), and should also fail clearly if the server never becomes healthy.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an operational script to rotate both organisation-deletion bearer tokens on the production host, then verifies rotation by proving the new tokens work and the old tokens are rejected—reducing the risk of “no-op” rotations where the server didn’t actually pick up new environment values.
Changes:
- Introduce
deploy/rotate-deletion-tokens.shto rotate both tokens in.env, restart the server viadocker compose up -d, and run acceptance/rejection checks. - Document the rotation procedure and rationale in
deploy/README.md.
File summaries
| File | Description |
|---|---|
| deploy/rotate-deletion-tokens.sh | New rotation + verification script for organisation-deletion tokens. |
| deploy/README.md | Adds runbook documentation for running the rotation script and interpreting results. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Maxerns <maksafk@gmail.com>
Signed-off-by: Maxerns <maksafk@gmail.com>
Signed-off-by: Maxerns <maksafk@gmail.com>
Signed-off-by: Maxerns <maksafk@gmail.com>
Signed-off-by: Maxerns <maksafk@gmail.com>
Signed-off-by: Maxerns <maksafk@gmail.com>
Maxerns
force-pushed
the
ops/rotate-deletion-tokens
branch
from
September 10, 2026 10:03
7450b48 to
80f4378
Compare
Signed-off-by: Maxerns <maksafk@gmail.com>
Signed-off-by: Maxerns <maksafk@gmail.com>
Signed-off-by: Maxerns <maksafk@gmail.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.
Why
Both organisation-deletion tokens need rotating, and the reason is mine: I printed them into a
session transcript while proving a line had landed in
.env, whengrepon the one variablewould have answered the question. They have been due for rotation since.
Rotation is also the kind of thing that gets done once, by hand, slightly differently each time,
and is then hard to be sure actually happened. So it is a script rather than a paste-able
command, and it verifies itself.
The part that makes it worth having
It checks the new token is accepted and that the old one is now refused. Only the second half
distinguishes a real rotation from a no-op edit: a server that restarted without re-reading its
environment accepts the new value from nowhere and keeps honouring the old one, and from outside
that looks exactly like success.
That is not hypothetical.
docker compose restartreuses the existing environment, which is whythis uses
up -d, and it is a mistake this deployment has made before.No token is printed
Not the old ones, not the new ones, not to the terminal and not to a log. The values are written
to
.envand read back from there for the checks. A rotation that shows you the replacement onthe way past has published it to whatever is recording the session, which is precisely how the
current pair needs rotating in the first place.
Fixed in review
Four findings, all valid, and the first is the uncomfortable one: the bearer tokens were passed
as
curl -Harguments, sopson a shared host would have handed them to any local user for theduration of each request. That is the same exposure I fixed in
scripts/check-restoredays ago,in the same way, and then reproduced here. Each request now reads its header from a file at mode
600, removed immediately and on the way out if something fails first.
The other three:
read_vartook every matching line, so a key repeated in.envwould haveproduced a multi-line value and, in an
Authorizationheader, header injection rather than awrong token. The health wait fell through on timeout, turning one clear problem into five
confusing ones. And
status()inside an assignment would have ended the script underset -ewhen curl could not connect, silently, at the moment the operator most needed telling; it now
always prints three digits, using curl's own
000for no response.Testing
Dry-run end to end against stubbed
dockerandcurl, both paths:values are gone, and the other entries in
.env(the Stripe key, the image pin) are untouched.it names the backup and the command to roll back, and exits 1.
Two portability problems surfaced by testing rather than by review.
declare -Aneeds bash 4 andmacOS ships 3.2, so the script could not run where it was written; and
sed -imeans differentthings to GNU and BSD sed, so in-place editing is only testable on whichever the author happens
to have. Both are gone: no associative arrays, and the file is rewritten through a temporary at
mode 600 and moved, which is a better shape for a secrets file than mutating it in place.
What this does not cover
The Stripe test key, which rotates in Stripe's own dashboard and has nothing to do with
.envon the host.