S246: audit development dependencies too, and state the corpus - #9
Merged
Conversation
This repo's CI had no security audit gate at all, so no published advisory against a locked dependency could ever fail it. On 2026-08-06 CVE-2026-67434 (HIGH, OS command injection, GHSA-hmqg-cxww-wqhq) landed against squizlabs/php_codesniffer and only phlix-server went red — the one repo that audited its development dependencies. Sibling repos that did have a gate ran `composer audit --no-dev`, which drops every require-dev package and so could not have caught it either. New `scripts/security-audit-check.php` audits the WHOLE lock via `composer audit --locked --format=json`, labels each finding [require] or [require-dev], reports abandoned packages and config-ignored advisories as loud but non-blocking, and prints the corpus it examined. No baseline file and no ignore list in the script: the only escape hatch is `config.audit.ignore` in composer.json, which the gate reports back as a loud IGNORED notice. Corpus measured on the committed lock: 33 locked packages (5 require, 28 require-dev). Floors MIN_AUDITED_PACKAGES=28 and MIN_AUDITED_DEV_PACKAGES=24; the dev floor is the direct anti-regression, so a returning --no-dev reads as a failure rather than a clean audit of a fraction of the lock. The gate is blocking: a new composer-audit job on push and pull_request, last in .github/workflows/test.yml, with no continue-on-error and no if: condition. 26 guard tests in tests/SecurityAuditCheckTest.php pin all of that shut by execution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 1 medium 1 minor |
| CodeStyle | 6 minor |
| Complexity | 3 medium |
| Performance | 2 medium |
| Comprehensibility | 1 minor |
🟢 Metrics 134 complexity · 0 duplication
Metric Results Complexity 134 Duplication 0
🟢 Coverage ∅ diff coverage · +0.00% coverage variation
Metric Results Coverage variation ✅ +0.00% coverage variation (-1.00%) Diff coverage ✅ ∅ diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (56243f0) 9 9 100.00% Head commit (802c529) 9 (+0) 9 (+0) 100.00% (+0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#9) 0 0 ∅ (not applicable) Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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.
The blind spot
composer audit --no-devcannot fail on a require-dev advisory, and the nine plugin repos had no audit gate at all. On 2026-08-06 the hub's audit job reported SUCCESS against a lock pinning asquizlabs/php_codesnifferversion that CVE-2026-67434 (HIGH, OS command injection) had just made vulnerable.This ports the gate merged into phlix-hub as
3e6e65c/72a7e4b.Policy: audit everything, block on everything
The concern
--no-devwas answering — production, or only the toolchain? — is now answered by labelling each finding[require]/[require-dev], at zero cost in coverage. The escape hatch isconfig.audit.ignoreincomposer.json, reported back as a loud IGNORED notice: in-repo, diffable, reviewable. No baseline file, no ignore list inside the script. The advisory that exposed the hole is command injection in a linter CI runs over pull-request content, on machines holding the deploy credentials — "it does not ship" is not "it cannot hurt you".Not a gate that proves nothing
=== 1each time.composer audit --locked --no-dev) returns success. For the plugin repos there is a second control —auditappears zero times in the workflow onorigin/master, so there was no gate to be blind.cp+md5summatch;vendor/file count unchanged;composer install/updatenever ran.yaml.safe_load, not grep: the job triggers onpush+pull_request, carries nocontinue-on-errorand noif:.continue-on-error,if:on the job,--no-devin the workflow,--no-devinAUDIT_ARGUMENTS, dev floor → 0.Orchestrator re-ran the plant, the control and the restore independently on
phlix-plugin-anidb(deliberately the repo using a different advisory —phpunit/phpunit10.5.64 → 10.5.61, CVE-2026-24765): new gate EXIT=1 naming the advisory, old form EXIT=0 on the identical lock, restoremd5sum 8d1acf1c…identical, tree clean.Adaptations that did not transfer from the hub
assertGreaterThan(2000, strlen($yaml))does not transfer — plugin workflows are 1.3–2.2 KB, so that constant would be a false red or vacuous depending on the repo. Replaced withstrlen(stripped) < strlen(raw)plus two positive substrings.continue-on-error: trueandif:on their Codacy steps, which the hub's does not — so the hub'ssubstrfromcomposer-audit:is only safe because the audit job is last. Pinned last, and the test asserts its own slice still contains the gate invocation.Part of S246.