fix(validate): run the five checks that were CI-only, from one copy each (#175) - #317
Merged
Conversation
…ach (#175) `validate.sh` claims "Everything CI runs, runnable locally" and five checks lived as inline shell in ci.yml with no local equivalent. The SOPS assert is the one that mattered: a file that is not encrypted is a committed plaintext secret, and the only thing that looked was a job you cannot consult before pushing — after which it has to be purged from history rather than reverted. Three image greps fold into check_image_pins.py, which already runs in both places and whose docstring already explains why a grep cannot see a pin that is simply absent (#65). They are now beside the parser that argument is about. Scope is `git ls-files`, not a walk of `.`. The shell versions walked the tree, which is right in CI's clean checkout and wrong here: this repository keeps git worktrees under .claude/worktrees/, so a local run would have descended into full copies of itself and reported another branch's findings as this one's. Tracked is also the right question — an untracked scratch file pinning :latest harms nobody. The floating-tag pattern keeps the original's blind spot exactly: `FOO=alpine:latest` is missed there too, because both require start-of-line or whitespace before the name. Parity, not a regression, and still a real gap. The digest check reads the parsed compose file rather than an awk over `image:` lines, so a quoted or flow-style value is not a hole, and it fails rather than passes when it finds no compose files at all. The dashboard PromQL parse uses the promtool binary-or-docker fallback the file already has, with a counted skip when neither is reachable. Its temp file is written inside REPO_ROOT, because the docker fallback bind-mounts REPO_ROOT and nothing else, and chmod 0644, because mktemp makes it 0600 and the reader is then the image's own user — which fails as "permission denied" and looks exactly like a broken query. The tracked-artefact check was written out twice and the copies had drifted. validate.sh's missed a second stack's .env, a nested .rendered/ and a certificates/ anywhere but the repository root — all three now caught, and .env.example still allowed. One script, both callers, second copy deleted. Co-Authored-By: Claude Opus 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
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.
Closes #175. All five checks now run locally, from one implementation each.
scripts/check_image_pins.pyscripts/check_image_pins.pyscripts/check_image_pins.pyscripts/validate.shscripts/check-sops-encrypted.shPlus
scripts/check-tracked-artefacts.sh, replacing the two divergent copies.The three greps went where the issue suggested
check_image_pins.pyalready runs in both places, and its docstring already explains why a grep cannot see a pin that is simply absent (#65). The greps that paragraph is about now sit beside the parser, so it is checkable rather than a note about code elsewhere.Scope is
git ls-files, not a walk of.— the one deliberate change in behaviour. The shell versions walked the tree, which is right in CI's clean checkout and wrong locally: this repository keeps git worktrees under.claude/worktrees/, so a local run would have descended into full copies of itself and reported another branch's findings as this one's. Verified: an untracked:latestunder.claude/produces zero hits. Tracked is also the right question, since an untracked scratch file pinning:latestharms nobody.The floating-tag check keeps the original's blind spot on purpose.
FOO=alpine:latestis not caught — both the old grep and this require start-of-line or whitespace before the name. Confirmed identical against the same input. Parity rather than regression, and still a real gap if anyone wants it closed separately.The digest check reads parsed YAML rather than
awk '$1 == "image:"', so a quoted or flow-style value is not a hole, and it fails rather than passes when it finds no compose files.The tracked-artefact divergence
The issue's table, re-tested against the new single implementation:
stacks/observability/.envstacks/other-stack/.envsome/other/.rendered/xnested/certificates/key.pemstacks/observability/.env.exampleThe patterns are unanchored, which is where the old copy went wrong:
^certificates/matched only at the repository root, and a secret does not become safe by being nested.Two things that bit during the work
chmod 0644.mktempcreates 0600, and whenPROMTOOLis the docker fallback the reader is the image's own unprivileged user — it fails aspermission denied, which looks exactly like a broken query and is not one. It is also written insideREPO_ROOT, because that fallback bind-mountsREPO_ROOTand nothing else.check-sops-encrypted.shfails when the glob matches nothing, rather than reporting "all encrypted" over zero files. That is the--emit-promqlguard's argument (make validate and CI have drifted apart in both directions #68) applied to the one artefact whose absence would otherwise be silent.Checks
validate.sh— 48 PASS (up from 45), 0 FAIL, 2 SKIP, the usual environmental two. New lines:Each folded-in check was also tested against a violation in a throwaway clone: a version pin outside compose.yaml, a space-preceded
:latest, and a digest stripped from a real image were all caught with the right message.🤖 Generated with Claude Code