Skip to content

RV-S4: stop the migration image baking local credentials (#174) - #204

Merged
amar-python merged 1 commit into
mainfrom
fix/174-no-baked-secrets-in-image
Nov 10, 2026
Merged

amar-python merged 1 commit into
mainfrom
fix/174-no-baked-secrets-in-image

Conversation

@amar-python

Copy link
Copy Markdown
Owner

Closes #174.

What this changes

build/config.local.env (written by build/setup.sh and scripts/provision_full_test_env.sh) and the concrete build/environments/env_<env>.sql files hold real credentials. Both are gitignored — but .gitignore does not filter the Docker build context, so docker build . from a configured checkout copied them into an image layer. Layers are immutable and cached indefinitely; once such a layer is pushed to a registry, rotating the leaked password is the only remediation, and only if somebody notices.

Both fixes from the 3 Nov decision, in one PR, defence in depth:

  • (a) .dockerignore — lists **/config.local.env, **/config.local.env.*, **/*.local.env and the four concrete build/environments/env_<env>.sql files, mirroring .gitignore lines 2–8, and re-includes build/environments/*.example.sql which the image needs.
  • (b) infra/Dockerfile — COPY build/ ./build/ replaced with an explicit source list. Nothing under build/ reaches the image unless a COPY names it. This is the layer that still holds when someone adds a new config file under build/ without reading this PR.

infra/README.md is new and documents why (item 3 of the issue), alongside a short map of what is in infra/.

Verification

No Docker daemon was available in the session this ran in, so this uses the fallback the issue explicitly allows: comparing the Dockerfile's build-context file list before and after. tests/test_docker_image_secrets.py does that statically — it synthesises the credential files (as the issue's verification step describes), expands the Dockerfile's COPY sources the way Docker would, and asserts none of them is reachable.

Fails on main's Dockerfile/.dockerignore, passes with the fix — both layers, independently:

$ git stash push .dockerignore infra/Dockerfile   # revert to main's versions
$ python -m pytest tests/test_docker_image_secrets.py
FAILED tests/test_docker_image_secrets.py::TestExplicitCopyList::test_no_copy_source_matches_a_sensitive_file
FAILED tests/test_docker_image_secrets.py::TestDockerignoreExcludesSecrets::test_sensitive_files_are_ignored
2 failed, 4 passed in 0.04s

$ git stash pop
$ python -m pytest tests/test_docker_image_secrets.py
6 passed in 0.03s

Build context under build/, before vs after, with synthetic secrets present:

=== build/ files in image context: BEFORE 49 / AFTER 38 ===
=== removed from the image by the fix ===
build/config.local.env
build/config.local.env.new
build/environments/env_dev.sql
build/environments/env_prod.sql
build/environments/env_staging.sql
build/environments/env_test.sql
build/terraform-github-repos/.gitignore
build/terraform-github-repos/main.tf
build/terraform-github-repos/outputs.tf
build/terraform-github-repos/terraform.tfvars.example
build/terraform-github-repos/variables.tf
=== synthetic secrets still reachable after the fix ===
0

All six credential-bearing files gone. build/terraform-github-repos/ also drops out — it is Terraform for GitHub repository management, which the runner never reads, so this is an image-size win rather than a functional change.

No loss of function. test_templates_are_still_copied asserts env_dev.example.sql, env_prod.example.sql, build/deploy_all.sh and build/te_core_schema.sql are all still in the image, so the deploy, load and evals actions keep every file they read. A docker run smoke test could not be executed without a daemon — flagging that as the one piece of the issue's "Done when" not evidenced here.

CI replica (free-tier job)

The integration job needs a PostgreSQL service that was not available here; the free-tier job was run in full.

Unit / regression / security / snapshot tests
  PASSED  : 150
  FAILED  : 0
  ERROR   : 0
  SKIPPED : 0
  NOT RUN : 221  (deselected by the marker filter)
  RESULT: PASS

make lint     -> flake8 clean, bandit clean
make health   -> 84 passed, 0 warnings, 0 failed
evals tier P  -> total 23, passed 23, failed 0, skipped 0

Note for #172

Per the issue, the RV-S1 fix must not reintroduce this exposure by committing a populated env_prod.sql to the image. The explicit COPY list makes that harder by construction: build/environments/*.example.sql is the only pattern reaching build/environments/ in the image, so a concrete env_prod.sql cannot arrive there without deliberately editing a COPY line. #172 should therefore generate it at runtime.


Generated by Claude Code

build/config.local.env (written by build/setup.sh) and the concrete
build/environments/env_<env>.sql files hold real credentials. Both are
gitignored, but .gitignore does not filter the Docker build context, so
`docker build .` from a configured checkout copied them into an image
layer. Layers are immutable and cached indefinitely: once pushed to a
registry, rotating the leaked password is the only remediation.

Two independent layers now stop that, per the 3 Nov decision:

- .dockerignore lists both shapes, mirroring .gitignore lines 2-8, and
  re-includes build/environments/*.example.sql, which the image needs.
- infra/Dockerfile replaces `COPY build/ ./build/` with an explicit
  source list, so nothing under build/ reaches the image unless a COPY
  names it. This is the layer that still holds when someone adds a new
  config file under build/.

tests/test_docker_image_secrets.py enforces both with no Docker daemon:
it synthesises the credential files, expands the Dockerfile's COPY
sources as Docker would, and asserts none is reachable - while asserting
the templates, deploy_all.sh and te_core_schema.sql still are. Both
layers fail against main's Dockerfile/.dockerignore and pass with this
change.

Side effect: build/terraform-github-repos/ is no longer copied. It is
Terraform for GitHub repository management, not needed by the runner.
Build context under build/ drops from 49 files to 38.

Does not change what the image can do: the deploy, load and evals
actions keep every file they read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6WqjTEDRRQGLEYPPFVXx
@amar-python
amar-python marked this pull request as ready for review November 10, 2026 06:38
@amar-python
amar-python merged commit 018edc2 into main Nov 10, 2026
3 checks passed
amar-python pushed a commit that referenced this pull request Nov 10, 2026
#231 and #232 both added the three M5 log steps, so main lists version,
schema_check and cross_upload_duplicates twice. zod collapses duplicate
enum values, so it is harmless at runtime, but the duplicates were an
accident. Remove them and add a test that reads the source (the parsed
schema can't show them); it fails with a duplicate present.

docs/ISSUE_TESTS.md: #174 is done on main (#204).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY
amar-python pushed a commit that referenced this pull request Nov 10, 2026
main (#224) regenerated docs/TEST_CASES.md after #204 merged, which cleared the
red free-tier job this branch was opened to fix. What remains is the file move.
The catalogue conflict is the only one; it is generated and gated by
`test_catalogue.py --check`, so it is regenerated, not merged by hand.

Free tier on the merged tree: 282 passed, 0 failed, 0 skipped; lint, health and
the catalogue check clean.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6WqjTEDRRQGLEYPPFVXx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RV-S4: Docker image can bake build/config.local.env and build/environments/env_*.sql from configured checkout

2 participants