12e8db8b0e633e6b54114f5a2ab8c0af769fb07c
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 3a218497b8 |
Keep in-repo agent scratch out of the build context and out of git (closes #27)
All checks were successful
check / check (push) Successful in 17s
The canonical .dockerignore and .gitignore both omitted the in-repo agent scratch directory. On this fleet that directory holds one worktree per in-flight agent -- an entire additional checkout of the repo each -- so under `COPY . .` all of it reached the build context and the image. Measured on this repo before the change: five planted scratch files, at every depth beneath the directory, all present inside a probe image built from the real context. Three consequences, only the first of which is about size. The context inflates by a multiple of the repo. Another session's unreviewed and sometimes uncommitted work is copied into a build artifact. And the directory is created and destroyed constantly by tooling, so it invalidates `COPY . .` for reasons that have nothing to do with this repo's content -- which is the accidental cache protection described at length in the issue thread, and the reason this change was sequenced behind the CHECK_EPOCH bust rather than landed alongside the rest of the .dockerignore work. The two entries are deliberately different shapes, because the two files have different semantics and neither is derived from the other. In .dockerignore the entry is anchored, `.claude`, with no `**/` prefix: the directory occurs exactly once, at the context root, and the prefixed form additionally matches any nested directory of that name. Measured rather than argued -- the `**/`-prefixed control was built and enumerated too, and it removes prompts/.claude/ from the context as well, which in a repo with a legitimately named nested directory would silently delete it from the build. In .gitignore the entry is unanchored, `.claude/`, because a .gitignore pattern already matches at every depth; `git check-ignore -v` confirms it covering both .claude/ and prompts/.claude/, so a `**/` prefix there would be redundant at best, and on an anchored pattern it would be actively wrong. Anchoring buys that at the cost of a residual exposure, and the vendored files now say so rather than only asserting the reason to anchor. "Occurs exactly once, at the context root" is a property of how agents are run, not of the tooling: the directory is created in the agent's working directory, so a monorepo running a per-service agent in services/api/ still ships services/api/.claude/ into the context and the image -- the exact exposure this change exists to close, left open in the repo shape where it is likeliest. The .dockerignore header block, the REPO_POLICIES.md bullet and both checklists state the gap and the remedy (anchored entries for the subdirectories that have one, or `**/.claude` once no legitimately named nested directory would be caught). Consuming repos receive the files and not the tracker, so a caveat that lives only in a PR body is not a caveat. It is not case-folded the way the neighbouring secret patterns are: tooling creates the directory in exactly one spelling, so a folded pattern would add no coverage. The earlier justification -- that a miss costs bloat rather than exposure -- is gone, because it contradicted this issue's own framing, in which the cost of a miss is unreviewed work in an image layer. The second half of this change is the consequence that ships broken silently. Excluding .git means `git describe` cannot run in any build stage, and it fails quietly there rather than erroring: `-X main.Version=` comes out empty, the binary reports no version, and the build still exits 0. The Go template in REPO_POLICIES.md had `ARG VERSION=dev` and never said where VERSION came from, which is precisely the gap a reader fills in with `git describe` inside the build. It now says: computed on the host, threaded in with `--build-arg VERSION=...`, shown as a complete command rather than as two rules each documenting half of one. script/docker and script/cibuild do it, with the same discipline the epoch already has -- assignment on its own line, because a failing command substitution inside an argument does not trip `set -e`, plus a non-empty fallback so a build from an export with no .git reports `unknown` rather than an empty string that reads as a successful version. That fallback is applied in exactly one place, and that place can actually execute. `git describe ... || true` leaves the value empty when it fails, and the `[ -n "$version" ]` line is what substitutes `unknown`. Folding the fallback into the substitution as `|| echo unknown` would have left the guard unreachable -- harmless in itself, but a guard that cannot fire is indistinguishable from one that works to every repo that copies it, and canonical text should not carry a check that is decorative. Measured firing in both sh and dash: not a git repository -> unknown, repository with no commits yet -> unknown, this repository -> the describe output. The checklist now carries the guard line as well, so the vendored guidance and the vendored script no longer disagree. The scripts pass VERSION unconditionally rather than growing a per-repo variant. This repo's Dockerfile declares no `ARG VERSION`, and BuildKit was measured accepting the unconsumed arg silently -- no warning, no cache effect, confirmed by the paired runs below in which the bootstrap layer still caches. The alternative, leaving it to each repo, reintroduces the trap: a repo that needs a version and finds no VERSION in its scripts writes `git describe` into the Dockerfile, which is the failure being closed. The same correction reaches the two Go documents that carry the GOLDFLAGS pattern, since a `$(shell git describe)` evaluated inside a build stage is exactly this empty version. Both are now `?=`, and their comments say precisely when that matters: where a build stage compiles by invoking make, `ARG VERSION` puts the value in the environment and `?=` defers to it, whereas the canonical Go template compiles with `go build` directly and uses the Makefile on the host only -- still `?=`, so that a repo which later moves its build behind make does not silently start shipping an empty version. Leaving them as `:=` would have left the corpus telling a reader one thing in the policy and the opposite in the styleguide. Both repo checklists gain the entries too. They are what an agent reads while writing these files, so they are where the wrong shape actually gets written: the .gitignore item is the one an existing repo never re-fetches, and it now names `.claude/` explicitly along with the warning not to prefix it. Verification, by enumerating a probe image rather than by reading the patterns. Standalone minimal Dockerfile held outside the context, `--no-cache` scoped to that one image, no prune of any kind. Before: all five planted scratch files in the image, 42 files total. After: zero, 37 files total, with README.md, script/check, prompts/NEW_REPO_CHECKLIST.md and a planted probe_src/app.md all still present as positive controls, so the exclusion is a real exclusion and not a COPY that stopped copying. Transferred context fell from 161.86kB to 68.82kB, recorded as corroboration only: BuildKit reports a delta, not a total, and an earlier run in this repo transferred 2.18kB while shipping 43 files. The CHECK_EPOCH verification was re-run under the changed context, because the context moved underneath the earlier measurement. Two consecutive script/cibuild runs on an unchanged tree: run 1 in 17.07s, run 2 in 5.73s, both executing the check layer with a distinct epoch and real prettier output from both lint and fmt-check. The `RUN script/bootstrap` layer is CACHED in run 2, which is the validity control -- it proves no concurrent prune landed between the runs and that no --no-cache path was taken, so the check layer executing is the bust working rather than a cold cache. Planted files were removed afterwards and their absence confirmed against the filesystem with `find`, not against `git status`, which cannot see them once .gitignore covers the directory -- the same blind spot that made the earlier secret exposure invisible. |
|||
| fd78aeb003 |
Keep secrets out of the Docker build context at every depth (closes #29)
All checks were successful
check / check (push) Successful in 21s
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. |
|||
| efbac580f9 |
Add repo scaffolding to meet repository standards
Add .gitignore, LICENSE (MIT), Makefile, Dockerfile, .dockerignore, and pin prettier via yarn lockfile for integrity-checked markdown formatting. Update REPO_POLICIES.md self-reference to point to this repo. Format markdown files with prettier. |