Four holes, all of which let untested or unchecked code reach an environment.
- PRs into `release` ran no build and no test. release is the branch that deploys
to prod, so the intended promotion path was the least-checked path in the repo:
the last seven release PRs ran shellcheck and the secrets check, nothing else.
pr-build now triggers on it.
- shell-lint and secrets-guard reported as standalone statuses outside `gate`,
and pr-build's own setup notes tell the operator to require only `gate`. Taken
literally that means a commit adding a plaintext SOPS file was mergeable. Both
are now called from pr-build and reduced into gate. They are deliberately not
domain-gated and not `needs: changes`: they are repo-wide invariants that must
hold on every PR, including one touching no app domain -- exactly the case
where every domain job skips and gate used to pass vacuously. For the same
reason `skipped` counts as a failure for these two, while it stays a pass for
the domain jobs it legitimately describes.
They keep their standalone pull_request trigger, so they will run twice on a
PR until branch protection is confirmed to require only `gate`. Cheap, and it
avoids breaking a rule that may still require them by name.
- build-push.yml built and pushed with no test step, and deploy.yml consumes a
TAG rather than a commit status -- so an image reaching the registry by any
route other than a dev/main PR (a direct push, a dispatch, a v*.*.* tag) was
never tested by CI, and beta/prod then rolled it. The test now sits between
build and push, so the artifact that deploys is the artifact that was tested.
Gating the artifact is what makes this work; gating the branch would not.
- backend/Dockerfile ran `go build` on daemon/ but never its tests. All 48 of
them (gateway, cron, apiclient, config) ran nowhere: `grep -rn "go test"
.forgejo/workflows/` found nothing, while the daemon owns the prod Discord
gateway websocket and every recurring-job timer. It now runs gofmt + vet +
test before the build, which is the pattern frontend/Dockerfile already uses --
and being inside `docker build` it gates build-push too. Verified clean: gofmt
reports nothing, vet passes, all four packages pass.
A test run could message real subscribers, and would have reported green while
doing it.
Every send gate in notifications/ reads its config from ambient env at import
time, and conftest neutralised none of it -- it set four env vars, none touching
Discord, SMTP or VAPID. test_notify.py calls run_pass() seven times without
patching notifications.discord, and notify.py:358 reaches
discord.post_subscription_alert() for every notification it creates. The
subscription channel notifies real people.
The failure mode is silent: notify.py's send path and discord.py's _bot_post
swallow every exception, so a real send raises nothing and no assertion goes red.
An operator who had sourced /etc/thermograph.env to debug -- routine -- turned
`pytest tests/notifications` into a live broadcast. CI was safe only by omission:
it runs in a container with no credentials baked in, and one added -e removed
that.
Two layers:
1. Config blanked, so every enabled()/dm_enabled()/subscription_enabled() gate
reports False. This is the state CI already runs in, now guaranteed locally
instead of depending on the shell.
2. The transports themselves replaced with sentinels that raise. A test that
forgets to patch one fails loudly rather than sending.
Tests that need to exercise a send path patch these themselves, which is the
point: the escape hatch becomes explicit in the test that takes it. The idiom is
lifted from tests/test_warm_content.py, which already did this for one call.
test_send_safety.py asserts the guarantee, because a block like this fails in
only one direction -- weaken it and every other test still passes, the sole
symptom being a message delivered to somebody.
Full suite: 457 passed, 8 skipped. Nothing relied on a live transport.
pages.yaml's title/description are interpolated as plain strings into <title>
and <meta name="description">, so html/template escapes them. Writing an HTML
entity in the YAML therefore escapes it a second time and the user sees the
source. Live on main right now:
<meta name="description" content="... a &plusmn;7-day seasonal window ...">
<title>Weather &amp; climate glossary: ...</title>
which renders as a literal "±7-day" in the SERP snippet and "&" in
the browser tab, on the about and glossary pages. The Python->Go rewrite did not
change this: the entities live in the shared data file and both engines
autoescape identically.
glossary.yaml is deliberately NOT touched. Its `body` is typed template.HTML and
rendered raw (glossary_term.html.tmpl), so the entities and <b> tags there are
correct and would break if "fixed".
The regression test runs against the real content dir, which the frontend
Dockerfile already copies into the builder stage, so it gates the image rather
than only local runs.
The SELECT guard was a denylist of DDL/DML verbs. DuckDB's file readers and
setting introspection are plain SELECTs, so it passed all of these:
SELECT * FROM read_csv_auto('/etc/passwd') -> 200, file contents
SELECT * FROM glob('/etc/*') -> 200, directory listing
SELECT current_setting('s3_secret_access_key') -> 200, the bucket credentials
The last one is the worst: bucket mode installed the S3 credentials with
SET s3_secret_access_key, and settings are readable back out. So one
unauthenticated POST returned the ERA5 bucket key in plaintext -- precisely what
the module docstring says this service exists to avoid ("without shipping S3
credentials to every web task"). /query had no authentication of any kind, while
the comparable /internal/* router has gated its whole surface since it landed.
Three changes, outermost first:
- /query now requires the shared internal token, reusing internal_routes'
dependency. Nothing calls it: the only in-repo caller of this service is
era5lake.py hitting /history, and Centralis reaches the lake through its own
DuckDB rather than this endpoint. /history is deliberately left open for now --
gating it means threading the token through the web->lake hot path.
- Credentials are installed as a DuckDB SECRET instead of SET s3_*. Secret values
are opaque: current_setting() returns NULL and duckdb_secrets() lists the name
and scope but no key material.
- Bucket mode disables LocalFileSystem after the views are created (they resolve
lazily, so ordering matters) and locks the configuration. Local mode cannot do
this -- its views read local parquet -- so the function denylist stays as
defence in depth for dev and tests.
The new guard tests assert the *guard* refused, not merely that the request
failed: several of these shapes (read_parquet on a text file, a bare
'/etc/passwd') error inside DuckDB anyway, so a status-only assertion would keep
passing with the guard deleted. That is the trap the original four-case test fell
into -- it tested negatives shaped like the regex that had been written.
shell-lint failed on two single-quoted strings. Both are deliberate and
double-quoting either one would break it silently rather than loudly.
In render_centralis_secrets, the verification step runs `bash -c 'set -a; . "$1"'`
with the rendered file passed as an argument. The "$1" must be expanded by that
inner shell. Double-quoted, it would interpolate the OUTER $1 — the function's
own first argument, the repo root — so the check would source the wrong path and
still report success. That check exists precisely to catch a bad render, so a
false pass there is worse than no check.
In verify-centralis-render.sh, the string is executed by the REMOTE shell over
ssh. Double-quoted, mktemp would run locally and the script would send the host
a path that does not exist there.
Suppressed with the reason at each site rather than loosened globally.
Claude-Session: https://claude.ai/code/session_0182KTMrsTHJc3TcewCatJFY
Brings the last two credential stores under SOPS, so all three environments and
the control plane are managed the same way.
dev.yaml — dev was not "intentionally not wired in", it was broken
The vault README claimed dev had no real secrets. Both halves were false.
Dev needed secrets it did not have: thermograph-dev-daemon-1 has been in a crash
loop, restarting every ~60s with "neither THERMOGRAPH_INTERNAL_TOKEN nor
THERMOGRAPH_AUTH_SECRET is set; refusing to start" — so dev has had no Discord
gateway and no job timers. With THERMOGRAPH_BASE_URL unset the app also falls
back to https://thermograph.org, so dev's IndexNow pings and verification links
claimed to be prod. dev.yaml gives it its own generated AUTH_SECRET and a real
base URL, and 12 values total.
And dev already held production credentials: backend-deploy-dev.yml injects
S3_ACCESS_KEY/S3_SECRET_KEY into every dev deploy, and the only provisioned
keypair for that bucket is read-write — on the bucket holding prod's backups.
Dev does not need them; without bucket creds the lake service 503s and history
falls through to the archive. Those workflow lines still need removing.
dev renders dev.yaml ALONE, via THERMOGRAPH_SECRETS_SKIP_COMMON=1. Eleven of
common.yaml's sixteen values are live production credentials, the render is a
plaintext concatenation consumed through env_file:, and dev is the operator's
desktop AND the Forgejo runner executing unreviewed dev-branch code with the
docker socket mounted. An override in dev.yaml would not help — last-wins
governs consumers, but prod's value is still physically a line in the file.
Verified: current renderer leaks 28 lines onto dev including both S3 keypairs
and the VAPID private key; patched gives exactly 12 keys and no prod credential.
Beta's render is byte-identical either way.
centralis.prod.yaml — its own file, its own renderer
/etc/centralis.env was hand-edited, which is how a JSON registry got written
unquoted into a shell-sourced file tonight and silently collapsed three
identities to one. The renderer now owns the file.
It is service- AND environment-scoped, not folded into prod.yaml, because
/etc/thermograph.env is loaded into every container in the app stack. Folding
these in would put CENTRALIS_AUTH_TOKEN and CENTRALIS_TOKENS — which
authenticate an endpoint carrying run_on_host and sql_query(write) — into the
web backend's environment, turning a read-anything bug in the app into a
foothold on the control plane.
Quoting is guaranteed three ways rather than assumed. Critically, `sops -d
--output-type dotenv` emits values verbatim, so reusing the existing render path
would have reproduced tonight's bug exactly. The new function decrypts to JSON
and emits POSIX single-quoted assignments; it then sources its own output in a
clean shell and compares every value before touching /etc; and it refuses to
write unless CENTRALIS_TOKENS parses as a non-empty subject->token object after
sourcing. Tested against 16 hostile values including embedded quotes, newlines
and ;rm -rf /.
Value-identity proven, not asserted: rendered vs live compared as *effective*
values on prod (a textual diff would report a false difference, and would report
a false match if the vault had captured quote characters as part of the value),
plus both env files producing a byte-identical `docker compose config` digest.
9 keys, both token subjects preserved. Nothing deployed.
Note for later: /etc/thermograph.env is also shell-sourced, and survives on raw
dotenv only because none of its 32 current values contains a shell-special
character. It is one quoted secret away from the same bug.
Claude-Session: https://claude.ai/code/session_0182KTMrsTHJc3TcewCatJFY
Two changes to how credentials are stored and distributed.
1. common.yaml now exists.
The renderer has always concatenated common.yaml then <env>.yaml, host winning,
and the README has always documented common.yaml with a checkmark. The file was
never created. So every value shared by prod and beta was duplicated in both
vaults, free to drift apart with nothing to detect it.
16 values move to common.yaml: VAPID keypair, metrics token, IndexNow key,
REGISTRY_TOKEN, the S3 endpoint/bucket and both S3 keypairs, plus shared config.
5 stay per-host because their values genuinely differ (APP_CPUS, DB_CPUS,
DB_MEMORY, WORKERS, THERMOGRAPH_BASE_URL). 8 exist only on prod — the Discord
and mail credentials, which beta does not have at all.
Three more are held back deliberately despite being identical today:
POSTGRES_PASSWORD, THERMOGRAPH_AUTH_SECRET, THERMOGRAPH_DATABASE_URL. These are
the credentials that let one environment act as another, and beta is the more
exposed box — it serves public Forgejo and Grafana. They match only because beta
was seeded from prod. Keeping them per-host costs one line each and preserves
the ability to diverge; putting them in common.yaml would encode the equivalence
as intentional and make breaking it a migration rather than an edit. Reasoning
recorded in the vault README, since a future reader will otherwise "fix" it.
Done without any plaintext leaving prod: ciphertext was shipped up, decrypted
against the host's age key, recombined, re-encrypted, and shipped back. The
consolidation refuses to write unless it has proved merged(common + env) equals
the original env vault exactly — same keys, same values — and re-verifies from
the written files afterwards. Both checks passed for prod and beta.
2. render_thermograph_secrets no longer fails open.
One `return 0` covered two different situations: "this host is not configured
for SOPS" (true of the LAN dev box, and correct to succeed) and "this host IS
configured but its vault is not where we looked". The second is a failure, and
returning 0 made it a silent no-op wearing a success code — a deploy would
report success having rendered nothing, and the host would keep serving whatever
/etc/thermograph.env already held, including after a rotation.
The likely cause is passing the wrong root: the function wants the directory
containing deploy/secrets, which on the hosts is /opt/thermograph/infra, not
/opt/thermograph. That mistake looked exactly like "not configured here", which
is why it went unnoticed. It now exits 1 and names the probable cause.
Claude-Session: https://claude.ai/code/session_0182KTMrsTHJc3TcewCatJFY
The previous commit swept in .claude/worktrees/city-resolver and
.claude/worktrees/thermograph-mentions as embedded git repositories. Those are
other Claude sessions' live checkouts and have no business in this tree.
Ignoring the directory so `git add -A` cannot do it again — the repo already
has a documented history of parallel sessions colliding through shared
checkouts, and this is the same hazard wearing a different hat.
Claude-Session: https://claude.ai/code/session_0182KTMrsTHJc3TcewCatJFY
`grep -oE "^[A-Z_]+="` cannot match a key name containing a digit. This estate
has exactly six such names — all the *_S3_* keys — so a prod audit returned 26
keys against 32 real ones, and reported the S3 and lake credentials as missing
from both live hosts.
They were present the whole time. The phantom was independently reproduced
twice by re-running the same pattern, which is what made it convincing, and it
was briefly recorded as one of two root causes of the lake being unqueryable
(#56). That issue has one cause: the missing duckdb-lake image.
An audit that under-reports is worse than no audit. A missing-secret finding
sends someone to provision a credential that already exists, and in a rotation
tool it would justify writing over one.
Fixed to `^[A-Z][A-Z0-9_]*=` in both the skill and key_gaps.py's docstring. The
match still stops at the `=`, so no value is read — that property is the reason
this grep exists rather than a parser.
Centralis's secrets_gaps carried the same bug and is fixed separately, with a
regression test.
Claude-Session: https://claude.ai/code/session_0182KTMrsTHJc3TcewCatJFY
The estate had zero alert rules. The only contact point was Grafana's factory
default pointing at the literal string <example@email.com>, and beta's
untracked compose override routed Grafana's SMTP at prod's Postfix — so
alerts, had any existed, would have been delivered by the box most likely to
be on fire. Prod served 86 5xx in 24h including five /healthz failures and
nobody was told.
Alerting (deploys to beta, which is where Grafana runs):
- 12 Loki-based rules. There is no Prometheus in this estate and Loki is the
only datasource, so every rule is log-derived.
- Thresholds come from a 24h backtest that happens to contain a real ~20min
prod outage at 04:20Z. Absolute counts, not ratios: prod runs ~7 req/min, so
one bad request is 1.4% and a ratio alert would scream all night. The 5xx
burst rule fires on the outage's 45 and 22 buckets and on nothing else in
the day; the largest benign bucket all day was 5.
- Routed to a new private #ops-alerts channel, not to any existing channel —
#weather-events, #announcements and #prod are product surfaces that notify
real subscribers.
- AlertingWatchdog is a dead-man's switch; its value is its absence.
- Delivery is proven, not assumed: the rules were provisioned into a throwaway
Grafana against beta's live Loki and a real alert arrived in Discord. This
matters because GET /api/v1/provisioning/contact-points returns [REDACTED]
for the URL — a contact point holding an uninterpolated env var looks
perfectly healthy and pages nobody. The only proof is a message arriving.
CI gains a structural check, because the existing one only proves YAML parses:
an alert rule whose condition names a missing refId is valid YAML, provisions
cleanly, and never fires. It also hard-fails on a literal Discord webhook in
the repo. Verified against all three breakages deliberately introduced.
Postfix supervision:
- The 13h outage was a boot-ordering race, not a Docker renumbering: postfix
started at 08:09:49, wg0 came up at :51, postfix fataled at :52 on a missing
docker_gwbridge address, and dockerd did not finish starting until 08:10:16.
Stock postfix@.service is ordered only After=network-online.target and ships
no Restart=, so one lost race became a permanent outage.
- An ExecStartPre gate now blocks up to 60s until every inet_interfaces
address actually exists, which absorbs the transient case inside a single
start attempt. That makes bounded retry correct: 5 attempts in 600s, then
failed — a genuinely broken config reaches a visible failed state in ~100s
instead of re-fataling every 15s forever.
- A 5-minute watchdog timer retries indefinitely and runs reset-failed, so
"failed" still self-heals. Worst case is ~5 minutes, not 13 hours.
- Wants=, not Requires=: a dockerd failure must not take down the loopback and
mesh listeners that do not depend on Docker at all.
Health checks must read config with `postconf -c`, never postmulti/postqueue/
postfix — those three RESOLVE inet_interfaces and so fatal precisely when an
address is missing, which made the first version of this check report
status=ok bound=0/0. A health check that fails open is worse than none.
DEPLOY.md carries the monitoring contract, including that systemctl is-active
postfix is a known-false signal: postfix.service is a wrapper whose
ExecStart=/bin/true, so it reports active forever while the real postfix@-
instance is failed with zero listeners. Reproduced live.
Known gap, documented not fixed: Alloy ships Docker stdout, Caddy files and
app JSONL, not journald — so no Postfix line reaches Loki and the mail rules
cannot fire until loki.source.journal is added.
Claude-Session: https://claude.ai/code/session_0182KTMrsTHJc3TcewCatJFY
After a cell's archive advances, the first content request recomputes a 45yr payload cold. Add warm_cities.warm_content — idempotent, cache-only, bounded — that pre-populates the content derived-store under the same (kind,key,content_token) the routes read, wired into the leader-gated notifier loop (50 cities/30-min tick) so the ~1000-city set stays warm within hours of each daily advance.
The three content handlers loaded the full ~45yr archive before checking the cache, so even hits paid the load. Restructure to resolve cell + compute content_token cheaply first, check the store, and load history only inside the miss-only build() closure. Cache hits and 304s now return without touching the archive. Keys/kinds/ETag unchanged.
Content pages keyed cache validity on the full-history hist_end, invalidating ~1-2x/day and paying a 45yr load even to compute the token. Add content_token(cell_id) = PAYLOAD_VER:CONTENT_VER:max_date, backed by an indexed MAX(date) (climate_store.history_max_date / climate.history_max_date), so it survives intra-day top-ups and needs no full-history load.
main took two direct PRs (#19 shell-lint, #21 the thermograph-daemon Go
service) while dev accumulated the ERA5 lake stack; both sides added compose/
stack services and touched the same seams. Resolutions keep both worlds:
- compose + Swarm stack: lake AND daemon are sibling services; backend env
carries THERMOGRAPH_LAKE_URL and THERMOGRAPH_INTERNAL_TOKEN.
- deploy.sh: backend deploys roll (backend lake daemon); all adds frontend.
- requirements.txt: websockets/apscheduler stay removed (moved to the Go
daemon), duckdb stays (the lake role's engine).
Merged tree: backend suite green, both compose configs validate.
The gateway bot and APScheduler were long-lived stateful I/O loops running
inside the async web app under a leader election. They move into a single Go
binary that owns ONLY that I/O -- websocket, RESUME, heartbeat, backoff, timers.
It owns no grading logic. Anything needing data calls back over a new
internal-only surface (/internal/discord/grade, /internal/jobs/*). Grading
depends on polars and the parquet cache; reimplementing it in Go would let the
bot's grades drift from the API's. The grade route returns gateway-ready JSON
and Go relays the bytes verbatim.
The binary ships in the backend image and runs as a second compose service off
the same tag, so the two ends of the /internal/* contract can never skew.
deploy.sh rolls daemon alongside backend -- without that the service would never
be created, since a single-service deploy uses --no-deps. It also probes the
image first and skips the daemon when rolling a tag that predates the binary:
infra tracks main while image tags are env-staged, so a host can legitimately be
asked to roll an older backend image, and creating the service anyway would
leave a container crash-looping on a missing binary.
replicas: 1 with order: stop-first replaces the leader election -- Discord
permits one gateway connection per bot token.
THERMOGRAPH_INTERNAL_TOKEN is optional: both ends derive it from
THERMOGRAPH_AUTH_SECRET via HMAC under a domain-separation label, so this needs
no new vault entry. The derivation is pinned to a shared cross-language test
vector asserted on both sides, so drift fails CI instead of 401ing every call.
Fail closed when neither secret is set.
Improvements over the Python: a close intended for RESUME uses 4000 rather than
1000 (Discord invalidates a session closed 1000, so the old default defeated its
own resume); MESSAGE_CREATE runs on a bounded worker pool; and a malformed HELLO
returns an error rather than a clean reconnect, which would otherwise reset
backoff and hot-loop against the gateway.
365 Python tests pass; Go build/vet/test -race clean; shellcheck 0 findings.
Adds .forgejo/workflows/shell-lint.yml (pinned shellcheck v0.11.0 + sha256, -x,
default severity, fail on any finding, not path-filtered) and drives all 26
scripts to zero findings.
Two defects shellcheck cannot see:
render-secrets.sh left DECRYPTED vault contents in /tmp whenever a sops decrypt
failed -- the caller's set -e aborted the function before either cleanup ran.
Now removed on every exit path, with `|| return 1` on both sops calls so a
decrypt failure can never write a partial /etc/thermograph.env regardless of the
caller's shell options. Explicitly not a `trap ... RETURN`: such a trap set in a
sourced function persists into the caller's shell and re-fires when the caller's
next `.`/source completes, where the function-local tmp is unset -- fatal and
silent under deploy.sh's set -u. The file now records that reasoning.
autoscale.sh ran `set -eu` without pipefail while piping docker stats into awk,
so a failed left side was swallowed and the loop autoscaled on empty input.
Promoted to pipefail with a missed sample treated as a skip; verified busybox ash
in docker:27-cli supports it.
Also: capture-fixtures.sh's `jq . || cat` ran cat after jq had consumed stdin,
silently writing truncated fixtures; deploy.sh/deploy-stack.sh `# shellcheck
source=` paths corrected for the monorepo layout.
The RETURN trap added in the previous commit leaked out of the function and
killed every deploy on a SOPS-configured host.
A RETURN trap set inside a SOURCED function is not function-scoped: it persists
in the caller's shell after the function returns, and a RETURN trap also fires
when a `.`/source completes. deploy.sh sources /etc/thermograph.env six lines
after calling render_thermograph_secrets, which re-fired the trap at top level
where `tmp` -- function-local -- is unset. Under deploy.sh's `set -u` that is
fatal, and silent: that line already sends stderr to /dev/null, so the deploy
rendered secrets and then died with no diagnostic before pulling or rolling
anything.
Replaced with explicit `rm -f "$tmp"` on each exit path, plus a comment
recording why the tidier-looking trap is wrong here so it doesn't come back.
The original defect the trap was meant to fix stays fixed: the decrypt-failure
path removes the plaintext temp file before returning 1.
The write section now captures its status in `rc` and cleans up once, rather
than ending on `rm` -- as the last command it was masking a failed in-place
`cat` write to status 0, so a half-written /etc/thermograph.env would have
deployed as if it succeeded.
Verified in a container against the real call pattern (strict-mode caller,
source lib, call, then source the rendered env): success path returns 0 and the
caller survives the subsequent source; decrypt-failure path aborts the caller
with no /etc/thermograph.env written; both leave zero temp files.
No static analysis has ever run over the ~2k lines of shell that deploy,
provision secrets, and bootstrap hosts as root over SSH. Add shell-lint.yml
(pinned shellcheck v0.11.0 + sha256, -x, default severity, fail on any
finding) and fix everything it reports, plus two defects it structurally
cannot see.
Not path-filtered, matching secrets-guard's call: a backstop that only runs
when you expect it to isn't a backstop. Scripts are discovered with find, so
new ones are covered on landing. The version is pinned to a static release
rather than apt's, so a drifted shellcheck can't fail CI on an unrelated push.
render-secrets.sh: the mktemp holding DECRYPTED vault contents was only
removed on the success path and one failure branch, so a sops decrypt failure
left plaintext POSTGRES_PASSWORD in /tmp indefinitely on a live host. A RETURN
trap makes removal unconditional, and the two sops calls now `|| return 1`
explicitly instead of relying on the caller's set -e (a bare set -e abort
skips the trap). The function stays free of `set -e` itself -- it is sourced,
and shell options would leak into the caller.
autoscale.sh: ran `set -eu` without pipefail while piping docker stats into
awk, so a failed left side was swallowed and the loop scaled on empty input.
Promoted to pipefail with avg_cpu's failure treated as a missed sample, so a
daemon hiccup can't kill the autoscaler. Verified busybox ash in docker:27-cli
supports pipefail and the script still parses there.
capture-fixtures.sh: `jq . || cat` ran cat after jq had already consumed
stdin, silently writing a truncated fixture; now a real if/else that fails
loudly. deploy.sh/deploy-stack.sh: `# shellcheck source=` paths corrected for
the monorepo layout, and /etc/thermograph.env marked unfollowable (it is
rendered at deploy time and cannot exist at lint time).
