From fd78aeb0035440264fefce5d935ca657c55027fe Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 9 Aug 2026 16:10:45 +0000 Subject: [PATCH] Keep secrets out of the Docker build context at every depth (closes #29) The canonical .dockerignore was three lines -- .git, node_modules, .DS_Store -- while the canonical Dockerfile does `COPY . .`, so a developer's local .env, *.pem or *.key was shipped into the build context and could land in an image layer. Nothing surfaced it because .gitignore covers those patterns, so the files are invisible to every git-based check. The obvious repair, copying .gitignore's secret patterns across, is worse than the gap it closes. .dockerignore does not use .gitignore semantics: Docker matches with moby/patternmatcher, which is filepath.Match semantics plus a `**` extension compiled to a regexp. `*` does not cross `/`, and a pattern without a leading `**/` is anchored at the build-context root. A file listing .env, *.pem and *.key therefore reads as solved, reviews as solved, and protects only the repository root, while config/.env and certs/server.key still ship. The three-line file at least invited scrutiny; the transplanted form manufactures confidence and stops anyone looking. So every depth-independent pattern here carries the `**/` prefix and only genuinely root-anchored entries stay unprefixed. `**/node_modules` fixes a defect the three-line file had today for any nested node_modules, independently of the secret exposure. Coverage is not limited to the three patterns the issue names, because the enumeration found more shapes reaching the image. `**/*.env` covers the prod.env / local.env convention, which the .env and .env.* spellings miss entirely; `.envrc` is a secrets file by direnv convention; *.p12 and *.pfx are bundles carrying private keys; and id_rsa, id_dsa, id_ecdsa and id_ed25519 are the extensionless SSH keys that were covered before only when someone happened to append a .key suffix. Matching is case-sensitive, so `**/*.key` does not match certs/SERVER.KEY, which is reachable on the case-insensitive filesystems most laptops use. Doubling each pattern with an ALL-CAPS twin is not the fix: measured against the planted set it still ships certs/Server.Key and certs/Ca.Pem while reading as though case were handled, which is this issue's failure mode restated in a new place. The matcher supports character ranges, so every secret name is written that way -- `**/*.[kK][eE][yY]`, `**/*.[pP][eE][mM]`, `**/.[eE][nN][vV][rR][cC]`, `**/[iI][dD]_[rR][sS][aA]` and the rest. The extensionless SSH keys and .envrc are folded for the same reason the extensions are: on the very filesystems that make SERVER.KEY reachable, direnv reads .ENVRC and ssh reads ID_RSA, so exempting them would have contradicted the rule that justifies the folding. Since `*` matches the empty string, `**/*.[eE][nN][vV]` already covers a bare .ENV and no separate literal .env entry is needed; the literal one is gone rather than left to imply that case is unhandled there. Deliberately not covered: bare `key` and `pem` filenames, which no tool produces and which collide with legitimate paths (a `**/key` pattern would delete an internal/key/ package directory from the context); `**/id_*`, which would match ordinary source such as id_generator.go and ID_MAP.go; and *.crt and *.cer, which are public certificates rather than secrets and are sometimes a legitimate build input. Each of those is planted as a positive control and verified present in the image after the change. The one predictable false positive is a committed env template: `**/*.env` excludes example.env. The remedy travels with the file rather than living in a review thread -- the header comment and the policy both say to re-include it with a negation, `!docs/example.env`, and never to delete the pattern, which would reopen the exposure for everything else it covers. The OS and editor patterns are included on their own merits rather than by mirroring .gitignore. None of them is ever a build input, and editor state in particular churns under a developer's hands, so each one is a source of `COPY . .` invalidation carrying no information about the source tree. Now that the checks are keyed on CHECK_EPOCH rather than on accidental context churn, there is no reason left to keep churn in the context. Language build artifacts are deliberately absent: they are per-repo, and the file's header comment tells consuming repos to add their own host-built binaries, which is the case that actually bites -- a host `make build` drops a multi-megabyte artifact into the context where .gitignore hides it from every git-based check. That comment gives the anchored form explicitly, `/myapp` rather than `**/myapp`, because the prefixed spelling also matches cmd/myapp/ and deletes the package directory. The header comment is the only part of this guidance a consuming repo actually receives, since it is vendored with the file. .gitignore is untouched. Its semantics are the inverse: an unanchored pattern already matches at any depth, so `**/`-prefixing it produces a file that is wrong in a way that looks careful. That asymmetry is why "derive one from the other" was the wrong instruction, and it is now written down in REPO_POLICIES.md in both directions, together with the case-sensitivity rule, the negation remedy, and the requirement to verify by enumerating the image rather than by reading the patterns. Every consuming repo inherits .dockerignore by copy, so the trap has to live where the next person looks, not only be fixed once here. Both repo checklists gain the same requirements. Verified by planting 130 secret files -- twenty-six name shapes, including every capitalisation of .env, .env.*, prod.env, .envrc, id_rsa, id_ed25519, ca.pem, server.key, bundle.p12 and bundle.pfx, at five depths from the context root to a/b/c -- alongside nine positive controls, then building a standalone probe image doing `COPY . .` and listing what actually landed inside it. The patterns as first written leaked 35 of the 130: every .ENVRC, .Envrc, ID_RSA, Id_Rsa, ID_ED25519, .ENV.PRODUCTION and .Env.Local, at all five depths. A lowercase-only control leaks 80, so the probe is not vacuous. After this change: zero, with all nine controls still present, including internal/ID_MAP.go and certs/CA.CRT. Transferred-context size is recorded but load-bearing on nothing: an earlier run reported 2.18kB transferred while 43 files, five of them secrets, were in the image. BuildKit transfers only the delta from the previous build, so the number describes the transfer and not the contents. Planted files were removed and their absence confirmed against the filesystem rather than against `git status`, which could not have seen them. `make docker` re-run after the change: the check layer executed rather than being served from cache, so the CHECK_EPOCH verification still holds under the altered build context. --- .dockerignore | 71 +++++++++++++++++++++++++++++- TODO.md | 15 +++++++ prompts/EXISTING_REPO_CHECKLIST.md | 12 +++++ prompts/NEW_REPO_CHECKLIST.md | 8 ++++ prompts/REPO_POLICIES.md | 59 ++++++++++++++++++++++++- 5 files changed, 162 insertions(+), 3 deletions(-) diff --git a/.dockerignore b/.dockerignore index 5414d56..d948f00 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,3 +1,70 @@ +# Docker matches this file with moby/patternmatcher: Go filepath.Match +# semantics plus a `**` extension, compiled to a regexp. Plain +# filepath.Match has no `**` at all. What follows from that: `*` does not +# cross `/`, and a pattern without a leading `**/` is anchored at the +# build-context root. Every depth-independent pattern therefore needs the +# `**/` prefix — without it `config/.env` and `certs/server.key` still +# ship while the file reads as solved. +# +# Root-anchored entries are for paths that occur exactly once, at the +# context root. A host-built binary is the usual case, and it must be +# written anchored: `/myapp`, never `**/myapp`. The prefixed form also +# matches `cmd/myapp/`, which deletes the package directory from the +# context. +# +# Matching is case-sensitive, so `**/*.key` does not match +# `certs/SERVER.KEY`, which is reachable on the case-insensitive +# filesystems most laptops use. Adding an ALL-CAPS twin per pattern is +# not the fix: it still misses `Server.Key` while reading as though case +# were handled. Character ranges cover every spelling in one line, so +# every secret name below is written that way — including the +# extensionless SSH keys and `.envrc`, because on those same +# case-insensitive filesystems direnv reads `.ENVRC` and ssh reads +# `ID_RSA`. +# +# `**/*.[eE][nN][vV]` also excludes a committed env template such as +# `example.env`. If the build genuinely needs one, re-include it with a +# negation after the pattern: `!docs/example.env`. +# +# Extend this file with the repo's own host-built artifacts (compiled +# binaries, test binaries, coverage output); those are per-repo and +# belong here because a host build otherwise drops them into the +# context. + +# Repository metadata: exactly one, at the context root. .git -node_modules -.DS_Store + +# Environment files. `*.env` covers both the bare `.env` name (`*` matches +# the empty string) and the `prod.env` convention. +**/*.[eE][nN][vV] +**/.[eE][nN][vV].* +**/.[eE][nN][vV][rR][cC] + +# Private keys and the bundles that carry them. Public certificates +# (*.crt, *.cer) are deliberately absent: they are not secrets and are +# sometimes a legitimate build input. +**/*.[pP][eE][mM] +**/*.[kK][eE][yY] +**/*.[pP]12 +**/*.[pP][fF][xX] +**/[iI][dD]_[rR][sS][aA] +**/[iI][dD]_[dD][sS][aA] +**/[iI][dD]_[eE][cC][dD][sS][aA] +**/[iI][dD]_[eE][dD]25519 + +# Dependencies: restored inside the image, never copied in. +**/node_modules + +# OS metadata. +**/.DS_Store +**/Thumbs.db + +# Editor state. Never a build input, and it churns under a developer's +# hands, so it invalidates COPY for reasons unrelated to the source. +**/*.swp +**/*.swo +**/*~ +**/*.bak +**/.idea +**/.vscode +**/*.sublime-* diff --git a/TODO.md b/TODO.md index 8d9557d..2826015 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,21 @@ fmt-check, and commit. # Completed Steps +- 2026-08-09: Closed the secret exposure in the canonical `.dockerignore`: a + developer's local `.env`, `*.pem` or `*.key` was reaching the Docker build + context under `COPY . .`, invisible to every git-based check because + `.gitignore` covers it. The patterns are written to `.dockerignore`'s own + `moby/patternmatcher` semantics — `**/`-prefixed so they hold at every depth, + which also fixes nested `node_modules` — rather than transplanted from + `.gitignore`, whose unprefixed form protects only the repository root while + reading as solved. Coverage extends past the `.env`/`.pem`/`.key` trio to the + `prod.env` convention, `.envrc`, PKCS#12 bundles and extensionless SSH keys, + every one of them case-folded with character ranges because matching is + case-sensitive and an ALL-CAPS twin per pattern still misses `Server.Key`. + `REPO_POLICIES.md` and both repo checklists now state that asymmetry and + require verification by enumerating the image rather than by reading the + patterns. Verified with a probe image before, against three naive forms + (unprefixed, lowercase-only, ALL-CAPS-doubled), and after. - 2026-08-09: Made the pinned golangci-lint actually propagate: REPO_POLICIES.md now carries the canonical `script/bootstrap` snippet for Go repos, which installs when the installed version does not match the pin (the old diff --git a/prompts/EXISTING_REPO_CHECKLIST.md b/prompts/EXISTING_REPO_CHECKLIST.md index 69cb02b..c32a497 100644 --- a/prompts/EXISTING_REPO_CHECKLIST.md +++ b/prompts/EXISTING_REPO_CHECKLIST.md @@ -37,6 +37,18 @@ with your task. `CHECK_EPOCH` rule in `REPO_POLICIES.md`. Without them the check layer is served from cache on an unchanged tree and the build reports a green it never ran. +- [ ] `.dockerignore` excludes the repo's own host-built artifacts (compiled + binaries, test binaries, coverage output), written root-anchored — + `/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An + existing repo is where such a binary is likeliest to already be sitting in + the build context, invisible to git. +- [ ] Every depth-independent pattern in `.dockerignore` carries a `**/` prefix; + only genuinely root-anchored entries such as `.git` are unprefixed, and + `.gitignore`'s patterns have not been transplanted unmodified. + `.dockerignore` anchors an unprefixed pattern at the context root, so the + transplanted form leaves `config/.env` and `certs/server.key` in the build + context while reading as solved — see the `.dockerignore` rule in + `REPO_POLICIES.md`. - [ ] Gitea Actions workflow in `.gitea/workflows/` runs `script/cibuild` on push — reference `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml` diff --git a/prompts/NEW_REPO_CHECKLIST.md b/prompts/NEW_REPO_CHECKLIST.md index 5b0bcfe..7965c10 100644 --- a/prompts/NEW_REPO_CHECKLIST.md +++ b/prompts/NEW_REPO_CHECKLIST.md @@ -52,6 +52,14 @@ Template files can be fetched from: `https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/REPO_POLICIES.md` - [ ] `Dockerfile` and `.dockerignore` — fetch `.dockerignore` from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` + - Extend `.dockerignore` with the repo's own host-built artifacts, giving + every depth-independent pattern a `**/` prefix — but write a repo-root + binary anchored, `/myapp` and never `**/myapp`, which would also match + `cmd/myapp/` and delete the package directory. Do not transplant + `.gitignore`'s patterns: `.dockerignore` anchors an unprefixed pattern at + the context root, so the copied form leaves `config/.env` in the build + context while reading as solved. See the `.dockerignore` rule in + `REPO_POLICIES.md`. - All Dockerfiles must run `make check` as a build step, and every stage containing a check-running `RUN` must declare `ARG CHECK_EPOCH` with the `RUN [ -n "$CHECK_EPOCH" ] || exit 1` guard immediately below it — see the diff --git a/prompts/REPO_POLICIES.md b/prompts/REPO_POLICIES.md index a893c8d..edc7826 100644 --- a/prompts/REPO_POLICIES.md +++ b/prompts/REPO_POLICIES.md @@ -331,7 +331,64 @@ style conventions are in separate documents: editor files (`.swp`, `*~`), language build artifacts, and `node_modules/`. Fetch the standard `.gitignore` from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when setting up - a new repo. + a new repo. These patterns are written to `.gitignore`'s own semantics, in + which an unanchored pattern already matches at every depth. They are not a + `.dockerignore` and must not be transplanted into one unmodified — see the + next rule. + +- **`.dockerignore` does not use `.gitignore` semantics, and copying patterns + across unmodified leaves secrets in the build context.** Docker matches with + `moby/patternmatcher`: Go `filepath.Match` semantics plus a `**` extension, + compiled to a regexp — plain `filepath.Match` has no `**` at all. So `*` does + not cross `/`, and a pattern without a leading `**/` is anchored at the + build-context root. A `.dockerignore` listing `.env`, `*.pem` and `*.key` + therefore excludes only the copies at the repository root; `config/.env` and + `certs/server.key` still reach the context and can land in an image layer. + That file is more dangerous than a short one with no secret patterns at all, + because it reads as solved and stops anyone looking. Give every + depth-independent pattern the `**/` prefix — `**/node_modules`, + `**/.DS_Store`, and the secret patterns in the canonical file, which are + additionally case-folded per the rule below — and leave only genuinely + root-anchored entries such as `.git` unprefixed. The inverse move is equally + wrong: never apply `**/` to `.gitignore`, where it is redundant and produces a + file that is wrong in a way that looks careful. Each file is written to its + own semantics; neither is derived from the other. Fetch the standard + `.dockerignore` from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` and extend + it with the repo's own host-built artifacts — a host `make build` that leaves + a compiled binary in the repo root puts that binary in the build context, + where `.gitignore` hides it from every git-based check. Write that binary + anchored, `/myapp` and never `**/myapp`: the prefixed form also matches + `cmd/myapp/` and deletes the package directory from the context. + +- **`.dockerignore` matching is case-sensitive, so cover capitalisation with + character classes rather than by doubling patterns.** `**/*.key` does not + match `certs/SERVER.KEY`, which is reachable on the case-insensitive + filesystems most laptops use. Adding an ALL-CAPS twin for each pattern is not + the fix: it still misses `Server.Key` and `Ca.Pem` while reading as though + case were handled — the same manufactured confidence as the root-anchored + form. The matcher supports character ranges, so one line covers every + spelling: `**/*.[kK][eE][yY]`, `**/*.[pP][eE][mM]`. Apply this to every secret + name, not only to extensions: the extensionless SSH keys and `.envrc` need it + for the same reason, since on the very filesystems that make `SERVER.KEY` + reachable, direnv reads `.ENVRC` and ssh reads `ID_RSA`. Note that `*` matches + the empty string, so `**/*.[eE][nN][vV]` already covers a bare `.ENV` and no + separate literal `.env` entry is needed. + +- **A pattern that also catches something the build needs is re-included with a + negation, not deleted.** The canonical `**/*.[eE][nN][vV]` excludes a + committed env template such as `example.env`; a repo whose build genuinely + reads one adds `!docs/example.env` after the pattern. Deleting the pattern + instead reopens the exposure for every other file it covers. + +- **Verify `.dockerignore` by enumerating the image, not by reading the + patterns.** Plant files at the root _and_ at least two directories deep, build + a probe image that does `COPY . .`, and list what actually landed + (`docker run --rm --entrypoint find IMAGE /app`). Reading the patterns and + agreeing they look right is exactly what lets the root-only form through. The + `transferring context` size is not a substitute: a nested secret is a few + bytes, and BuildKit transfers only the delta from the previous build, so the + reported size describes the transfer and not the contents of the image. - **No build artifacts in version control.** Code-derived data (compiled bundles, minified output, generated assets) must never be committed to the