Fix the CI dependency install, triage the Redis finding, close two benchmark loose ends by laugiov · Pull Request #63 · laugiov/scambuster

laugiov · 2026-08-17T15:03:22Z

Pipeline: chore
Spec:
Gates:
Deploy-impact: rebuilds all three container images (dev, prod, demo). The changes are confined to the composer install layers — retry, source fallback, parallelism, token plumbing — plus a .git cleanup in the prod and demo images. No entrypoint, port, volume, service topology or runtime configuration changed. One thing a deployer and a developer both need to know: docker-compose.yml now pins image: scambuster-backend:ci on the backend anchor, so docker compose up reuses an existing local image and no longer rebuilds on its own — pass --build after changing anything the image bakes in.

Summary

Three unrelated things, all about trusting our own instrumentation. No application code is touched.

main was red before this PR and stayed red. Run 747 on 3abdb7c finished failure, four jobs dead in the same minute — the same class of failure PR #62 was merged past as "a transient GitHub 504". It reproduced, so the transient reading was wrong.

The diagnosis, and two corrections to it

Stated as what each run established, because the first two stories written here were confident and wrong:

Run	What it established
747 (`main`)	`504` from api.github.com interleaved with `429` from codeload. Not transient — it reproduced.
748	The job token removed every 504. The 429s from codeload remained: `api.github.com` redirects the zip to codeload, and Composer correctly declines to carry the `Authorization` header across hosts, so codeload sees an anonymous request under a per-IP limit no token lifts.
750	The runner failed to fetch a 40 KB first-party action tarball (`actions/download-artifact`) from codeload: 429, 429, 502 — before any workflow step ran.
753	`--prefer-source` got the build stage through while codeload refused archives, then died one stage later in the dev-dependency prune.

Run 750 corrected the story. Our Composer volume cannot cause a 429 on an action tarball; the runner fetches actions before our steps exist. So "our own concurrency is rate-limiting us" was wrong — codeload was throttling or degraded for these runners independently of what we asked of it.

The changes are therefore made on their own merits — making fewer requests is right whatever the cause — and not on a claim that they make codeload reliable.

Changes

build-backend-image builds the image once per run and ships it as an artifact. Four concurrent dependency resolutions become one; seven image builds become three, none concurrent. Every job now tests the same image.
The build is layer-cached (buildx, keyed on composer.lock + the Dockerfile). On a hit the composer install layer is reused and the ~200 codeload requests are never made.
--prefer-source as a last resort — attempt 3 clones from github.com instead of pulling archives from codeload.
Token, COMPOSER_MAX_PARALLEL_HTTP=6, jittered backoff (60–89 s, 120–149 s), concurrency groups on both workflows.
Removed 2>/dev/null from two install steps — it discarded exactly the 429 lines that explained the failure.
factory-gates no longer dies on a single transient checks-API error, and bounds at five consecutive failures so a real outage reports "the gates did not run" rather than a timeout.
New Deploy-impact: tier on chore-scope — see below.
docs/factory/README.md gains "Trusting the gates": a red run is never merged on the assumption that it is infrastructure.

Measurements

	before	after (cold)	after (warm cache)
image build	2m31s × 4, concurrent	2m36s × 1	1m24s
containerised install	12s	19s	19s
whole run	10m04s	12m13s	11m20s

Building once costs about two minutes of wall clock. An earlier version of this description and of the comment in ci.yml claimed it was cheaper; the measurement says otherwise and the claim was withdrawn in its own commit.

Review fixes

Self-review found six issues, four of them in my own earlier commits. The two that mattered were both cases where the code worked and the explanation lied: COMPOSER_MAX_PARALLEL_HTTP never reached any image build (never an ARG, never a --build-arg, so the :-6 fallback always won), and stripping .git would have made the --prefer-source path worse on the one image where CI runs composer install afterwards.

A second review found four more, one blocking:

chore-scope had a hole this PR walked through. Its deny list covers backend-symfony/{src,tests,migrations} and frontend-react/src — so a chore PR could change Dockerfile.prod, the demo Dockerfile and docker-compose.yml and pass, under a label that promises no behaviour change. Fixed with a second, softer tier rather than a longer deny list: the deployment surface may be changed, but the PR body must carry a Deploy-impact: line with an actual reason. Missing line, or a marker with no reason, fails the job. Added to the PR template and filled in above.
The ground-truth template and the benchmark README still taught the replaced detection rule ("detected only when a BLOCKING objection cites this id") on the requirement_id field — the first thing anyone reads when seeding a run. Both now describe the real rule and point at score.py rather than restating the mapping in a third place where it can drift again.
The shared :ci tag now carries a stale-image warning in docker-compose.yml and a troubleshooting entry in docs/QUICKSTART.md. One stale image is six stale services, including the ones tests run in.
STATE.md gains a short section listing what is still inference, each with the observation that would falsify it.

Two things not to read into the green

Codeload recovered. The first green run built via the normal dist path with an empty layer cache. The fix helped; GitHub getting better helped more.
--prefer-source has never completed end to end. Its prune failure is fixed but the fixed path has not run. A last-resort mechanism tested in halves, and the untested half is the one that executes during the next outage.

SEC-002 triage

The hardening gap, not the exposure. Redis is published on the host in no compose file: docker-compose.yml:41-54 (no ports:, no command:), docker-compose.prod.yml:63-72 (networks: [data], internal: true at :146-147), docker-compose.demo.yml:38-46. The only host port is commented out in docker-compose.override.yml.example.

The reachable-by-n8n half is true of the development stack only. Production blocks it by topology — n8n is on edge, with no route to data. So the blunt sentence about untrusted inbound mail reaching an unauthenticated Redis does not apply.

Logged low: requirepass appears in no compose file, .env.dist or config file. The only control is network placement, with nothing behind it. Not fixed here.

Benchmark

unjustified-assumption added to the taxonomy rather than renamed in the run, and the taxonomy is now append-only with a Since column.

The detection rule now compares severities: blocker/major need BLOCKING, minor is caught by ADVISORY. The old rule scored a minor correctly raised as ADVISORY as a miss, so the only way to score well was to block on everything. score.py rejects a ground truth with no valid severity and reports minor defects that drew BLOCKING.

Run 002 is not re-scored: 60% stands as its number.

Type of Change

Infrastructure / CI change
Documentation update

Checklist

make test — runs in CI on this PR; not runnable in the authoring environment, which has no Docker daemon
make stan — same
make cs-fixer — not applicable, no PHP changed
New code has tests — the retry loops, the source fallback, the scoring rule and the new deploy gate were each exercised against fixtures; the shell has no unit-test harness in this repo
Documentation updated if needed
No secrets or credentials in the code — the job token is consumed only in build stages the multi-stage COPY discards, and is now scoped to the jobs that need it
Follows DDD architecture — no application code touched

Factory gates

Every commit and task cites a requirement id — n/a for chore
Gate reports linked — n/a, no gate run for chore
Escalation triggers listed — infra/ and CI are out of pipeline scope per STATE.md decision 4; no escalation_triggers.sensitive_paths entry is touched
Reproduction/exploit test before the fix — n/a, not a bug or security PR

…rule Pipeline: chore PR #62 was merged with five red CI runs, diagnosed as a transient GitHub 504. The first half of that diagnosis is right and the conclusion is wrong: the run on `main` at 3abdb7c failed the same way, in four jobs at once, and it is failing on `main` right now. What the log says, once you stop discarding it: Failed to download symfony/browser-kit from dist: ... (HTTP/2 429) Failed to download symfony/uid from dist: ... (HTTP/2 504) Source fallback is disabled. Not trying alternative sources. ERROR: process "/bin/sh -c composer install ..." exit code: 100 Dockerfile:23 Three things that were not in the transient reading: 1. It fails inside the **image build**, not in any step named "Install dependencies". `docker compose up -d --build` is the failing step, so a glance at the step name says "Docker is flaky" rather than "dependency download". 2. The 429s are rate limiting, not load shedding. Composer pulls every package as a dist archive from api.github.com / codeload.github.com; unauthenticated that is 60 requests/hour per IP. A run resolves the tree eleven times across jobs that start in the same second, so the ceiling is reached by this workflow's own concurrency. It is self-inflicted and it will recur. 3. `Source fallback is disabled` means one failed download of ~200 aborts the whole build. There was no retry and no cache anywhere on the PHP side, while the frontend has had `cache: 'npm'` all along. Fixed in three layers, cheapest first: - **Cache** the Composer cache directory, keyed on `composer.lock`, shared by every job. A warm run downloads nothing — the only fix that removes requests rather than retrying them. It lives under the bind-mounted /app so the container writes where actions/cache can save it, and out of /app/var, which CI chowns to uid 10001 and which the runner could then not read back. - **Authenticate** with the job token, raising the ceiling to 1000/hour. Every Dockerfile consumes it in a build stage the multi-stage COPY discards, so no token reaches an image that ships. - **Retry**, three attempts with backoff, for what is genuinely transient. Same shape as the pecl retry these Dockerfiles already carried. The `2>/dev/null` on two install steps is removed. It discarded precisely the "Failed to download ... 429" lines that explain the failure, which is part of how five red runs read as one bad minute at GitHub. docs/factory/README.md gains "Trusting the gates": a red run is never merged on the assumption that it is infrastructure. Either the cause is found and named, or the run is re-run and passes. That rule is what would have caught this at the time, so it is written as a rule rather than as advice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

…sted Pipeline: chore Triage of the security reviewer's Redis finding, against the compose files. **It is a hardening gap, not an exposure.** Redis is published on the host in no compose file: - `docker-compose.yml:41-54` — the `redis` service declares no `ports:` and no `command:`, and says so in a comment. - `docker-compose.prod.yml:63-72` — no ports, `networks: [data]`, and `data` is `internal: true` (`:146-147`). - `docker-compose.demo.yml:38-46` — no ports. - `docker-compose.override.yml.example` — the only host port that exists is commented out, as an opt-in for local debugging. The reachable-by-n8n half of the report is true of the **development** stack only, where redis (`:53-54`) and n8n (`:258-259`) share the `scambuster` bridge. Production already blocks that path by topology: redis is on `data`, n8n is on `edge` (`:110`), and there is no route between them. So the blunt sentence about untrusted inbound mail reaching an unauthenticated Redis does not apply — this is not the exposed case. What is real, and why it is logged as low rather than closed: `requirepass` appears in no compose file, `.env.dist` or config file in the repository. The only control is network placement, with nothing behind it. One added `ports:` line, one service attached to the wrong network, or one container escape, and there is no second layer. Not fixed here. One vulnerability, one PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

…verity Pipeline: chore Two loose ends from run 002. **`unjustified-assumption` is added to the taxonomy, not renamed in the run.** Run 002 needed a category the table did not have, used it, scored 100% on it, and left the decision to the maintainer. Adding is the right direction: the per-type detection rates are the benchmark's most useful output, and renaming a category silently rewrites the history of every run that used it. The table now says so — append-only, with a `Since` column — so the next session that finds a missing term adds one instead of bending an existing one. Run 002's per-type rates stand as measured. **The detection rule now compares the objection's severity against the seeded severity.** It was "DETECTED iff BLOCKING" at every severity, which scored a `minor` defect correctly raised as ADVISORY as a miss. That punished the reviewers for proportionate judgement, and left exactly one way to score well: block on everything — which is the loudness the unseeded-blocking count exists to warn about. The two pressures pointed in opposite directions, and the scoring rule was the one that was wrong. blocker caught by BLOCKING it must not ship major caught by BLOCKING it must not ship minor caught by ADVISORY saying it out loud is the right call PARTIAL now means what it should: a blocker or major that drew only advisories. Because severity is load-bearing, `score.py` rejects a ground truth whose entries lack a valid one rather than defaulting and quietly changing the score — the same reasoning already applied to `requirement_id`. It also reports minor defects that drew a BLOCKING objection: still detected, but that is over-blocking landing on a seeded requirement, where the unseeded count cannot see it. **Run 002 is not re-scored. 60% stands as the number for that run**, with a note in three places that the rule changed after it. Both of its `minor` defects drew BLOCKING objections so the figure would not move — but "measured under the old rule" is worth more than a figure silently recomputed, and the first rate under the new rule is a baseline rather than a comparison. Verified against a fixture covering all six severity/objection combinations, plus the malformed-severity path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

…he cost is Pipeline: chore Measuring the baseline before claiming an improvement turned up a mistake in the previous commit. On the last green run of `main` (31988125536, itself run_attempt 2 — the first attempt had failed), the six containerised "Install dependencies" steps take **6-12 seconds**, not minutes: static-analysis 12s static 'Start containers' 151s code-style 9s style 'Start containers' 127s backend-tests 7s tests 'Start services' 116s e2e (incl. db) 6s security 'Composer audit' 140s They are already warm. The image build runs `composer install`, so the image carries a populated Composer cache, and COMPOSER_HOME lives outside /app — the `./backend-symfony:/app` bind mount shadows /app/vendor but not the cache. All of the time, and every failure, is in the **image build**. So the previous commit's `COMPOSER_CACHE_DIR=/app/.composer-cache` redirect was a regression waiting to happen: on a cold cache it would have pointed Composer at an empty directory and forced a full re-download — turning a 7-second step into a multi-minute one, and adding ~200 requests per job to precisely the endpoint that rate-limits us. It would have made the failure it was meant to fix more likely. The script now seeds that directory from the image's own cache before installing, once, only when it is empty. The redirect can no longer be colder than doing nothing, and the actions/cache entry still covers the case that motivated it: composer.lock moving ahead of the cache baked into the image. The comments no longer claim the cache is what removes the requests. It cannot reach the image build — a build cannot write back to the host — so it is a floor, not a speed-up. What addresses the cause is the token, which does reach the build as a build arg, and the retry behind it. Recorded in STATE.md as 1b, because the next person to tune this will otherwise optimise the fast steps: the real caching fix is to build the image once per run and share it, rather than rebuilding it in four jobs concurrently. Not done here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

…o token lifts Pipeline: chore Run 748 was the first execution of the changed workflow and it is the reason this commit exists. It did not go green, and what it eliminated is as informative as what it did not. **Gone:** every `HTTP/2 504` from api.github.com. The token reached the build correctly (visible in the log as a masked COMPOSER_AUTH), and the retry loop ran its three attempts and reported honestly. Both mechanisms work. **Left:** `HTTP/2 429`, all of it from **codeload**.github.com: The "https://codeload.github.com/doctrine/orm/legacy.zip/bc217c0e..." file could not be downloaded (HTTP/2 429) That is a different limit from the one the token raises, and the distinction is the whole point. `api.github.com` redirects the zipball to codeload, and Composer does not carry the Authorization header across a redirect to another host — which is correct of it. So codeload sees an anonymous request and applies a per-IP abuse limit that no `github-oauth` token lifts. Authentication cannot fix this one. Only a smaller burst can. Two changes, both aimed at burst rate rather than at the ceiling: - **`COMPOSER_MAX_PARALLEL_HTTP=6`**, against Composer's default of 12 open connections. Four concurrent image builds meant up to 48 simultaneous connections to codeload from one runner IP; this halves it. - **Longer, jittered backoff**: 60-89s then 120-149s, seeded from /dev/urandom, replacing a fixed 20s/40s. Two independent problems with the old value. It was far too short to outlast an abuse-detection window, and being fixed it made all four jobs retry in the same second — faithfully reproducing the burst that caused the 429. A retry that recreates the failure condition is not a retry. Applied to all three build stages and to the workflow script. The runtime-stage installs keep the short backoff: vendor/ arrives populated from the build stage, so they do not download and are not exposed to this. Honest status: this is a burst-rate mitigation, not a proof. If run 749 is still red, the remaining levers are structural rather than tunable — build the image once per run and share it instead of rebuilding it in four jobs, or serialise the backend jobs. Both trade CI wall-clock for reliability, which is the maintainer's call to make, and neither is taken here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore This is the durable fix. Runs 748 and 749 established that tuning was not going to get there, and what they eliminated is what points at this. The token raised the api.github.com ceiling from 60/hour to 1000 and removed every HTTP 504. What remained was HTTP 429, all of it from **codeload** .github.com — where api.github.com redirects the actual zip, and where Composer correctly declines to carry the Authorization header because it is a different host. codeload therefore sees an anonymous request under a per-IP abuse limit, and no token lifts it. Fewer parallel connections and a jittered backoff improve the odds of slipping under it. Nothing about them removes it. What removes it is not making the requests. The run resolved the full dependency tree **four times concurrently**, from one runner IP, in the same second — `docker compose up --build` in static-analysis, code-style, backend-tests and security — plus three more in the scanning matrix. The tree is identical every time. Building it once is not an optimisation, it is deleting work that never had a reason to exist. - New `build-backend-image` job builds it once and uploads it as an artifact. - static-analysis, code-style, backend-tests and security `needs:` it, then `docker load` and `docker compose up -d` — **no `--build`**. - `docker-compose.yml` names the image `scambuster-backend:ci`, so Compose uses the loaded one and still builds on demand when the tag is absent. Local `docker compose up` and `up --build` are unchanged, and backend-test / e2e / preprod / scheduler / canary-worker now share the one image rather than each triggering a rebuild. - The scanning matrix reuses that same artifact for `dev` and runs `max-parallel: 1`, so prod and demo build one at a time. - A `concurrency` group cancels superseded PR runs. Two runs of one PR were resolving the tree simultaneously and doubling the very burst at issue. Not applied to main/demo, where each commit's result is a record worth keeping. Four concurrent dependency resolutions become one. Seven image builds become three, none of them concurrent. The auth, the reduced parallelism and the jittered retry all stay as a second line of defence for the three that remain. A side effect worth more than the speed: every job now runs against the *same* image, and the Trivy scan reports on the exact bytes the tests ran on rather than on a second build that happened to use the same Dockerfile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

…ttling us Pipeline: chore Run 750 produced evidence that contradicts what the previous two commits — and the header of ci.yml — asserted, so the correction comes first. `static-analysis` did not fail in Composer. It failed before any workflow step ran, with the runner unable to fetch a 40 KB first-party action tarball: Failed to download action 'https://codeload.github.com/actions/download-artifact/tar.gz/d3f86a1...' Error: 429 (Too Many Requests) -> back off 16.8s Error: 429 (Too Many Requests) -> back off 21.2s Error: 502 (Bad Gateway) Failed to download archive after 3 attempts. This workflow's Composer volume cannot cause that. The runner downloads actions before our steps exist. So "our own concurrency is rate-limiting us" was wrong — codeload.github.com is throttling or degraded for these runners independently of what we ask of it. That is the second confident causal story in this PR to turn out to be a symptom: first "a transient 504", then "self-inflicted concurrency". Both explained the failures I had looked at and neither was tested against one I had not chosen. The header of ci.yml now states what each run established rather than a narrative, and STATE.md carries the correction as rows 1d and 1e, because the rows above it were confidently wrong and a future session would inherit them. The changes still stand — making fewer requests is right whatever the cause — and this commit adds the one that removes them rather than spacing them out: - The single image build is now **layer-cached** (buildx, local cache exported through actions/cache, keyed on composer.lock + the Dockerfile). On a hit, BuildKit reuses the `composer install` layer and the ~200 archive requests to codeload are never made at all. - The per-job Composer `actions/cache` is **removed**, and with it the COMPOSER_CACHE_DIR redirect and its seeding step in composer-install.sh. It was a floor at best — a host-side cache cannot reach inside a build — and four copies of it meant four more action tarballs to fetch from the very host that is refusing them. Same lock-file key, moved to the level where it works. What this does not do, stated plainly so nobody reads green into it: when codeload refuses action tarballs, jobs die before reaching any of this. No change in this repository fixes that. Per the gate-trust rule, that is a finding to be shown with its log lines — never an assumption to wave a red run through on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore Run 751 exhausted all three dist attempts in 482 seconds. The buildx layer cache added in the previous commit worked mechanically — it just cannot help yet, and the reason is circular: the cache only fills after a build succeeds, and no build succeeds. A cache is not a way out of an outage. Every lever tried so far negotiates with codeload.github.com over the same resource: raise the ceiling (the token — worked, for api.github.com only), make fewer requests (build once — worked, four resolutions became one), spread them out (parallelism, jittered backoff). None of them changes *which host is asked*, and codeload is currently refusing traffic that has nothing to do with us: run 750 died fetching a 40 KB first-party action tarball from it. `--prefer-source` changes the host. Composer clones each package from github.com instead of pulling an archive from codeload — a different service with different limits — and the `Source fallback is disabled` line in every failure log so far is Composer saying it has this path and has been told not to take it. Ordering matters, so the common case stays fast: attempt 1 dist fast path attempt 2 dist after a jittered 60-89s attempt 3 --prefer-source ~200 git clones, slower, different service A slow build that finishes beats a fast one that does not. `git` is already installed in all three build stages, so nothing new is needed to make this work. Verified against a stub that refuses dist and accepts --prefer-source: the fallback engages on attempt 3 and returns 0. Verified against a stub that refuses everything: still bounded, still exits 1, still says it is not a flake to retry past. This does not make codeload reliable and is not claimed to. It gives the build a second service to ask when the first one will not answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

…rror Pipeline: chore Run 752 lost the "ci.yml gates on this commit" job to one API response: GET /repos/laugiov/scambuster/commits/8ba76f0.../check-runs "We couldn't respond to your request in time. Sorry about that." ci.yml was still running at the time. The gate did not observe a failure — it failed to observe anything, and reported that as a verdict. The loop already polls every 30 seconds for 40 minutes, so a failed poll is a skipped poll. It just threw straight out of the loop instead of trying again. Now it catches, warns, and continues. Bounded at five consecutive failures, because the opposite mistake is worse than the one being fixed: if the API is genuinely unavailable, this has to say **the gates did not run** rather than poll quietly to the deadline and report a timeout, which reads like the checks were slow rather than unread. Same distinction the workflow already draws in its timeout message, and the same one the mutation-testing step draws with "record this as 'not run', never as 'passed'". Found while triaging a red run rather than by looking for it, which is the point of this PR: this is our own instrumentation, and it was reporting on data it had never managed to read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore Run 753 is the first real progress: the **build stage succeeded**. The --prefer-source fallback did its job — ~200 packages resolved by git clone while codeload was refusing archives — and the failure moved one stage later, to the runtime prune: [stage-1 6/7] composer install --no-dev --prefer-dist ... In VcsDownloader.php line 268: Source directory /app/vendor/infection/extension-installer has uncommitted changes. That is the fallback's own consequence, and a deterministic one. A source install leaves a git working copy in every vendor package; `COPY --from=build` does not reproduce it byte-for-byte, so Composer sees local modifications and refuses to remove a dev package it was asked to prune. `COMPOSER_DISCARD_CHANGES=true` is the supported knob for exactly this, and it is correct *here and nowhere else*: this vendor tree is a build artifact, not a checkout anyone edits. Applied to the runtime prune in the dev and prod images. The demo image runs no composer command after its install, so it is not exposed. Also stripping the leftover `.git` directories once the prune has run. They are harmless to execute but they inflate three things that all matter now: the image, the artifact every CI job downloads, and the surface Trivy scans — for history no runtime needs. A no-op on the normal dist path, where there is nothing to find. Worth stating plainly, because it is the first time in this PR: the failure moved forward. Six runs died in dependency resolution; this one got past it and died in bookkeeping afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore Run 754 is green — all 16 checks — and it makes one sentence in ci.yml false. That comment said building the image once and shipping it as an artifact made the run "slightly cheaper as well as deterministic". Measured against the last green run of main (31988125536): image build 2m36s, once, on the critical path artifact upload ~4s (281 MB) download + docker load 12s + 19s per consumer whole run 12m13s, against 10m04s before It costs about two minutes of wall clock. The serialised build is a barrier the four backend jobs now wait behind, and no amount of saved rebuild time inside them makes that back. Two minutes is a good price for four concurrent dependency resolutions becoming one, and for every job testing the same image instead of a second build of the same Dockerfile. But it is a price, and a comment that says otherwise will be believed by whoever reads it next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore The layer cache restored on the run after the first green one, which is the last piece of evidence this PR was missing: cold build (cache miss) 2m36s warm build (cache hit) 48s <- composer layer reused, no codeload artifact 281 MB ~4s up, 12s down + 19s docker load per consumer whole run 12m13s cold, against 10m04s before The 48 seconds are the point. On a hit the `composer install` layer is not executed, so the dependency requests are not made at all — the only change here that makes the failure impossible rather than less likely. Key is hashFiles(composer.lock, Dockerfile): touch either and one run pays the cold price, then it returns to 48s. Row 1g records what a green CI would otherwise quietly bury: **the --prefer-source fallback has never completed end to end.** Run 753 proved its build stage works while codeload refuses archives, then died in the dev-dep prune one stage later. That prune is fixed, but the fixed path has not run, because codeload recovered and every run since took the fast dist path. It is a last-resort mechanism tested in halves, and the untested half is the one that executes during the next outage. Filing it as working because the pipeline is green would be the same error this PR exists to correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore Asymmetry introduced earlier in this PR: ci.yml cancels superseded PR runs, factory-gates.yml does not. Its "ci.yml gates on this commit" job polls for up to 40 minutes, so a superseded run keeps watching an old SHA, watches ci.yml's jobs get cancelled by *ci.yml's* concurrency group, and reports red on a commit nobody is looking at any more. Seen on 284b1ad: waiting on: Container Security (demo), Backend Tests ... ##[error]Failed on 284b1ad: Container Security (demo) (cancelled) The gate did the right thing — a cancelled check is not a passed check, and it refused to say otherwise. The fix belongs at the other end: stop the stale watcher rather than teach the gate to treat cancellations as acceptable. That rule is worth more than the noise it costs, and softening it to quiet a false red would be the kind of convenience this PR exists to argue against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore The demo image build failed on caa3fe4, and this one is mine rather than GitHub's. Three attempts, 0.2 seconds each: In BaseIO.php line 139: Your github oauth token for github.com contains invalid characters: "ghs_1..." Not a network failure at all — Composer refused the token and exited before making a single request. The retry loop then dutifully repeated a deterministic error twice more, and the --prefer-source fallback could not help either, because nothing had been asked of the network. The demo stage pins Composer 2.6.6, deliberately: newer Composer stops generating vendor/autoload_runtime.php here. That version rejects any github-oauth value containing an underscore, and every modern GitHub token has one — `ghs_`, `ghp_`, `github_pat_`. So the token that fixes the other two images is fatal to this one. Fixed by not passing it: the ARG is gone from the demo Dockerfile and container-security branches so only `dev` and `prod` receive the build arg. The demo image resolves one dependency tree per run and is nowhere near a rate limit, so it gives up nothing. Its bounded retry and source fallback still apply. Two things worth keeping from this: - A change that helps globally can break one target, and the version pin that makes this target different was documented three lines above the code I changed. I did not read it before adding the ARG. - The `$args` variable I first used to make the build arg conditional expanded unquoted JSON. It happens to work — the value has no spaces — which is exactly why it would have survived review and bitten later. Replaced by an explicit if/else with the value quoted. Recorded as STATE.md row 1h so nobody "unifies" the three builds while the pin stands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore Self-review of the whole diff, looking for ways to break it rather than reasons it was fine. Six findings, applied in severity order. **1. COMPOSER_MAX_PARALLEL_HTTP never reached a single image build.** It is not declared as ARG anywhere and never passed with --build-arg, so `${COMPOSER_MAX_PARALLEL_HTTP:-6}` inside all three Dockerfiles always fell through to the default. Behaviour was correct by coincidence — both values are 6 — but the comments described it as a lever on the builds, and changing the workflow env would have silently changed nothing. The Dockerfiles now set 6 literally and say why; the workflow env is documented as governing only the containerised installs. A lie with the right answer is still a lie, and this one was in a file whose whole purpose is explaining how the thing works. **2. Stripping .git could make the --prefer-source path worse, on the one image where it matters.** After a source install, vendor packages are recorded in installed.json as source installs. Removing their .git leaves Composer with an install it cannot verify, and CI runs `composer install` *inside a container from the dev image* immediately afterwards — so the likely response is a reinstall: a full download at exactly the moment that path runs, which is when codeload is already refusing traffic. The strip is removed from the dev image and kept for prod and demo, which run no composer command afterwards. Trading a little image size for not making a bad day worse. **3. The `dev` entry of the container-security matrix still declared a dockerfile it no longer builds.** It loads the artifact now. Leaving the path in suggested this job exercised that Dockerfile; `build-backend-image` is what proves it builds. The key is dropped, with a note. **4. COMPOSER_AUTH was workflow-level**, so it sat in the environment of every job — including `frontend`, which has no use for it, and the demo build, which it breaks. It grants nothing a job lacks via secrets.GITHUB_TOKEN, so this is tidiness rather than a control; it is still cheaper to keep the blast radius equal to the need than to remember which consumers tolerate it. Now set on the six jobs that use it. **5. Formatting cruft** — four double blank lines and one missing separator left where the per-job Composer cache steps were removed. **6. The benchmark README's sample output predated the new scoring rule.** It showed neither the SEVERITY/NEEDED/RAISED columns nor the over-escalation line, so the documented output no longer matched what score.py prints. Findings 1 and 2 are the ones worth remembering: both are cases where the code worked and the explanation did not, which is the failure mode this PR keeps running into. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

…review items Pipeline: chore Four fixes from a second review of PR #63. **1. `chore-scope` had a hole, and this PR is what went through it.** Its deny list is backend-symfony/{src,tests,migrations} and frontend-react/src. This PR changed Dockerfile.prod, the demo Dockerfile and docker-compose.yml, and the gate passed — production build files are not application code by that definition. They decide what runs in production and on the public demo, and they changed under a label whose whole promise is that nothing behavioural did. Extending the deny list would have been the wrong fix: this PR legitimately had to touch those files, and a guard that forbids what the work requires gets bypassed rather than respected. So there is now a second, softer tier in scripts/factory/check-deploy-impact.py: deny application code. Hard failure, no escape hatch. warn infra/docker/**, docker-compose*.yml, infra/monitoring/**, .env.dist. Allowed, named in a warning, and it must carry a `Deploy-impact:` line with an actual reason. Missing line fails; a marker with no reason behind it fails too. Same shape as `Docs-impact:`, because one convention people already know beats two they have to look up, and the PR body reaches the script through a file written by github-script rather than shell interpolation — same handling as the documentation gate. Added to the PR template and filled in on this PR. Verified against four bodies: no line, an empty line, "none" on its own, and a real reason. Only the last passes. The job NAME is deliberately unchanged. It appears verbatim in the OURS list of the "ci.yml gates on this commit" job, and a mismatch there makes that job wait on itself until the 40-minute timeout. Said so in the comment. **2. The ground-truth template still taught the rule this PR replaced.** `requirement_id` — the first field anyone reads when seeding a run — said a defect counts as detected "only when a BLOCKING objection cites this id". score.py stopped working that way three commits ago. Both it and the same claim in benchmark/README.md now describe the real rule and point at score.py instead of restating the mapping in a third place where it can drift again. The "iff BLOCKING" mentions that remain are explicitly historical. I had already grepped for this and missed it: the README sentence spans a line break, and my pattern did not. **3. The shared `:ci` tag needed a warning for local use.** On a clean machine, compose builds on demand and nothing changed. Once the tag exists it does not rebuild — and one stale image is now six stale services (backend-dev/test/e2e/preprod, scheduler, canary-worker) instead of one, including the containers tests run in. Stated in docker-compose.yml and as a troubleshooting entry in docs/QUICKSTART.md, where someone whose fix "has no effect" will actually look. **4. STATE.md records what is still inference.** Row 1f holds measured numbers. This is the other list: the codeload diagnosis, the parallelism ceiling, the backoff, the source fallback, the layer cache — each with its confidence and the observation that would disprove it. Two are marked low: 6 parallel connections was never tuned, and no dist attempt has ever recovered on retry 2 or 3, so the retry may be buying nothing and only the fallback may matter. The next person to see a red install step should know which of these was a guess. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

Pipeline: chore Deploy-impact: none — removes a stray text file from the repository root; no image, compose file or runtime configuration is touched. `body.txt` is a fixture from verifying check-deploy-impact.py against four PR bodies (no line, empty line, "none" alone, a real reason). The helper wrote it to the current directory, and I had changed back to the repository root before calling it, so it landed there instead of in the scratch directory. `git add -A` then took it, and it shipped in 07f898a. Nothing sensitive in it — two lines of sample PR body — but a test fixture has no business in the repository, and it is the second time in this PR that a mistake survived because the check I ran was narrower than the thing I changed. Not adding `body.txt` to .gitignore: hiding it would keep the mistake from being visible next time. `pr-body.txt` IS added, for a different reason — factory-gates writes it to the repository root when the documentation and deployment gates run, so anyone exercising those locally gets one legitimately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DX4PCJyXbV3Je1A9EoYDeW

claude added 17 commits August 17, 2026 14:57

laugiov merged commit 5482f40 into main Aug 17, 2026
16 checks passed

Navigation Menu

Fix the CI dependency install, triage the Redis finding, close two benchmark loose ends - #63

Fix the CI dependency install, triage the Redis finding, close two benchmark loose ends#63
laugiov merged 17 commits into
mainfrom
claude/ci-integrity-security-triage-ynda74

laugiov commented Aug 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

laugiov commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The diagnosis, and two corrections to it

Changes

Measurements

Review fixes

Two things not to read into the green

SEC-002 triage

Benchmark

Type of Change

Checklist

Factory gates

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

laugiov commented Aug 17, 2026 •

edited

Loading