Harden the supply chain, fix the purge tool, add log-based alerting - #5
Merged
Conversation
… purges
Running the dry run against real history exposed three defects in this script,
none of which were visible by reading it.
1. The script embedded the leaked SNMP community as a literal, and the runbook
quoted it three more times. So a *successful* history purge left the secret
sitting in the working tree — inside the tool written to remove it. The
comment above the literal even claimed the opposite ("kept here ... so the
string itself is not re-committed"), which was exactly backwards: the script
is tracked, so the string was committed.
Literals now come from a gitignored .purge-secrets.txt written by the
operator at purge time, via --secrets-file. The repository no longer contains
the string anywhere.
2. .gitleaks.toml allowlisted both files by path, so the scanner was configured
not to report a leak that was genuinely there. Those two entries are gone.
Only the gitignored secrets list is allowlisted now.
3. The verification step failed open. It detected leaks via the exit status of
git rev-list --all | xargs -I{} git grep -q <literal> {}
but xargs returns 123 when any invocation exits 1-125, so a literal present
in some commits and absent from others made the pipeline non-zero and the
check reported clean. Measured on this repository: a string present in 17 of
49 commits was reported absent. Under `set -o pipefail` the same 123 also
aborted the script before the check printed anything.
Detection is now by output presence with the pipeline guarded, it scans the
rewritten worktree as well as history, and it names the offending file.
Verified both directions rather than only the happy path: purging a string that
genuinely survives the rewrite now reports FAIL and points at
scripts/purge-history.sh; purging the real community string reports PASS. The
real repository is untouched by the dry run (50 commits before and after).
The Loki ruler was configured and pointed at Alertmanager but had no rules, so
the log pipeline was a search box rather than something that could tell you
anything unprompted.
Adds 8 rules in loki/rules/security.rules.yaml covering conditions that only
exist in logs: SSH brute force (warning at 20 failures in 5m, critical at 100),
SSH accepted from outside VLAN 50/99, repeated sudo failures, user and group
creation, kernel OOM kills, read-only remounts, and disk I/O errors. They carry
the same severity and category labels as the Prometheus rules, so Alertmanager
routing and inhibition are shared with no extra configuration.
Two things this surfaced:
* Loki's local ruler reads <directory>/<tenant>/ and, with auth_enabled
false, the tenant is literally "fake". Getting that path wrong produces no
error at all — just a ruler that silently evaluates nothing. The ruler
directory moved out of /loki so the read-only rule mount does not collide
with the data volume.
* A rule alerting on a host that stops logging was written and then removed.
When a host goes quiet it produces no series, so `rate(...) == 0` never
evaluates for it, and LogQL has no way to supply a known-host list. The
global case is already covered by LokiIngestionStalled on the Prometheus
side.
Validation: promtool cannot check these — it parses PromQL and rejects every
LogQL stream selector. scripts/check_loki_rules.sh boots the pinned Loki image
with the rules mounted, fails on a parse error, and then asserts the ruler
actually evaluated them, since silence is not evidence when the alternative is
that it never read the files.
`loki -verify-config` is deliberately not used as the check: it validates the
config file and never opens the rule files. Confirmed by experiment — a rule
file containing `count_over_time({{{BROKEN` passes -verify-config and is caught
only by the boot check.
Verified in both directions against the real Loki v3.3.2 binary: all 8 rules
parse and are evaluated by the ruler across both groups, and deliberately
breaking one LogQL expression makes the checker exit 1 and name the rule.
There was a problem hiding this comment.
Pull request overview
This PR hardens the repository’s git-history purge workflow to avoid reintroducing secrets during rewrites, and adds Loki ruler-based (LogQL) alerting with CI validation so log-only security/system conditions can page via the existing Alertmanager routing.
Changes:
- Refactors
scripts/purge-history.shto read purge literals from a gitignored.purge-secrets.txtand improves post-rewrite verification. - Adds Loki log-based alert rules and wires the ruler rule directory/tenant mount correctly in Loki config + compose.
- Introduces
scripts/check_loki_rules.shand runs it from bothscripts/validate.shand CI.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| stacks/observability/loki/rules/security.rules.yaml | Adds 8 LogQL alert rules for auth/system/security signals. |
| stacks/observability/loki/loki-config.yaml | Moves local ruler rules directory to /etc/loki/rules with tenant-path commentary. |
| stacks/observability/compose.yaml | Mounts rules into /etc/loki/rules/fake to match the auth_enabled: false tenant. |
| scripts/validate.sh | Runs the new Loki rules validation script during local validation. |
| scripts/purge-history.sh | Removes hardcoded secret literal, reads from .purge-secrets.txt, and improves leak detection/reporting. |
| scripts/check_loki_rules.sh | New script to boot Loki and validate rule parsing/evaluation. |
| Makefile | Adds check-loki-rules target. |
| docs/runbooks/purge-git-history.md | Updates purge runbook to use .purge-secrets.txt-based workflow. |
| docs/roadmap.md | Marks Loki rules work item complete. |
| docs/observability.md | Documents Loki ruler alerting, tenant path behavior, and the validation approach. |
| .gitleaks.toml | Updates allowlist paths (removes prior allowlist entries; adds .purge-secrets.txt). |
| .gitignore | Ignores .purge-secrets.txt. |
| .github/workflows/ci.yml | Adds CI step to validate Loki rules. |
Suppressed comments (2)
docs/runbooks/purge-git-history.md:69
- This
git log -S "$(cat .purge-secrets.txt)"check has the same multi-line issue as above: it won’t work if.purge-secrets.txtcontains more than one literal. Loop over each literal so the “must be empty” assertion is meaningful.
cd /tmp/<scratch>/repo
git log --oneline | head
git log --all -S "$(cat .purge-secrets.txt)" --oneline # must be empty
**docs/runbooks/purge-git-history.md:117**
* `git grep -I -e "$(cat .purge-secrets.txt)" ...` will not work with a multi-line secrets file (newlines become part of the pattern), and it also treats the literal as a regex. Loop over the file and use `-F` so secrets with regex metacharacters are checked correctly.
git log --all --oneline -- certificates/ # empty
git grep -I -e "$(cat .purge-secrets.txt)" $(git rev-list --all) # no matches
gitleaks detect --no-banner --redact -c .gitleaks.toml --log-opts="--all"💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # | ||
| # Usage: scripts/check_loki_rules.sh | ||
|
|
||
| set -uo pipefail |
Comment on lines
+36
to
+38
| # Rewrite every path in the real config to point inside the scratch dir, so the | ||
| # rules are checked against the same settings production uses. | ||
| python3 - "$STACK/loki/loki-config.yaml" "${WORK}" > "${WORK}/loki.yaml" <<'PY' |
Comment on lines
+77
to
+83
| # Confirm the ruler actually loaded them. A silent "no errors" is not evidence | ||
| # if the ruler never read the files at all. | ||
| evaluated="$(grep -o 'rule_name=[A-Za-z]*' "${OUT}" | sort -u | wc -l)" | ||
| if ((evaluated == 0)); then | ||
| printf '\033[0;31m FAIL\033[0m the ruler evaluated no rules — check the tenant path\n' | ||
| exit 1 | ||
| fi |
Comment on lines
+109
to
+114
| # promtool cannot check these — it parses PromQL and rejects every LogQL | ||
| # stream selector. Loki itself is the only thing that understands them, | ||
| # so the checker boots the pinned image with the rules mounted. | ||
| - name: Validate Loki rules | ||
| run: ./scripts/check_loki_rules.sh | ||
|
|
Comment on lines
+127
to
+134
| hit="$(git -C "${target}" rev-list --all \ | ||
| | xargs -r -I{} git -C "${target}" grep -lI -e "${literal}" {} -- 2>/dev/null \ | ||
| | head -n1 || true)" | ||
| [[ -n "${hit}" ]] && { leaked=1; printf ' history: %s\n' "${hit}"; } | ||
|
|
||
| hit="$(grep -rlI -e "${literal}" "${target}" \ | ||
| --exclude-dir=.git --exclude="${ignore_name}" 2>/dev/null | head -n1 || true)" | ||
| [[ -n "${hit}" ]] && { leaked=1; printf ' worktree: %s\n' "${hit}"; } |
| ```bash | ||
| git show 647d90a~1:certificates/Gandalf.Gondor.Lab/ca-key.pem | head -1 | ||
| git log --all -S '7H3r315N05p00N' --oneline | ||
| git log --all -S "$(cat .purge-secrets.txt)" --oneline |
Comment on lines
+51
to
+53
| # The list of literals to purge, written by the operator at purge time. | ||
| # Gitignored; never committed. | ||
| '''\.purge-secrets\.txt$''', |
CI failed with 'the ruler evaluated no rules'. The guard did its job — it
refused to report success when nothing had been validated — but the cause was
the harness, not the rules.
mktemp -d creates a 0700 directory owned by the invoking user, and the Loki
image runs as uid 10001. In CI the container therefore could not traverse its
own working directory, exited after three seconds, and evaluated nothing. The
local run passed because the loki binary ran as root against the same paths.
Fixes:
* chmod -R a+rwX the throwaway directory, applied again after the generated
config is written so the container can read that too. Confirmed the barrier:
the directory is 0700 before and 0777 after.
* Distinguish the two failure modes in the output. Exit status 124 means loki
ran the full window and genuinely evaluated nothing (a tenant path problem);
anything else means it died early. Either way the last 15 log lines are now
printed, so the next failure explains itself instead of needing a local
reproduction.
* Guard the PyYAML import. It is not guaranteed on a clean runner and the
failure without this is an opaque ModuleNotFoundError inside a heredoc.
Verified: shellcheck clean, the check still passes against the real Loki v3.3.2
binary (8 rules parsed, 8 evaluated), and the short-boot path prints the
diagnostic rather than a bare failure.
All correct, and one of them repeats the exact mistake this PR was opened to fix. * .gitleaks.toml allowlisted .purge-secrets.txt by path — the same "teach the scanner to ignore a real finding" pattern the purge fix removes. Entry gone. Worth being precise about what that does and does not buy: removing it does not make gitleaks catch an accidental commit of that file. It is gitignored, so the filesystem scan skips it, and it holds bare literals with no keyword context for any rule to match. Verified by force-adding it and re-scanning — still clean. The real control is an assertion that the file is never tracked, now in both CI and scripts/validate.sh alongside the existing .env and .rendered/ checks, and confirmed to fire. * Leak detection used grep without -F. A rotated community is a random string and may contain regex metacharacters; without -F those are either mis-parsed or make grep error out, and a grep that errors reports no match — one more way to fail open, which is the failure mode this script already had once. Both the history and worktree scans now use -F. Verified with the literal a[b(c).*+?d. * The runbook used `git log -S "$(cat .purge-secrets.txt)"`, which folds a multi-line file into one search string and matches nothing. The file format is documented as one literal per line, so all three call sites now loop, and the git grep example uses -F. * check_loki_rules.sh ran under `set -uo pipefail` without -e, so a failed cp or config rewrite could still reach a cheerful PASS. Now `set -euo pipefail`, with the one command expected to fail — the timeout that kills Loki — guarded explicitly. * The evaluated-rule count matched rule_name=[A-Za-z]*, which can match empty and misses digits and underscores in alert names. Now anchored to a valid identifier with the optional opening quote stripped. The PyYAML preflight raised in review was already fixed in ddd4d71. Re-verified after the changes: shellcheck clean across all five scripts, the Loki check still passes against the real v3.3.2 binary (8 parsed, 8 evaluated) and still fails on a deliberately broken expression, and the purge dry run passes for both a metacharacter-laden literal and the real one.
…erval The permission fix worked — CI shows Loki running the full 45s and shutting down cleanly, so it was reading its config. It still reported no rules evaluated. Cause is timing, not configuration. The rule groups use interval: 1m, and Loki jitters a group's first evaluation across its interval, so a 45s window can legitimately see none. Locally the jitter happened to land early, which is why it passed here and failed there — the check was non-deterministic in a way that made a real timing issue look like a broken tenant path. The throwaway copies of the rules now have their group interval rewritten to 5s before Loki starts. The committed rules keep interval: 1m; only the scratch copy is touched, and the LogQL is never modified. A 25s window is now sufficient where 75s was previously marginal, so the 45s default has real headroom. Also: the container now runs with --user "$(id -u):$(id -g)". Without it Loki writes its WAL and index as uid 10001, the runner cannot delete the scratch directory, and cleanup floods the log with "rm: Permission denied" immediately after the verdict. The cleanup trap is non-fatal regardless, so a cleanup failure can never mask the result. Verified: 8 rules parsed and 8 evaluated within a 25s window, the deliberately broken expression still exits 1 and names the failing alert, and the committed rules file is unchanged at interval: 1m.
…ing them Dependabot merged version bumps into main while this branch was open, which exposed a design flaw in the CI added in #1: the image pins were duplicated across compose.yaml, ci.yml, validate.sh and check_loki_rules.sh, and Dependabot only updates compose.yaml. The result after those merges: compose.yaml prom/prometheus:v3.13.2 grafana/loki:3.7.4 grafana/alloy:v1.18.0 ci.yml prom/prometheus:v3.1.0 (n/a) grafana/alloy:v1.6.1 scripts prom/prometheus:v3.1.0 grafana/loki:3.3.2 grafana/alloy:v1.6.1 CI was set to validate configs against versions two years older than the ones the stack deploys, and would have reported green while doing it. Validating the wrong version is worse than not validating, because it still produces a passing check. scripts/image-for.sh now reads the pin straight out of compose.yaml, and ci.yml resolves the images into $GITHUB_ENV at run time rather than declaring them. compose.yaml is the single source of truth, so a Dependabot bump automatically reaches every check. The Makefile's snmp-generator is derived from the snmp-exporter pin — the two are released together and the generator is not a compose service, so it cannot be looked up directly. A new CI step fails on any pinned image version outside compose.yaml, so the duplication cannot creep back in. The gitleaks image stays declared in ci.yml because it is a CI tool rather than part of the stack. Verified against the versions actually deployed, not the ones I had lying around: the Loki rules parse and evaluate on 3.7.4 (8/8), loki-config.yaml is valid on 3.7.4, and config.alloy passes fmt --test on v1.18.0 with an empty diff against its canonical formatting — so the formatter did not change behaviour between v1.6.1 and v1.18.0. Also corrects comments that named specific tool versions now resolved dynamically.
## Digest pinning
A tag is a mutable pointer. `prom/prometheus:v3.13.2` is whatever the publisher
last pushed under that name, and a tag can be moved — silently, by a compromised
or coerced publisher. A digest is the content hash: it matches or the pull fails.
Every service image is now `repo:tag@sha256:...`, keeping both halves. The tag
stays readable and says which version you are on; the digest is what Docker
enforces. Dependabot understands this form and updates both together.
scripts/pin-digests.sh resolves digests through the registry HTTP API rather
than `docker pull`, so it needs no daemon and downloads no layers. Without
arguments it reports drift and exits non-zero, so it doubles as a check;
--write applies. CI additionally asserts every compose image carries a digest.
Two things this surfaced:
* The Makefile derives prom/snmp-generator from the snmp-exporter pin, and
with digests that silently produced snmp-generator tagged with the
*exporter's* digest — a reference that cannot be pulled. image-for.sh grew
a --tag-only mode for exactly this case.
* The README badges hardcoded Prometheus v3.1.0, Grafana 11.5 and Loki 3.3
while the stack ran v3.13.2, 13.0.2 and 3.7.4 — the same drift class fixed
in the previous commit, in the first thing a reader sees. Version numbers
are gone from the badges rather than left to rot again; there is no
mechanism that could keep them honest.
## SECURITY.md
A security-focused repository with no disclosure policy is a gap. It covers how
to report privately (GitHub Security Advisories), an explicit request not to
probe the live network, the three known historical exposures with their current
status, what the repository deliberately never publishes and why, and the
controls CI enforces.
It documents the known exposure rather than quietly carrying it: a written-down,
accepted exposure reads very differently from an overlooked one.
Verified: digests resolve for all six images and re-running reports them
unchanged; the generator derivation yields prom/snmp-generator:v0.30.1 with no
digest; all 57 relative links across the docs resolve; and the full local sweep
passes including the new digest check.
Gerrrt
changed the title
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.
Five changes. Every defect below was invisible to inspection and only appeared when something actually ran.
1. The history-purge tool was a copy of the secret it purges
Running
make purge-history-dry-runagainst real history exposed three defects in the script shipped in #1.It embedded the leaked SNMP community as a literal, and the runbook quoted it three more times — so a successful rewrite left the secret in the working tree, inside the tool written to remove it. The comment above the literal claimed the opposite ("kept here … so the string itself is not re-committed"), which was exactly backwards: the script is tracked.
.gitleaks.tomlallowlisted both files by path, configuring the scanner not to report a leak that was genuinely there.The verification failed open. It detected leaks via the exit status of
git rev-list --all | xargs -I{} git grep -q <literal> {}.xargsreturns 123 when any invocation exits 1–125, so a literal present in some commits but not others makes the pipeline non-zero and the check reports clean. Measured here: a string present in 17 of 49 commits was reported absent. Underset -o pipefailthat same 123 also aborted the script before it printed anything.Literals now come from a gitignored
.purge-secrets.txt; detection is by output presence with the pipeline guarded, uses-Fso a rotated community containing regex metacharacters cannot slip through, scans the rewritten worktree as well as history, and names the offending file.2. Loki had a ruler and no rules
8 rules in
loki/rules/security.rules.yamlfor conditions that only exist in logs: SSH brute force (warning at 20 failures/5m, critical at 100), SSH accepted from outside VLAN 50/99, sudo failures, user/group creation, kernel OOM kills, read-only remounts, disk I/O errors. Sameseverity/categorylabels as the Prometheus rules, so routing and inhibition are shared. 40 rules total.<directory>/<tenant>/, and withauth_enabled: falsethe tenant is literallyfake. Getting it wrong produces no error — just a ruler that silently evaluates nothing.rate(...) == 0never evaluates for it. A rule that cannot fire is worse than no rule.promtoolcan't validate LogQL.scripts/check_loki_rules.shboots the pinned Loki image with the rules mounted, fails on a parse error, then asserts the ruler actually evaluated them.loki -verify-configis deliberately not the check — it never opens the rule files, confirmed by experiment:count_over_time({{{BROKENpasses it.3. CI was validating the wrong versions
Dependabot's bumps merged to
mainwhile this branch was open, exposing a design flaw in the CI added in #1 — image pins were duplicated across four files, and Dependabot only updatescompose.yaml.v3.13.2v3.1.03.7.43.3.2v1.18.0v1.6.1CI was validating against versions roughly two years older than what deploys — and reporting green while doing it.
scripts/image-for.shnow reads the pin fromcompose.yaml, CI resolves images into$GITHUB_ENVat run time, and a new step fails on any pinned version outsidecompose.yaml.4. Supply chain: pinned by digest
A tag is a mutable pointer — whatever the publisher last pushed, movable silently by a compromised or coerced one. A digest is the content hash.
All six images are now
repo:tag@sha256:…, keeping both halves: the tag stays readable, the digest is what Docker enforces. Dependabot updates both together.scripts/pin-digests.shresolves through the registry HTTP API rather thandocker pull, so it needs no daemon and downloads no layers; bare it reports drift and exits non-zero,--writeapplies. CI asserts every image carries a digest.Two things this surfaced:
snmp-generatorfrom thesnmp-exporterpin, which with digests producedsnmp-generatorcarrying the exporter's digest — unpullable.image-for.shgrew--tag-only.Prometheus-v3.1.0,Grafana-11.5,Loki-3.3against a stack running v3.13.2 / 13.0.2 / 3.7.4 — the same drift as §3, in the first thing a reader sees. Version numbers removed rather than left to rot; nothing could have kept them honest.5. SECURITY.md
Private disclosure via GitHub Security Advisories, an explicit request not to probe the live network, the three known historical exposures with current status, what the repository deliberately never publishes, and the controls CI enforces. The exposure is documented rather than quietly carried — a written-down, accepted exposure reads very differently from an overlooked one.
What the CI failures taught
Both were in the harness, not the rules, and the second is the more interesting.
mktemp -dcreates a0700directory and the Loki image runs as uid 10001, so the container could not read its own config and died in three seconds. Local runs were as root, which hid it.interval: 1m. Locally the jitter landed early and passed; in CI it landed late and failed — a timing artifact masquerading as a broken tenant path. Throwaway copies now getinterval: 5s; committed rules keep1m, LogQL untouched.Review feedback
All seven comments were correct and are addressed. The notable one: I had allowlisted
.purge-secrets.txtin.gitleaks.toml— the same mistake this PR exists to fix. Removed. But removing it does not make gitleaks catch an accidental commit: the file is gitignored so the scan skips it, and it holds bare literals with no keyword context. Verified by force-adding and rescanning. The real control is an assertion that it is never tracked, in both CI andvalidate.sh, confirmed to fire.Verification
CI green on all three jobs. Local sweep green:
yamllint,shellcheck(7 scripts),markdownlint,gitleaks(tree + history),docker compose config, dashboards, digest drift, no-duplicate-pins, and the Loki rules check. All 57 relative links across the docs resolve.Checked against the versions actually deployed, not the ones already downloaded:
loki-config.yamlvalid on 3.7.4.config.alloypassesfmt --teston v1.18.0 with an empty diff against canonical formatting — the formatter did not change behaviour across twelve minor versions.Not verified: the stack still has not been started end-to-end — no Docker daemon in the authoring environment. Loki and Alloy were exercised as standalone binaries, which is what validates the rules and config, but the composed stack has not run.
Note
None of this unblocks the manual work — rotating the SNMP communities, running the purge, and
make secrets-initstill need host and device access. It does make the purge safe to actually run.