feat(php-backend-sdlc): security-audit skill — multi-subagent OWASP red-team → fix loop - #7
Merged
Merged
Conversation
…gent
Adds an adversarial, authorized red-team / penetration-testing capability to
the plugin, planned via the BMAD autonomous chain (specs/autonomous/
2026-06-14-security-audit-skill/: research, brief, PRD, architecture, epics,
readiness — READY).
- skills/security-audit/SKILL.md (331 lines): the multi-subagent loop —
triage → fan out one security-auditor per OWASP/vuln family in parallel →
attack the running service (black-box HTTP/GraphQL) + SAST/taint/dep/secret/
config → verify every candidate by reproduction (no false positives) → map to
CWE+OWASP+severity → root-cause, suppression-free fix via php-implementer with
a regression test per fix → re-dispatch only still-open families until a clean
pass. MAX_ITERATIONS=5, degrade paths, defensive/authorized-only boundary.
- skills/security-audit/reference/{owasp-catalog,attack-playbooks,
remediation-patterns}.md: the full OWASP corpus across editions (Top 10
2003–2021, API 2019/2023, LLM, Mobile N/A-for-backend, ASVS 5.0, WSTG 4.2,
CWE Top 25 2024) + per-family probe/reproduce playbooks + secure-by-default,
cheat-sheet-cited remediations.
- agents/security-auditor.md: the per-family red-team subagent (Bash/Read/Glob/
Grep, opus, never edits code — reports verified findings that route to
php-implementer).
- docs/profile-schema.md: new make.security (nullable) + capabilities.
dynamic_security_testing keys.
- Integration: SKILL-DECISION-GUIDE, AI-AGENT-GUIDE, sdlc-review triage (21→22),
component-counts.bats (6→7 agents, 21→22 skills), README + parent architecture.
All plugin CI gates pass locally: 197 bats (0 fail), markdownlint 0 errors,
generalization-audit clean, profile-keys-check clean, frontmatter complete,
SKILL.md ≤500 lines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughAdds a Changessecurity-audit Skill and security-auditor Agent
Sequence Diagram(s)sequenceDiagram
actor Developer
participant SkillOrchestrator as security-audit
participant Auditors as security-auditor ×N (parallel)
participant Implementer as php-implementer
Developer->>SkillOrchestrator: invoke security-audit
SkillOrchestrator->>SkillOrchestrator: triage OWASP families → PROBE / N/A
par per PROBE family
SkillOrchestrator->>Auditors: dispatch(family, playbook, profile, base_url)
Auditors->>Auditors: static lane (SAST / dep / secret / config)
Auditors->>Auditors: dynamic probe against running service
Auditors->>Auditors: reproduce candidate → promote or downgrade
Auditors-->>SkillOrchestrator: verified FINDINGs + family verdict
end
SkillOrchestrator->>SkillOrchestrator: deduplicate + order by severity
loop per verified finding
SkillOrchestrator->>Implementer: route finding (root-cause fix required)
Implementer->>Implementer: write failing regression test → apply fix → test passes
Implementer-->>SkillOrchestrator: fix done
SkillOrchestrator->>Auditors: re-verify still-open families
end
alt zero open findings after make.ci
SkillOrchestrator-->>Developer: run report (all families CLOSED)
else MAX_ITERATIONS=5 exhausted or breaker tripped
SkillOrchestrator-->>Developer: SDLC escalation block
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plugins/php-backend-sdlc/skills/security-audit/reference/attack-playbooks.md">
<violation number="1" location="plugins/php-backend-sdlc/skills/security-audit/reference/attack-playbooks.md:64">
P2: Incorrect WSTG v4.2 test ID: IDOR is WSTG-ATHZ-03, not WSTG-ATHZ-04. WSTG-ATHZ-04 is 'Testing for Weak Password Policy' and has no relation to Insecure Direct Object References.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/php-backend-sdlc/commands/sdlc-review.md`:
- Around line 42-53: The skill count references are inconsistent across
documentation files. Update both affected files to reflect the current count of
22 skills instead of the outdated 21. In
plugins/php-backend-sdlc/commands/sdlc-review.md at lines 42-53, change the
applicability triage description from 21 to 22 skills. In
plugins/php-backend-sdlc/README.md at lines 59-63, update the overview section
to also reference 22 skills. Ensure both locations now consistently state the
same skill count in their respective stage descriptions.
In
`@plugins/php-backend-sdlc/skills/security-audit/reference/attack-playbooks.md`:
- Around line 363-364: Remove the `git log -p` example from the Tools section in
the secret-scan line since the security-auditor tool forbids git commands,
making this approach incompatible with the static analysis lane. Keep only the
non-git alternatives like gitleaks, entropy grep, Semgrep crypto rules, Grep,
and curl that can function within the security-auditor constraints.
In `@plugins/php-backend-sdlc/skills/security-audit/SKILL.md`:
- Line 77: Remove the reference to the non-existent `/sdlc-security` command
from line 77 in the SKILL.md file. The command is not implemented and is causing
prompt-lint validation failures, so delete the text that mentions this command
to keep the command surface accurate at 8 commands as specified in FR-2.
In `@specs/autonomous/2026-06-14-security-audit-skill/architecture.md`:
- Around line 230-244: The current documentation makes claims about CI job scope
that don't match their actual boundaries. At
specs/autonomous/2026-06-14-security-audit-skill/architecture.md lines 230-244:
remove the assertion that profile-keys-check validates agent headers (it only
checks skills/*/SKILL.md) and clarify that generalization-audit scans only the
plugin tree, not docs/profile-schema.md or README.md. At
specs/autonomous/2026-06-14-security-audit-skill/epics.md lines 121-122: split
the acceptance criteria to separate the skill-header checking requirement from a
narrower generalization-audit scope that reflects what the job actually covers.
At specs/autonomous/2026-06-14-security-audit-skill/implementation-readiness.md
lines 75-86: do not mark the CI-scope mismatch as resolved until the
documentation at all three locations is corrected to match the actual job
boundaries.
In `@specs/autonomous/2026-06-14-security-audit-skill/prd.md`:
- Line 31: The documentation claims that a CI workflow enforces a line-count
check (wc -l) on SKILL.md files, but no such automated check actually exists in
the current workflows. Update the acceptance criteria and NFRs across all
affected files to reflect that the 500-line budget is author-enforced and
spot-checked during E6-S1 review, not CI-automated. In prd.md at line 31, reword
the AC to remove the "(CI line-count check)" implication and instead state
"author-enforced ≤ ~500-line budget, spot-checked in E6-S1". Apply the same
spot-check phrasing consistently at architecture.md NFR-9 (line 130), remove the
CI-automation implication at architecture.md line 277, update epics.md
release-gate text (lines 121-128) to match this wording, and keep
implementation-readiness.md (lines 75-86) at READY-WITH-CONDITIONS status until
all source docs are synchronized.
- Around line 23-31: The PRD requirement for the skill file structure currently
states that sections must include `## Profile keys consumed` but does not
explicitly mandate that this section appears as the first H2 heading. Modify the
skill format requirement in the PRD to explicitly state that `## Profile keys
consumed` must be the first H2 section (appearing immediately after the YAML
frontmatter and before
Context/Task/Success-Criteria/Steps/Constraints/Verification sections). This
ensures the load contract is unambiguous and cannot be satisfied by a file that
places the section elsewhere.
- Around line 76-80: The requirements for `make.security: null` contain
contradictory acceptance criteria: the FR-9 section states the plugin should run
a bundled/static fallback or degrade with a note, while a later open question
frames this as two different choices (bundle a script versus degrade entirely).
Choose a single, definitive behavior for `make.security: null` (either
bundled/static fallback or full degradation with a note), then update the prd.md
document to remove all alternate or contradictory wording. Ensure the chosen
behavior is stated consistently in both the FR-9 description and any related
open questions or sections that mention this configuration option.
In `@specs/autonomous/2026-06-14-security-audit-skill/product-brief.md`:
- Line 20: The current documentation treats SANS and CWE Top 25 as equivalent
taxonomies, which creates confusion. At
specs/autonomous/2026-06-14-security-audit-skill/product-brief.md lines 20-20,
reword the corpus list to establish CWE Top 25 as the primary taxonomy and
mention SANS only as historical context or background, removing any language
that positions them as equivalent. At
specs/autonomous/2026-06-14-security-audit-skill/research.md lines 53-57, remove
the statement asserting SANS and CWE Top 25 are the same taxonomy, and
explicitly identify CWE Top 25 as the authoritative source of truth for the
coverage enumeration.
- Line 66: Replace the generic placeholder `capabilities.*` in the product brief
with the explicitly named capability key `capabilities.dynamic_security_testing`
to ensure the schema contract is specific and consistent with the profile-keys
check and downstream documentation. This should appear in the section describing
the two minimal new profile keys, where it currently states "a `capabilities.*`
boolean gating dynamic testing".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59d36d51-c8c8-4058-bdb3-b843c8f8e7f3
📒 Files selected for processing (18)
plugins/php-backend-sdlc/README.mdplugins/php-backend-sdlc/agents/security-auditor.mdplugins/php-backend-sdlc/commands/sdlc-review.mdplugins/php-backend-sdlc/docs/profile-schema.mdplugins/php-backend-sdlc/skills/AI-AGENT-GUIDE.mdplugins/php-backend-sdlc/skills/SKILL-DECISION-GUIDE.mdplugins/php-backend-sdlc/skills/security-audit/SKILL.mdplugins/php-backend-sdlc/skills/security-audit/reference/attack-playbooks.mdplugins/php-backend-sdlc/skills/security-audit/reference/owasp-catalog.mdplugins/php-backend-sdlc/skills/security-audit/reference/remediation-patterns.mdplugins/php-backend-sdlc/tests/component-counts.batsspecs/autonomous/2026-06-09-php-backend-sdlc-plugin/architecture.mdspecs/autonomous/2026-06-14-security-audit-skill/architecture.mdspecs/autonomous/2026-06-14-security-audit-skill/epics.mdspecs/autonomous/2026-06-14-security-audit-skill/implementation-readiness.mdspecs/autonomous/2026-06-14-security-audit-skill/prd.mdspecs/autonomous/2026-06-14-security-audit-skill/product-brief.mdspecs/autonomous/2026-06-14-security-audit-skill/research.md
Dogfooded the new security-audit methodology on its own PR content — fanned out OWASP-family red-team subagents + review lenses, 3-vote-verified. 3 confirmed, all root-cause-fixed: - CWE-918 (medium): attack-playbooks.md SSRF probe named the live cloud instance-metadata endpoint (169.254.169.254 IMDS) as a target, contradicting the no-exfiltration / in-scope-only boundary. Removed it as a target; the endpoint now appears only in an explicit prohibition, and the probe uses a benign in-container listener / local IMDS stub. - security-audit SKILL.md escalation block used a non-canonical `---SECURITY-AUDIT ESCALATION---` marker that the /sdlc loop aggregator (which greps `=== SDLC ESCALATION ===`) could not collate. Replaced with the canonical block + the 7 standard fields (stage/iteration/exit_condition/ status/blocking_finding/iteration_log/recommended_action). - AI-AGENT-GUIDE.md stale "only load-testing ships a reference/ directory" — security-audit/ now ships one too; corrected the inventory line. markdownlint 0 errors, generalization clean, escalation now matches the canonical contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- SKILL.md: reword "no /sdlc-security command" so the literal /sdlc-security token no longer trips the prompt-quality dead-command check (L22) — this was failing prompt-lint AND the toolkit's own real-plugin-is-clean tests (test_lint_all / test_references), which cascaded into the Coverage + lint-selftest CI jobs. One reword clears all of them. - attack-playbooks.md: BOPLA secondary WSTG id WSTG-ATHZ-04 (which is IDOR) -> WSTG-ATHZ-02 (Bypassing Authorization Schema), the correct property-level authz reference. (IDOR's own WSTG-ATHZ-04 is correct and kept.) - attack-playbooks.md: secret-scan lane is now git-free (gitleaks/trufflehog filesystem mode over the working tree) — the security-auditor agent has no git, so `git log -p` was non-executable as written. lint_all 0 findings, toolkit real-plugin-clean tests pass, markdownlint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…accuracy - docs/evidence/security-audit-dogfood-run.md: records the dogfood red-team campaign (multi-subagent OWASP fan-out → 3-vote verify → root-cause fix → re-verify), the 3 confirmed+fixed findings, the clean shell-script lanes, and the documented dynamic-lane degrade (no runnable PHP target in-plugin). - specs planning docs: corrected CI-scope claims (profile-keys-check = skills/*/SKILL.md only; generalization-audit excludes docs/), pinned "## Profile keys consumed" as first H2, reframed the SKILL ≤500-line bound as a review budget (no wc -l CI gate), made make.security:null ⇒ bundled static lane consistent, unified CWE-Top-25/SANS taxonomy wording, and pinned capabilities.dynamic_security_testing explicitly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… count)
Round-2 of the self-red-team (8 candidates → 2 confirmed, 3-vote):
- CWE-20 (generate-profile.sh): the python-fallback composer_req() indexed
require/require-dev without a shape guard, so a crafted composer.json with a
non-object require (e.g. {"require":"x"}) raised an uncaught AttributeError
and aborted profile generation on no-jq machines — contradicting the script's
"a missing capability never fails generation" contract. Now coerces a
non-dict root/section to {} (mirrors composer_name()'s full try/except),
degrading to "absent". Pre-existing gap surfaced by the new skill.
- README.md command table said "21-skill triage" while the plugin ships 22
(README already said 22 elsewhere) — corrected.
generate-profile bats green (23 ok), README markdownlint clean, prompt-lint 0.
(The "21" strings remaining under docs/testing/ + docs/evidence/qa-install-setup
are historical v1-campaign records, accurate as history — left unchanged.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… invariant) Round-3 of the self-red-team (8 candidates → 3 confirmed, 3-vote): - CWE-59 (generate-profile.sh): no reject_irregular guard — a directory/FIFO at the profile path let `mv -f` drop the temp file inside it / clobber the FIFO and exit 0 on a broken state (sibling inject-governance.sh already guards this). Added the guard + a bats regression (dir at path → exit 1, no litter). - CWE-918 (SKILL.md + security-auditor.md): the "in-scope target" boundary was self-referential (any dispatched URL became "in scope"). Made it a verifiable invariant — before any dynamic probe the target host MUST resolve to loopback / RFC1918-private / a container-compose network, else refuse (skip-with-note + boundary-violation), even if a public host is supplied. - SKILL.md fan-out contract: added the producer side the agent requires — boot the service via make.start when capabilities.dynamic_security_testing is true, and pass the resolved in-scope base URL as a dispatch field (degrades to static-only when the capability/make.start is absent). SKILL.md 351 lines, generate-profile bats 24 ok, markdownlint/generalization/ prompt-lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-4 of the self-red-team (9 candidates → 1 confirmed): - CWE-367 (inject-governance.sh): write_managed preserved the overwrite mode via `chmod --reference="$file"`, a second dereference of the attacker-controllable path AFTER render_managed's symlink/irregular guard — a TOCTOU where $file could be swapped for a symlink in the window, letting an attacker choose the new in-repo file's permission mode (mv is rename-based so no out-of-tree write / secret read; impact limited to the mode). Now the mode is captured once under the guard (stat in render_managed) and applied as a numeric value, with no later symlink-following dereference. inject-governance bats 23 ok; mode preservation verified (664 round-trips). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…romotion Round-5 dogfood red-team found the security-auditor no-false-positive contract (and its static-only degrade path) omitted the FR-7 in-tree static-only exception that SKILL.md:164-168 already states. That made the agent require live-service reproduction for EVERY candidate, forcing committed-secret (CWE-798) and vulnerable-pinned-dependency findings into false negatives under static-only, and contradicting the agent's own degrade smoke-test (which expects CLEAN|FINDINGS reachable static-only). Add the static-only promotion path to both the no-false-positive rule and the dynamic-degrade path: a candidate is promoted by live reproduction OR, for the two classes with no running surface (committed secret/CWE-798, vulnerable pinned dependency), by a deterministic in-tree demonstration — mirroring the skill contract verbatim. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…aseline) Builds the detection-fidelity test campaign for the security-audit skill: a deterministic static lane and an LLM-as-judge behavioral lane that prove the skill flags the vulnerabilities it claims to cover and stays silent on secure code. Docs (plugins/php-backend-sdlc/docs/testing/): - security-audit-test-strategy.md: two-lane model, per-family coverage matrix, positive/negative/edge taxonomy, risk order, exit + loop protocol. - security-audit-test-plan.md: test items, pass/fail per lane, CI wiring. - security-audit-test-cases.md: positive/negative/edge cases per OWASP family, each mapped to a fixture and a corpus.py entry. Harness (tools/security-audit-validation/, outside the shipped plugin tree): - corpus/: 58 PHP fixtures + 2 composer.json across 17 families (vulnerable / clean / edge triads), the seeded "vulnerable PRs". - rules/security-audit.yml: local semgrep pack (taint + pattern) modelling the security-auditor SAST lane for 11 statically-detectable families. - detect.py: static + FR-7 dep detection harness — asserts a true positive on every vulnerable fixture and a strict true negative on every clean one (45 static + 2 dep = 47/47). Invokes semgrep with -j 1 for portability. - judge/run_seed_judge.py: claude-sonnet behavioral lane covering the logic families (BOLA/IDOR, BFLA, BOPLA, auth, rate) no static rule can decide; skip-clean without the CLI (never a false green). - tests/: stdlib unittest, 100% line+branch coverage of the harness. CI: new security-audit-validation.yml (ruff, ty, xenon, bandit, 100% coverage, semgrep TP/TN, skip-clean judge). Existing CI and python-quality untouched. pyproject ruff include extended to the new toolkit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 new issue
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
| model: str, | ||
| votes: int, | ||
| caller=_call_claude, | ||
| reader=None, |
…e gaps, document 16 blind spots
Ran the adversarial subagent round: 11 red-team authors generated 44 fresh
fixtures built to evade the detection rules; semgrep arbitrated each
deterministically. 25 candidate gaps surfaced.
Closed 9 soundly-fixable static gaps (each locked in by an SC-*-E* regression
fixture, baseline now 60/60):
- sqli: Doctrine DBAL executeQuery/executeStatement/executeUpdate sinks
- command: backtick-operator sink (regex), $_SERVER taint source, (int)/intval
sanitizers (cleared two false positives)
- deserialization: pattern-mode (flag unserialize/call_user_func of any
non-constant, incl. parameter sources) + call_user_func('unserialize') alias
- path: file()/SplFileObject read sinks
- xss: printf/vprintf sinks
- crypto: hash('md5'|'sha1', $cred) alias forms
- secret: null-coalesce credential default ($k = $v ?? "literal")
- xxe: XMLReader::setParserProperty(SUBST_ENTITIES, true)
- redirect: multi-arg header() sink + (int) sanitizer (cleared a false positive)
- all taint rules: $_SERVER added as a source
The remaining 16 are inherent OSS-static blind spots (interprocedural flow,
dynamic dispatch, output-context sensitivity, value-semantics name heuristics,
non-constant flags). Kept in the corpus as JL-* fixtures (static_expect=None)
so the judge lane — and any future interprocedural engine — must reach the
right verdict; documented in the strategy doc "Static-lane blind spots".
Harness: 60/60 static+dep pass, 66 unittests, 100% coverage, ruff/ty/xenon/
bandit clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…atic lane (73/73) Second adversarial round: authors told what round 1 covers, asked for genuinely new evasions. 44 more fixtures, 28 candidate gaps. Fixed 13 more soundly-fixable static gaps (each with an SC-*-E* regression; static lane 60→73/73): - sqli: prepare()/mysqli_prepare sinks; PDO::quote + *_real_escape_string sanitizers (cleared a false positive) - command: pcntl_exec sink; proc_open([...]) array-form exclusion (cleared a FP) - deserialization: yaml_parse sink - path: highlight_file/show_source sinks; (int)/intval sanitizers - ssrf: curl_setopt_array sink; (int)/intval sanitizers (cleared a FP) - xss: exit/die/php://output sinks - secret: a second rule for credential-named property assignments - crypto: hash_hmac / crypt alias forms - xxe: DOMDocument::$resolveExternals vector - all taint rules: filter_input/filter_input_array/getallheaders/ apache_request_headers sources The remaining 18 fell entirely into the already-documented blind-spot classes (interprocedural flow, dynamic dispatch, array-element/handle flow, dynamic/ OR-folded flags, value-semantics name judgements, custom sanitizers, template config) — no NEW soundly-fixable static gap. Kept as JL-* judge-lane fixtures. That zero-new-sound-gap result is the convergence signal. Harness: 73/73 static+dep, 100% coverage, ruff/ty/xenon/bandit clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 34 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tools/security-audit-validation/corpus/deserialization/edge_yaml_parse.php">
<violation number="1" location="tools/security-audit-validation/corpus/deserialization/edge_yaml_parse.php:5">
P2: Security test fixture incorrectly labels yaml_parse call as 'with object support' — the code does not pass a callbacks array or enable yaml.decode_php, meaning it is not actually vulnerable to PHP object injection in a default environment. This mislabeling would cause false positives in automated security audits.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // SC-DESER-E5: yaml_parse with object support deserializes untrusted input. | ||
| function loadCfg(): mixed | ||
| { | ||
| return yaml_parse($_POST['cfg'] ?? ''); |
There was a problem hiding this comment.
P2: Security test fixture incorrectly labels yaml_parse call as 'with object support' — the code does not pass a callbacks array or enable yaml.decode_php, meaning it is not actually vulnerable to PHP object injection in a default environment. This mislabeling would cause false positives in automated security audits.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/security-audit-validation/corpus/deserialization/edge_yaml_parse.php, line 5:
<comment>Security test fixture incorrectly labels yaml_parse call as 'with object support' — the code does not pass a callbacks array or enable yaml.decode_php, meaning it is not actually vulnerable to PHP object injection in a default environment. This mislabeling would cause false positives in automated security audits.</comment>
<file context>
@@ -0,0 +1,6 @@
+// SC-DESER-E5: yaml_parse with object support deserializes untrusted input.
+function loadCfg(): mixed
+{
+ return yaml_parse($_POST['cfg'] ?? '');
+}
</file context>
…sweep (81/81) Third adversarial round (44 fixtures, 30 candidate gaps). Fixed 8 more soundly-fixable static gaps (static lane 73→81/81): $_FILES upload-filename source; get_headers; print_r/var_dump/var_export output sinks; igbinary_unserialize; mhash; copy; and a define()-constant credential rule. Convergence reached: by round 3 no NEW blind-spot class appeared (the classes are saturated and carried as JL-* judge-lane fixtures), and the common + frequently-seen sinks are all covered. The residual is the asymptotic niche-sink tail (PHP's dangerous-function surface is effectively unbounded) plus the stable blind-spot classes — both the judge lane's and live dynamic probing's territory. Loop is reproducible via tools/security-audit-validation/. Harness: 81/81 static+dep, 100% coverage, ruff/ty/xenon/bandit clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
…gfood gap) Dogfooding the skill on a live user-service PR booted the service under a fuzz/CI env (APP_ENV=schemathesis) that disables the rate limiter and enables debug. That masks the rate/resource-exhaustion family (false CLEAN) and inflates error-disclosure (CWE-209) and GraphQL-introspection findings (false positives). The auditors caveated it correctly by hand, but the skill did not require it. §5.2 now mandates capturing and reporting the booted runtime env, passing it in every dispatch, and cross-checking the prod/dev config source for any family whose verdict depends on an env-toggled control before reporting CLEAN or promoting a finding — plus a preference for a prod/dev-like profile on the dynamic pass. Tightens recall on env-sensitive families and precision on env-inflated ones. 362 lines (NFR-9 budget held), markdownlint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…al-PR critic A completeness-critic pass over the live user-service audit found three vuln classes the playbooks/catalog did not cover — each a real missed finding on a PR. Add them so future runs catch them: - Rate family: limiter-KEY abuse. The rate playbook only probed volume; it now probes the key-derivation — an attacker-controlled bucket key (request-body email, XFF header, account id) enables victim-bucket lockout (CWE-770+CWE-639) and per-key brute-force evasion, even when the limit value is correct. With a cross-ref to BOLA and a reminder to verify on a non-fuzz env boot. - GraphQL family: security-control-as-amplifier (CWE-674). The depth/complexity caps run in the executor; a kernel.request query inspector that walks a hostile unauthenticated query before the executor is a pre-cap recursion amplifier the "is max_query_depth set?" check never reaches. - Auth family: identity resolution / account linking (CWE-290/287/362) — normalization-collision uniqueness bypass (partial unique index + un-backfilled rows + TOCTOU) and social-login email-trust auto-link takeover. - Catalog: the SQLi/DQL dispatch row now lists CWE-943 (NoSQL operator injection) for doctrine-odm targets, fixing a label drift that under-weighted Mongo stacks. Pairs with feb5522 (boot-env-awareness): the schemathesis profile that disables the rate limiter is exactly why the limiter-key class was invisible to a by-the-book live run. NFR-2 denylist clean, markdownlint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…serialization playbook Convergence-critic pass found a class the deserialization family still missed: message-queue consumers deserialize the queue body exactly like an HTTP endpoint, but the playbook probed HTTP entry points only (cookie/header/body/upload). On the audited service, three messenger transports (send-email, failed-send-email, insert-user-batch) set no serializer and fall back to native_php_serializer (unserialize) on externally-writable SQS queues — a CWE-502 worker sink. The deserialization family now includes a worker-ingestion sub-probe: audit every messenger transport for a safe serializer vs the native-PHP default, flag any externally-writable transport on the native serializer, map routed message classes to handlers, reproduce via a local-broker publish, and cross-ref rate/resource exhaustion for the infinite-retry poison-message worker DoS. NFR-2 denylist clean, markdownlint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rd critic gap) Round-3 convergence critic found a 2FA brute-force throttle attached to the REST 2FA path but absent on the GraphQL mutations that reach the same handler (the GraphQL auth-target resolver maps only registration/signin) — CWE-307 second- factor brute-force / per-victim lockout-DoS bypass via the GraphQL door. Generalize it: the rate family now carries an "enforcement parity across protocol doors" probe — list every protocol surface (REST op + GraphQL mutation + message consumer) reaching a guarded handler and confirm the same limiter/ guard fires on all of them; a GraphQL mutation->limiter map omitting a family the REST resolver covers is a bypass. Parity check also applies to BFLA authz guards and CSRF. NFR-2 denylist clean, markdownlint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Kravalg
approved these changes
Jun 14, 2026
This was referenced Jun 14, 2026
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
2 participants
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.
Closes #6.
Adds an adversarial, authorized/defensive security-research capability to the
php-backend-sdlcplugin, built via the BMAD method + Ralph (autonomous planning chain inspecs/autonomous/2026-06-14-security-audit-skill/→ parallel-subagent implementation).What's added
skills/security-audit/SKILL.md(331 lines) — the multi-subagent loop: triage → fan out onesecurity-auditorper OWASP/vuln family in parallel → attack the running service (black-box HTTP/GraphQL) and SAST/taint/dependency/secret/config → verify every candidate by reproduction (no false positives) → map to CWE + OWASP id + CVSS-ish severity → root-cause, suppression-free fix viaphp-implementerwith a regression test per fix → re-dispatch only still-open families until a clean pass.MAX_ITERATIONS=5, explicit degrade paths, defensive/authorized-only boundary.skills/security-audit/reference/— full OWASP corpus across editions (Top 10 2003–2021, API 2019/2023, LLM, Mobile N/A-for-backend, ASVS 5.0, WSTG 4.2, CWE Top 25 2024) + per-family probe/reproduce playbooks + secure-by-default, cheat-sheet-cited remediations (keeps SKILL.md lean).agents/security-auditor.md— per-family red-team subagent (Bash/Read/Glob/Grep,opus); never edits code — verified findings route tophp-implementer.docs/profile-schema.md— newmake.security(nullable) +capabilities.dynamic_security_testingkeys./sdlc-reviewtriage,component-counts.bats(7 agents / 22 skills), README, parent architecture — all count-consistent.Safety
Strictly defensive / authorized-use-only: the skill + agent probe only the profile-resolved local service the user owns, never exfiltrate, mutate state only via the service's own API, run container-only, and never weaponize against third parties.
Verification (local)
197 bats (0 fail), markdownlint 0 errors, generalization-audit clean, profile-keys-check clean, frontmatter complete, SKILL.md ≤500 lines, counts 22 skills / 7 agents / 8 commands.
🤖 Generated with Claude Code
Summary by cubic
Adds a defensive security-audit to the
php-backend-sdlcplugin: a multi‑subagent OWASP red‑team that verifies findings and routes fixes viaphp-implementer, with asecurity-auditoragent and new profile keys. Now boot‑env aware, covers message‑queue worker deserialization, and enforces cross‑protocol guard parity; validation holds at 81/81 TP/TN. Closes #6.New Features
security-auditskill andsecurity-auditorsubagent: per‑family fan‑out, dynamic + static probing, verify by reproduction, map to CWE/OWASP + severity, then route fixes tophp-implementer.capabilities.dynamic_security_testingis true, boot viamake.startand pass an in‑scope base URL; degrade to static‑only when unavailable. Profile schema addsmake.security(nullable) andcapabilities.dynamic_security_testing.Bug Fixes
generate-profile.shshape and non‑regular‑file guards, and a TOCTOU fix ininject-governance.sh.Written for commit 4f79021. Summary will update on new commits.