fix(skills): strengthen DoubleClickjacking validation

strix/skills/coordination/root_agent.md

@@ -58,6 +58,13 @@ Structure agents by function:
 - Business logic flaws
 - Infrastructure vulnerabilities
 
+**Sensitive Browser Confirmations**
+- For every browser workflow containing a security-sensitive or consequential user action—whether authenticated or unauthenticated—assign `browser_security` to the agent testing that workflow or create a dedicated UI-redressing agent. This includes actions that change protected state, grant authority, submit credentials, expose sensitive data, or cross a trust boundary.
+- Its task must cover both iframe and cross-window/DoubleClickjacking paths; framing protections close only iframe coverage.
+- A workflow crossing origins or ports remains eligible when every involved origin is an explicit scan target. Same-origin policy blocks DOM reads, not top-level navigation, popups, opener relationships, focus changes, or window closing; cross-origin separation alone does not close UI-redressing coverage.
+- Never expand authorization from a link alone. If a required origin is not an explicit target, leave the path `needs_follow_up` and do not claim clickjacking or the related workflow is cleared; scope expansion must come from scan configuration.
+- Before accepting `no_issue_found` or `ruled_out` for cross-window UI redressing, reject closure evidence that cites popup-as-tab behavior, ignored geometry, unavailable OS-level input, or only a manual control click; keep the exact two-click path `needs_follow_up`. A successfully executed protected consent, approval, or grant must be impact-calibrated, not dismissed because it is idempotent, pre-seeded, or issues no immediate token or data.
+
 **Exploitation and Validation**
 - Proof-of-concept development
 - Impact demonstration

strix/skills/custom/dependency_cve_scanning.md

@@ -136,12 +136,8 @@ fi
 - A finding with a call stack ⇒ `reachability=reachable_call_path`, put the
   call-path excerpt (entrypoint → vulnerable function) in
   `reachability_evidence`.
-- Listed as affecting a required module but with no reachable symbol ⇒ the
-  call graph found no path to the vulnerable function: `not_reached` when
-  application code imports the module (cite the govulncheck result and the
-  import), `not_imported` when nothing imports it. When govulncheck could not
-  analyze the module (build errors, missing deps), fall back to the
-  import/symbol checks below instead of claiming a level.
+- Listed as affecting a required module but with no reachable symbol ⇒ fall
+  back to the import/symbol checks below (`imported` / `not_imported`).
 
 **All other ecosystems — import check, then symbol match:**
 
@@ -152,25 +148,15 @@ fi
    `PyYAML` → `yaml`). No hits ⇒ `not_imported`, with the search scope stated
    in `reachability_evidence`. For a **transitive** dependency, the check is
    whether application code imports it directly; if not, it is reachable only
-   through the packages that depend on it — walk every introducing chain
-   from the lockfile (e.g. `express > body-parser > qs`) and inspect the code
-   of the package one hop above the vulnerable one on each chain
-   (`body-parser`, not just `express`; its installed sources, or the class
-   references of its jar) for the affected API. No chain references it ⇒
-   `not_reached` (below), citing every artifact you inspected; referenced ⇒
-   that package's call is the usage site, continue the trace upward from
-   there. If you could not inspect every chain, or one of them is unclear,
-   the package is still loaded, so the level is `imported`, never
-   `not_reached`.
+   through the direct dependency — check whether the direct dep's usage can
+   hit it (if unclear, use `imported` when the direct dep is used at all).
 2. **Symbol match — per CVE, not per package.** Read each CVE's own advisory
    (GHSA/NVD/OSV `affected[].ecosystem_specific.imports` or the advisory
    text) for the affected functions/classes/APIs. Search application code for
    those symbols (`ast-grep` pattern or `rg -n`). Hits ⇒
    `vulnerable_symbol_used`, with repo-relative `file:line` of each hit (up
-   to a handful) in `reachability_evidence`. Imported, search completed, and
-   you can say why the vulnerable code does not run here ⇒ `not_reached`
-   (below). Imported but the search was partial, the result is unclear, or
-   the advisory names no symbols ⇒ `imported`.
+   to a handful) in `reachability_evidence`. Imported but no affected-symbol
+   usage found (or the advisory names no symbols) ⇒ `imported`.
    Different CVEs on the same package usually affect **different** symbols
    (one hits a parser, another a header check) — never copy one CVE's
    verdict/evidence onto its siblings; run the symbol search against each
@@ -196,23 +182,6 @@ fi
    dynamic loading, unparsable sources) ⇒ `unknown` and say why in
    `assumptions`.
 
-**`not_reached` — loaded, but this CVE's vulnerable code does not run here.**
-Use it when the package is imported (so `not_imported` is wrong) and you
-showed that the code path the advisory describes cannot execute in this
-codebase, and `reachability_evidence` says what you checked. The judgement is
-per CVE: it rests on what triggers *this* flaw (an affected function, a mode,
-an option, a kind of input) against what this code does. Situations that
-commonly support it, as inspiration rather than a checklist: the affected API
-has no caller in application code and the search was complete; the only call
-site is behind a condition that is false here (a polyfill for an older
-runtime, a flag that is off, a path only a dev tool runs); a transitive
-package's parent never references the affected API; the package is build or
-test tooling that the shipped artifact never executes; a library compiled into
-the runtime whose flaw sits in a mode the application never uses (a TLS server
-path in a client-only service). An inconclusive or skipped check stays
-`imported`. `not_reached` is still an evidence level, not proof of safety:
-dynamic dispatch, reflection and framework wiring can evade static search.
-
 Cheap-first budgeting: the import check is one search per package — always do
 it. Do the per-CVE symbol match for every CVE whose advisory names affected
 symbols (they can be batched into one multi-pattern search per package);
@@ -220,18 +189,13 @@ prioritize `critical`/`high`/KEV when the budget is tight; a CVE whose symbol
 search was skipped may still be reported as `imported` (the import check is
 real evidence), but its `reachability_evidence` must state that the
 affected-symbol check was not performed, so a skipped search is never
-mistaken for a completed one with no hits, and is never filed as
-`not_reached`. Never let this analysis stall
+mistaken for a completed one with no hits. Never let this analysis stall
 reporting — `unknown` with a reason beats an unverified claim.
 
 Anti-overclaim rules:
 
 - `not_imported` still does NOT mean safe (dynamic `import()`/reflection/
   framework wiring evade static search) — never phrase it as "not exploitable".
-- `not_reached` needs the check that shows the vulnerable code does not run,
-  named in `reachability_evidence`; "no call found" after a partial search is
-  `imported`, and one CVE's `not_reached` never carries over to a sibling CVE
-  on the same package.
 - `reachable_call_path` is reserved for call-graph tools (govulncheck); a
   symbol grep hit is `vulnerable_symbol_used`, no matter how convinced you are.
 - The tool rejects any level other than `unknown` without

strix/skills/vulnerabilities/browser_security.md

@@ -1,6 +1,6 @@
 ---
 name: browser-security
-description: Browser-internals security testing for browsing-context relationships, postMessage, client-side path traversal, XS-Leaks, service workers, Web Workers, navigation behavior, CSP interactions, caches, and cross-origin state machines
+description: Browser-internals security testing for clickjacking, UI redressing, DoubleClickjacking, browsing-context relationships, postMessage, client-side path traversal, XS-Leaks, service workers, Web Workers, navigation behavior, CSP interactions, caches, and cross-origin state machines
 ---
 
 # Browser Security
@@ -43,6 +43,15 @@ Draw the context graph. Security checks on `event.origin`, `event.source`, or a
 - Check whether a blocked intermediate frame leaves a useful browsing-context relationship intact.
 - Use random per-flow names or `_blank` with `noopener` where an opener relationship is unnecessary.
 
+### Clickjacking and UI Redressing
+
+- **UI-redressing closure gate:** Never mark clickjacking as ruled out from `X-Frame-Options` or CSP `frame-ancestors` alone. Before closure, test both frame embedding and cross-window/popup retargeting.
+- Do not treat a different origin or port as protection against UI redressing. Same-origin policy blocks DOM access, not top-level navigation, `window.open()`, opener relationships, focus changes, or window closing. Test only explicitly authorized targets; if a required origin is outside scope, keep the path `needs_follow_up` and do not claim the vulnerability class is cleared.
+- For framed cases, prove the overlay and sensitive click. For DoubleClickjacking, use two real browser clicks and trace click 1 → window/focus/navigation/positioning change → click 2 → sensitive control. Never substitute scripted `.click()` calls.
+- Reproduce in the victim state required by the workflow—authenticated when necessary—with realistic user activation and stable geometry; capture window bounds and timing.
+- **Evidence-limited closure:** Verify a repeatable sensitive server-side state change with a recording or deterministic event trace and a negative control. If the exact sequence cannot be tested, record `needs_follow_up`, not `ruled_out` or `no_issue_found`. Popup-as-tab behavior, ignored geometry, unavailable OS-level input, or a manual control click are not counterevidence. Close the path only when a named mitigation blocks it, or when geometry is honored and both physical clicks are delivered without a sensitive action.
+- **Impact floor:** Successful execution of a protected consent, approval, or grant is an integrity impact even when idempotent, pre-seeded, or followed by no immediate token or data access. Use downstream capability to calibrate severity, not erase the demonstrated protected action.
+
 ### Client-Side Path Traversal
 
 Trace the complete source-to-request pipeline:
@@ -146,7 +155,7 @@ Also inspect DevTools network initiators, service workers, storage, CSP violatio
 
 ### Source Review
 
-- Search for `postMessage`, message listeners, `window.open`, named targets, opener/parent access, frame creation, and sandbox attributes.
+- Search for `postMessage`, message listeners, `window.open`, named targets, opener/parent access, frame creation, `dblclick`, `mousedown`/`mouseup`, focus/blur, move/resize, navigation handlers, and sandbox attributes.
 - Search for router parameter APIs flowing into `fetch`, Axios, navigation, or HTML rendering.
 - Search for service-worker registration, Cache API writes, worker constructors, Blob URLs, and dynamic imports.
 - Search for raw HTML sinks and trust escape hatches in every supported frontend framework.
@@ -161,7 +170,7 @@ Also inspect DevTools network initiators, service workers, storage, CSP violatio
 5. **Prove the primitive** - Data transfer, path change, state oracle, cache modification, or context capture.
 6. **Escalate deliberately** - Chain to a privileged action, sensitive disclosure, SSRF, or executable DOM sink.
 7. **Cross-browser check** - At minimum record Chromium/Firefox/Safari applicability when the primitive is browser-specific.
-8. **State interaction requirements** - Click, drag, popup permission, timing window, login state, and visual deception.
+8. **State interaction requirements** - Record each physical click or drag, popup permission state, active context, focus, geometry, navigation, timing window, login state, and visual deception.
 
 ## Validation
 
@@ -171,7 +180,7 @@ Also inspect DevTools network initiators, service workers, storage, CSP violatio
 4. For XS-Leaks, repeat randomized success/failure trials and quantify separation and noise.
 5. For workers/caches, show which later context consumes the modified data.
 6. For client-side traversal, capture the final network request and the security-relevant response/action.
-7. For interaction-dependent chains, provide a screen recording or deterministic event trace.
+7. For interaction-dependent chains, show both click targets, intermediate context changes, and the resulting sensitive state change in a recording or deterministic event trace.
 
 ## False Positives
 

strix/tools/reporting/tool.py

@@ -1693,7 +1693,6 @@ _VALID_REACHABILITY = frozenset(
     {
         "not_imported",
         "imported",
-        "not_reached",
         "vulnerable_symbol_used",
         "reachable_call_path",
         "unknown",
@@ -1841,9 +1840,6 @@ def _build_dependency_metadata(
 _REACHABILITY_EVIDENCE_LABELS = {
     "not_imported": "not imported by application code",
     "imported": "imported by application code; affected API usage unconfirmed",
-    "not_reached": (
-        "imported by application code; the vulnerable code of this advisory is not reached"
-    ),
     "vulnerable_symbol_used": "the advisory's affected API is used in application code",
     "reachable_call_path": (
         "a call path from application code to the vulnerable function was proven"
@@ -2161,11 +2157,6 @@ async def create_dependency_report(
       can evade static search).
     - ``imported`` — application code imports the package, but usage of
       the advisory's affected API was not confirmed.
-    - ``not_reached`` — application code imports the package, and the
-      analysis showed that the vulnerable code of this advisory does not
-      run here (the affected API has no caller, the only call site is
-      behind a condition that is false in this codebase, the parent
-      package never reaches it, ...). Per advisory, never per package.
     - ``vulnerable_symbol_used`` — the advisory's affected
       function/class/API appears in application code.
     - ``reachable_call_path`` — a call-graph tool (e.g. ``govulncheck``)
@@ -2219,9 +2210,8 @@ async def create_dependency_report(
             repository root. This binds the finding to its exact file so
             remediation can target the right repository.
         reachability: Usage-evidence level from static analysis — one of
-            ``not_imported`` / ``imported`` / ``not_reached`` /
-            ``vulnerable_symbol_used`` / ``reachable_call_path`` /
-            ``unknown``. Claim only what the
+            ``not_imported`` / ``imported`` / ``vulnerable_symbol_used`` /
+            ``reachable_call_path`` / ``unknown``. Claim only what the
             evidence proves; when in doubt use ``unknown``.
         reachability_evidence: **Required.** The concrete proof for the
             claimed level, or, for ``unknown``, what you searched and why

tests/test_reporting_fields.py

@@ -668,39 +668,6 @@ async def test_dependency_report_records_reachability(report_state: ReportState)
     assert report["severity"] == "high"
 
 
-async def test_dependency_report_records_not_reached(report_state: ReportState) -> None:
-    result = await _do_create_dependency(
-        title="CVE-2021-23337 in lodash 4.17.20",
-        description="Command injection via template.",
-        target="repo/package.json",
-        cve="CVE-2021-23337",
-        package_name="lodash",
-        installed_version="4.17.20",
-        impact="Command injection where template is used.",
-        remediation_steps="Upgrade to 4.17.21.",
-        assumptions="Static search only; no dynamic dispatch into lodash was found.",
-        package_ecosystem="npm",
-        manifest_path="package-lock.json",
-        fixed_version="4.17.21",
-        cwe=None,
-        advisory_cvss=7.2,
-        technical_analysis=None,
-        fix_effort="low",
-        reachability="not_reached",
-        reachability_evidence=(
-            "src/util/strings.ts:3 imports lodash; a complete search of src/ finds no "
-            "call to _.template, the only API the advisory affects."
-        ),
-        contextual_cvss_breakdown=_DEP_CONTEXT,
-        contextual_cvss_reasoning=_DEP_REASONING,
-    )
-
-    assert result["success"] is True
-    report = report_state.vulnerability_reports[0]
-    assert report["dependency_metadata"]["reachability"] == "not_reached"
-    assert "the vulnerable code of this advisory is not reached" in report["evidence"]
-
-
 async def test_dependency_report_rejects_reachability_without_evidence(
     report_state: ReportState,
 ) -> None: