Add explicit CSPT detection skill, categorization, fixtures, and tests

[afraa786]

Add Client-Side Path Traversal (CSPT) detection support
Closes #776. Adds a dedicated CSPT skill, fixtures, a machine-readable finding_class, SARIF/dedup separation from server-side traversal, and automated regression coverage for source-to-sink detection and browser outbound-path confirmation.

Changes in response to review
The maintainer's "changes required" review raised 6 issues; all are addressed:

Real data-flow + browser/proxy detection is now tested.
tests/test_cspt_browser_capture.py drives every fixture in headless Chromium (Playwright) against a local HTTP request-capture server, and asserts the exact outbound path the server observed: positives emit the traversed path (e.g. /admin/keys/detail, /admin/config, /admin/settings, /admin/summary), negatives emit no traversed request. It also pins WHATWG URL ground truth: raw ../../ and %2e%2e/%2e%2e/ collapse, while un-decoded ..%2f..%2f does not.
tests/test_cspt_dataflow.py is a deterministic taint tracker that follows a source through assignments/transforms into a sink's path argument. It fails closed: a fixture with an unrelated source and sink (present but unconnected) is not flagged (proven by two meta-tests).
positive_normalized.html was reworked to read its route from the fragment (hash router), so it serves correctly under a plain static server.

%2f normalization guidance corrected. The skill and runbook no longer claim the browser decodes %2f/%2e during path normalization. They now distinguish browser-recognized dot-segments (.., %2e%2e with literal /), encoded separators that stay literal (%2f), and application-layer decoding before URL construction. Fragment examples use raw ../../. Every claim has a browser assertion.

SARIF primary-fingerprint collision fixed. A shared _class_token() helper prefers the structured finding_class over a title keyword and is used in both _class_fingerprint() and the synthetic branch of _primary_fingerprint(). The reviewer's exact repro (locationless CSPT vs. server-side traversal, both CWE-22) now yields different partialFingerprints.primaryLocationLineHash values; a regression test asserts this. CSPT reports must carry a discriminator (endpoint/method or code location) so multiple locationless CSPT findings don't collapse. finding_class is included in dedup (_prepare_report_for_comparison + candidate + judge prompt).

Confirmation model separated. The skill now distinguishes a confirmed CSPT primitive (outbound path differs due to attacker-controlled client input) from confirmed exploit impact (credentials / reach / chain). Credentials and reach drive reportability and severity but do not decide whether traversal occurred, preventing false negatives.

Loadable skill names fixed. All agent-facing references use the underscore keys client_side_path_traversal and path_traversal_lfi_rfi (including skill frontmatter). No hyphenated cross-references remain.

Docs updated. CSPT is listed in the public skills catalog (docs/advanced/skills.mdx). Branch rebased onto latest main (clean, no conflicts).

Verification
CSPT-specific tests: 54 passed
Full test suite: 321 passed, 1 skipped, 3 failed
ruff (lint + format): clean on changed files
mypy: clean
bandit: clean

The 3 full-suite failures (test_config_loader::test_persist_current_sets_0600_mode, two test_runner_root_prompt::…) reproduce on clean main and are not introduced by this PR.

Commands:
uv run python -m pytest tests/test_cspt_skill.py tests/test_cspt_dataflow.py tests/test_cspt_browser_capture.py # 54 passed
uv run python -m pytest # full suite
uv run ruff check strix/ tests/ && uv run mypy strix/ && uv run bandit -c pyproject.toml -r strix/

The browser tests require Playwright + Chromium (uv run playwright install chromium); they importorskip and skip cleanly where unavailable.

Known follow-up (not blocking)
uv.lock: left at format revision 2. The playwright>=1.40 dev dep is declared in pyproject.toml but not yet in the lock, because the local uv (0.11.26) rewrites the lock to revision 3 (renaming upload_time to upload-time on every entry): about 3000 lines of unrelated churn. Run uv lock with the repo's pinned uv (or pin uv via required-version) to add the dep cleanly. The release build (uv sync --frozen, default deps only) is unaffected.
CI: there is no test workflow in this repo today (only build-release.yml on tags + pre-commit.ci for lint/type/security). When a test CI is added, it should run uv run playwright install chromium so the CSPT browser tests execute rather than skip.

[greptile-apps]

Greptile Summary

This PR adds dedicated Client-Side Path Traversal detection and reporting support. The main changes are:

• A CSPT skill and browser evidence runbook.
• Machine-readable finding classification and SARIF metadata.
• Positive and negative browser fixtures.
• Tests for skill loading, fixture structure, and categorization.

Confidence Score: 4/5

SARIF class separation and the documented browser paths need fixes before merging.

• Structured finding classes are accepted and persisted correctly.
• Class fingerprints can still collapse distinct traversal classes when titles vary.
• Two core fixtures document request paths different from those the browser generates.

strix/report/sarif.py, tests/fixtures/cspt/positive_direct.html, tests/fixtures/cspt/positive_encoded.html, docs/cspt-evidence-runbook.md

Security Review

The CSPT guidance contains incorrect normalized-path expectations, which can produce misleading security evidence. SARIF class identity also remains dependent on title wording, allowing CSPT and server-side traversal findings to share a fingerprint.

Important Files Changed

Filename	Overview
strix/tools/reporting/tool.py	Adds validation and propagation for the optional CSPT finding class while preserving the existing default.
strix/report/sarif.py	Exports the structured finding class, but class fingerprints still depend on title keywords.
strix/skills/vulnerabilities/client_side_path_traversal.md	Adds detailed CSPT detection and reporting guidance, including path examples with incorrect normalization outcomes.
docs/cspt-evidence-runbook.md	Adds a manual browser and proxy workflow, but its main fixture example expects the wrong outbound path.
tests/fixtures/cspt/positive_direct.html	Adds a direct fragment-to-fetch traversal fixture whose documented normalized path is incorrect.
tests/fixtures/cspt/positive_encoded.html	Adds an encoded query-to-XHR traversal fixture whose payload escapes the documented API prefix.
tests/test_cspt_skill.py	Tests classification and fixture structure but does not verify actual normalized request paths or title-independent fingerprints.

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
strix/report/sarif.py:786
**Structured Class Can Still Collide**

A report with `finding_class="client_side_path_traversal"` but a title such as `Path Traversal via fragment` still matches the generic `path traversal` keyword. It then receives the same class fingerprint as server-side traversal, so dismissal tooling can collapse the two classes despite the new structured field. The fingerprint should use the non-default `finding_class`, with title matching only as a legacy fallback.

### Issue 2 of 3
tests/fixtures/cspt/positive_direct.html:8-9
**Traversal Reaches A Different Path**

Joining `../../admin/keys` to `/api/orders/` while retaining `/detail` normalizes to `/admin/keys/detail`, not `/api/admin/keys`. The fixture and runbook therefore tell investigators to expect the wrong request path from the browser, which can invalidate the intended evidence comparison.

### Issue 3 of 3
tests/fixtures/cspt/positive_encoded.html:8-9
**Encoded Example Escapes API Prefix**

After decoding, `/api/resources/../../admin/config` normalizes to `/admin/config`, not `/api/admin/config`, because the two parent segments remove both `resources` and `api`. The fixture consequently demonstrates a different endpoint from the one its evidence claims.

_{Reviews (1): Last reviewed commit: "Add explicit CSPT detection skill, categ..." | Re-trigger Greptile}

[afraa786]

Pushed a fix (c9cbfa7) addressing the Greptile review feedback:

• SARIF fingerprint now prioritizes the structured finding_class field over
  title-keyword matching, so CSPT findings no longer collapse onto the same
  fingerprint as server-side traversal when titles vary.
• Corrected the normalized path expectations in positive_direct.html and
  positive_encoded.html (both had incorrect .. traversal math) and updated
  the runbook/tests to match.

Ready for another look

[Am0stafa]

Review status: Changes required

Thank you for the work on this. The dedicated Client-Side Path Traversal (CSPT) skill, fixtures, reporting field, and documentation are a useful foundation. However, this PR cannot be accepted in its current form because it does not yet satisfy several acceptance criteria from issue #776, and parts of the validation guidance are technically incorrect.

The following items need to be addressed before the PR is ready for another review.

---

1. Actual data-flow detection and browser/proxy confirmation are not tested

The acceptance criteria explicitly require:

Automated tests prevent regressions in data-flow detection and browser/proxy confirmation.

The current tests do not exercise either capability.

test_positive_fixture_has_source_flowing_into_a_request_sink only verifies that each HTML file contains:

• Any recognized source string.

• Any recognized sink string.

• A textual concatenation marker.

This does not establish that the source actually flows into the sink. A fixture containing unrelated source and sink statements would still pass.

Likewise, the normalization tests exercise a hand-written Python path function. They do not validate browser URL parsing, execute a request, inspect an HTTP Archive (HAR), or confirm what appears at the proxy/server.

There is also no automated test showing that Strix:

1. Selects or loads the CSPT skill during a relevant scan.

2. Identifies the source-to-sink flow.

3. Drives the vulnerable source.

4. Observes the resulting outbound request.

5. Distinguishes positive and negative fixtures at runtime.

The PR description acknowledges that live confirmation is manual, but that does not meet the issue’s automated-regression requirement.

Required changes

• Add a browser-based integration harness around a local request-capture server.

• Execute every positive fixture and assert the exact outbound path observed by the server/browser.

• Execute every negative fixture and assert that no traversed request is emitted.

• Add a real data-flow or structural-analysis test. This can use an Abstract Syntax Tree (AST) query, Semgrep rule, or another deterministic implementation consistent with Strix’s architecture.

• If Caido cannot run in Continuous Integration (CI), use a local HTTP capture server for CI and retain a separate Caido integration/manual test for proxy-specific behavior.

• Ensure the tests fail if the source and sink exist but are not connected.

The positive_normalized.html fixture also cannot currently be exercised using the documented static server. Its source expects the page to be loaded under /view/..., but python -m http.server returns 404 for that route rather than serving the fixture. See the route-dependent source. This fixture needs an SPA fallback server or a different executable design.

---

2. The primary encoded-fragment example does not traverse

The runbook uses the following fragment payload:

..%2f..%2fadmin%2fkeys

See the runbook procedure.

In that flow, the application does not decode the fragment before concatenating it into the request URL. Percent-encoded slashes remain part of a path segment; the browser does not turn %2F into path separators during URL normalization.

Therefore:

/api/orders/..%2f..%2fadmin%2fkeys/detail

does not normalize to:

/admin/keys/detail

The WHATWG URL Standard recognizes .., .%2e, %2e., and %2e%2e as double-dot segments when separated by actual / delimiters. A segment containing %2f is not treated as a parent segment.

The skill currently makes the broader incorrect claim that browsers decode %2e and %2f during path normalization. See skill lines 119–124.

The encoded query fixture is not proof of that behavior because URLSearchParams.get() and decodeURIComponent() decode the query value before it reaches XMLHttpRequest.open().

Required changes

• Use raw ../../ for the direct fragment example; or

• Use encoded dots with literal separators, such as %2e%2e/%2e%2e/; or

• Explicitly decode the value in application code before concatenation, if the fixture is intended to model application-layer decoding.

• Correct the skill so it distinguishes:

  • Browser-recognized encoded dot segments.

  • Encoded separators that remain encoded.

  • Application/framework decoding before URL construction.

• Add a real URL/browser assertion for every documented encoded variant.

---

3. SARIF primary fingerprints can still collapse CSPT and server-side traversal findings

The latest commit correctly changed the custom class fingerprint to prefer the structured finding_class. However, the primary Static Analysis Results Interchange Format (SARIF) fingerprint used for code-scanning reconciliation still derives the synthetic class from the title.

See _primary_fingerprint().

For locationless Common Weakness Enumeration (CWE)-22 findings, both results are anchored to SECURITY.md. If a CSPT finding has a generic title such as Path Traversal via fragment, _class_keyword() returns path traversal, which is also returned for a server-side traversal finding.

I reproduced this using:

• A CSPT result with finding_class="client_side_path_traversal".

• A server-side traversal result with finding_class="dynamic".

• No physical code locations or endpoint metadata.

Both received the same partialFingerprints.primaryLocationLineHash, even though their custom strix_vuln_class_hash values differed.

The new fingerprint regression test asserts only the custom property. It does not inspect partialFingerprints, so the primary collision remains undetected.

Required changes

• Introduce a shared class-token helper that prefers a non-default structured finding_class.

• Use that helper in both _class_fingerprint() and the synthetic branch of _primary_fingerprint().

• Add a regression assertion that CSPT and server-side traversal receive different partialFingerprints.primaryLocationLineHash values.

• Require CSPT reports to populate endpoint/method or a physical source location when available. Otherwise, multiple locationless CSPT findings can still collapse together.

• Include finding_class in the deduplication candidate and _prepare_report_for_comparison(). Currently, reporting deduplication does not receive the new structured discriminator.

---

4. The confirmation model is too restrictive and will cause false negatives

The skill says a case is CSPT only when all of the following are present:

• A traversed path.

• Victim credentials.

• A benign control.

• Successful reach to an unintended endpoint.

It later classifies requests without credentials or successful reach as “not CSPT.” See the confirmation and false-positive sections.

This conflates two separate questions:

1. Does the CSPT primitive exist?
   Attacker-controlled client input changes the final request path.

2. What security impact is currently demonstrated?
   Credentials, sensitive data, state change, response processing, Cross-Site Scripting chains, or another reachable target.

A path manipulation primitive may be valid even when the immediate request is unauthenticated or returns 404. It may still become exploitable when chained with response rendering, a public state-changing endpoint, application-added authorization, or another client-side sink.

Required changes

Separate the validation model into:

• Confirmed CSPT primitive: the final outbound path differs from the intended path because of attacker-controlled client input.

• Confirmed exploit impact: the request reaches a meaningful endpoint or participates in a demonstrated chain.

Credentials and successful reach should affect reportability and severity, but they should not redefine whether path traversal occurred.

---

5. Skill references use a non-loadable name

The runtime-discoverable skill key is:

client_side_path_traversal

However, the reporting tool refers agents to:

client-side-path-traversal

See the reporting-tool documentation.

The hyphenated name is not accepted by load_skill; the loader resolves the filename-based underscore name. The CSPT skill similarly refers to path-traversal-lfi-rfi, while the loadable key is path_traversal_lfi_rfi.

Required changes

• Use the actual loadable keys in all agent-facing references:

  • client_side_path_traversal

  • path_traversal_lfi_rfi

• Alternatively, intentionally implement frontmatter-name aliases in the skill loader and test both forms.

---

6. Documentation and PR state need cleanup

The public skills documentation does not list the new CSPT skill, leaving the documented skill catalog incomplete.

The PR description also reports 25 CSPT tests, but the current branch contains 30. The full-suite counts are similarly stale.

At the time of this review, the branch is 11 commits behind main. A virtual merge completed without conflicts and the CSPT-specific tests still passed, but the branch should be updated before final review.

Required changes

• Add CSPT to the public skills catalog.

• Update the PR description with current test counts and commands.

• Rebase or merge the latest main.

• Re-run all checks after updating the branch.

• Ensure CI or another recorded status check is attached to the final commit.

---

Verification performed

Check | Result -- | -- CSPT-specific tests on the PR head | 30 passed CSPT-specific tests after a virtual merge with current main | 30 passed Ruff lint and formatting | Passed mypy | Passed Bandit | Passed git diff --check | Passed Full test suite | 270 passed, 1 skipped, 2 failed

The two full-suite failures were reproduced on current main and do not appear to be introduced by this PR. Nevertheless, the passing CSPT tests are primarily structural and do not prove the runtime detection behavior required by the issue.

---

Required before re-review

• Add real automated source-to-sink detection coverage.

• Add browser-based outbound-request assertions for every fixture.

• Make the normalized route fixture executable in the documented environment.

• Correct the %2f normalization guidance and affected examples.

• Fix the primary SARIF fingerprint collision.

• Include finding_class in report deduplication.

• Separate CSPT primitive confirmation from impact validation.

• Correct loadable skill names in cross-references.

• Update the public skills documentation.

• Update/rebase the branch and refresh the PR’s verification results.

Once these blockers are addressed with reproducible evidence, the PR should be in a much stronger position for acceptance.

## Review status: Changes required

Thank you for the work on this. The dedicated Client-Side Path Traversal (CSPT) skill, fixtures, reporting field, and documentation are a useful foundation. However, this PR cannot be accepted in its current form because it does not yet satisfy several acceptance criteria from [issue #776](#776), and parts of the validation guidance are technically incorrect.

The following items need to be addressed before the PR is ready for another review.

---

1. Actual data-flow detection and browser/proxy confirmation are not tested

The acceptance criteria explicitly require:

Automated tests prevent regressions in data-flow detection and browser/proxy confirmation.

The current tests do not exercise either capability.

[test_positive_fixture_has_source_flowing_into_a_request_sink](https://github.com/usestrix/strix/blob/c9cbfa76665869b815605aa65751dc1ce78d636b/tests/test_cspt_skill.py#L116-L150) only verifies that each HTML file contains:

• Any recognized source string.
• Any recognized sink string.
• A textual concatenation marker.

This does not establish that the source actually flows into the sink. A fixture containing unrelated source and sink statements would still pass.

Likewise, the [normalization tests](

strix/tests/test_cspt_skill.py

Lines 349 to 402 in c9cbfa7

	def _normalize_url_path(path: str) -> str:
	"""Resolve ``.`` / ``..`` segments the way a browser normalizes a URL path.
	Leading ``..`` that would escape the root are clamped (dropped), matching
	the URL parser rather than a filesystem join.
	"""
	parts = path.split("/")
	out: list[str] = []
	for part in parts:
	if part == "..":
	if out and out[-1] not in ("", ".."):
	out.pop()
	elif part != ".":
	out.append(part)
	return "/".join(out)
	def test_normalize_helper_matches_documented_examples() -> None:
	# Guard the helper itself so the assertions below mean something.
	assert _normalize_url_path("/api/orders/../../admin/keys/detail") == "/admin/keys/detail"
	assert _normalize_url_path("/api/resources/../../admin/config") == "/admin/config"
	assert _normalize_url_path("/api/users/../../admin/keys") == "/admin/keys"
	def test_positive_direct_fixture_documents_correct_normalized_path() -> None:
	text = (FIXTURES / "positive_direct.html").read_text(encoding="utf-8")
	# The fixture builds this request URL from orderId="../../admin/keys":
	assert _normalize_url_path("/api/orders/../../admin/keys/detail") == "/admin/keys/detail"
	# ...and the comment must document that correct result, not the wrong
	# /api/admin/keys.
	assert "/admin/keys/detail" in text
	assert "/api/admin/keys" not in text
	def test_positive_encoded_fixture_documents_correct_normalized_path() -> None:
	text = (FIXTURES / "positive_encoded.html").read_text(encoding="utf-8")
	assert _normalize_url_path("/api/resources/../../admin/config") == "/admin/config"
	assert "/admin/config" in text
	assert "/api/admin/config" not in text
	def test_no_fixture_or_doc_claims_the_wrong_api_admin_path() -> None:
	# Belt-and-suspenders: the incorrect "/api/admin/..." normalized paths must
	# not reappear in any fixture, the skill, or the runbook.
	repo = Path(__file__).parent.parent
	targets = [
	FIXTURES / "positive_direct.html",
	FIXTURES / "positive_encoded.html",
	repo / "docs" / "cspt-evidence-runbook.md",
	repo / "strix" / "skills" / "vulnerabilities" / "client_side_path_traversal.md",
	]
	for path in targets:
	text = path.read_text(encoding="utf-8")
	assert "/api/admin/keys" not in text, f"{path.name} claims wrong path /api/admin/keys"

) exercise a hand-written Python path function. They do not validate browser URL parsing, execute a request, inspect an HTTP Archive (HAR), or confirm what appears at the proxy/server.

There is also no automated test showing that Strix:

1. Selects or loads the CSPT skill during a relevant scan.
2. Identifies the source-to-sink flow.
3. Drives the vulnerable source.
4. Observes the resulting outbound request.
5. Distinguishes positive and negative fixtures at runtime.

The PR description acknowledges that live confirmation is manual, but that does not meet the issue’s automated-regression requirement.

Required changes

• Add a browser-based integration harness around a local request-capture server.
• Execute every positive fixture and assert the exact outbound path observed by the server/browser.
• Execute every negative fixture and assert that no traversed request is emitted.
• Add a real data-flow or structural-analysis test. This can use an Abstract Syntax Tree (AST) query, Semgrep rule, or another deterministic implementation consistent with Strix’s architecture.
• If Caido cannot run in Continuous Integration (CI), use a local HTTP capture server for CI and retain a separate Caido integration/manual test for proxy-specific behavior.
• Ensure the tests fail if the source and sink exist but are not connected.

The positive_normalized.html fixture also cannot currently be exercised using the documented static server. Its source expects the page to be loaded under /view/..., but python -m http.server returns 404 for that route rather than serving the fixture. See [the route-dependent source](

strix/tests/fixtures/cspt/positive_normalized.html

Lines 16 to 23 in c9cbfa7

	// SOURCE: route segment, e.g. /view/....//....//admin/settings
	const segment = location.pathname.replace(/^\/view\//, "");
	// Ineffective sanitizer: a single non-recursive strip. "....//" -> "../".
	const cleaned = segment.replace(/\.\.\//g, "");
	// SINK: still-tainted value joined into the request path — CSPT.
	fetch("/api/profile/" + cleaned, { credentials: "include" })

). This fixture needs an SPA fallback server or a different executable design.

---

2. The primary encoded-fragment example does not traverse

The runbook uses the following fragment payload:

..%2f..%2fadmin%2fkeys

See [the runbook procedure](

strix/docs/cspt-evidence-runbook.md

Lines 41 to 44 in c9cbfa7

	# 3b. TRAVERSAL — drive the same source with a crafted value.
	agent-browser open "<target>/#/orders/..%2f..%2fadmin%2fkeys"
	agent-browser wait --load networkidle
	agent-browser network requests # note the traversed path, e.g. /admin/keys/detail

).

In that flow, the application does not decode the fragment before concatenating it into the request URL. Percent-encoded slashes remain part of a path segment; the browser does not turn %2F into path separators during URL normalization.

Therefore:

/api/orders/..%2f..%2fadmin%2fkeys/detail

does not normalize to:

/admin/keys/detail

The [WHATWG URL Standard](https://url.spec.whatwg.org/#url-path-segment) recognizes .., .%2e, %2e., and %2e%2e as double-dot segments when separated by actual / delimiters. A segment containing %2f is not treated as a parent segment.

The skill currently makes the broader incorrect claim that browsers decode %2e and %2f during path normalization. See [skill lines 119–124](

strix/strix/skills/vulnerabilities/client_side_path_traversal.md

Lines 119 to 124 in c9cbfa7

	- **Direct**: `../`, `../../`, `..%2f` un-decoded, leading `/` (absolute-path
	override: `id=/admin/keys` → `/api//admin/keys` or an absolute join).
	- **Encoded**: `%2e%2e%2f`, `%2e%2e/`, `..%2f`, double-encoded `%252e%252e%252f`
	when an app decodes before joining. The browser decodes `%2e`/`%2f` in the path
	during normalization — test both raw and encoded because framework routers may
	decode a layer first.

).

The encoded query fixture is not proof of that behavior because URLSearchParams.get() and decodeURIComponent() decode the query value before it reaches XMLHttpRequest.open().

Required changes

• Use raw ../../ for the direct fragment example; or

• Use encoded dots with literal separators, such as %2e%2e/%2e%2e/; or

• Explicitly decode the value in application code before concatenation, if the fixture is intended to model application-layer decoding.

• Correct the skill so it distinguishes:

  • Browser-recognized encoded dot segments.
  • Encoded separators that remain encoded.
  • Application/framework decoding before URL construction.

• Add a real URL/browser assertion for every documented encoded variant.

---

3. SARIF primary fingerprints can still collapse CSPT and server-side traversal findings

The latest commit correctly changed the custom class fingerprint to prefer the structured finding_class. However, the primary Static Analysis Results Interchange Format (SARIF) fingerprint used for code-scanning reconciliation still derives the synthetic class from the title.

See [_primary_fingerprint()](https://github.com/usestrix/strix/blob/c9cbfa76665869b815605aa65751dc1ce78d636b/strix/report/sarif.py#L892-L899).

For locationless Common Weakness Enumeration (CWE)-22 findings, both results are anchored to SECURITY.md. If a CSPT finding has a generic title such as Path Traversal via fragment, _class_keyword() returns path traversal, which is also returned for a server-side traversal finding.

I reproduced this using:

• A CSPT result with finding_class="client_side_path_traversal".
• A server-side traversal result with finding_class="dynamic".
• No physical code locations or endpoint metadata.

Both received the same partialFingerprints.primaryLocationLineHash, even though their custom strix_vuln_class_hash values differed.

The new [fingerprint regression test](

strix/tests/test_cspt_skill.py

Lines 303 to 341 in c9cbfa7

	def test_sarif_class_fingerprint_uses_finding_class_over_colliding_title(tmp_path: Path) -> None:
	# Regression: a CSPT finding whose *title* contains the generic "path
	# traversal" keyword must still be separated from a genuine server-side
	# path-traversal finding. Before the fix the class fingerprint was derived
	# from the title keyword alone, so both collapsed onto the same hash even
	# though the structured finding_class distinguished them. The structured
	# field must win.
	write_sarif(
	tmp_path,
	[
	{
	"id": "vuln-0001",
	"title": "Path Traversal via fragment", # collides on the generic keyword
	"severity": "high",
	"cwe": "CWE-22",
	"finding_class": "client_side_path_traversal",
	"timestamp": "2026-07-16 10:00:00 UTC",
	"code_locations": [{"file": "app.js", "start_line": 12}],
	},
	{
	"id": "vuln-0002",
	"title": "Path Traversal in download file parameter",
	"severity": "high",
	"cwe": "CWE-22",
	"finding_class": "dynamic",
	"timestamp": "2026-07-16 10:00:00 UTC",
	"code_locations": [{"file": "server.py", "start_line": 30}],
	},
	],
	)
	results = json.loads((tmp_path / "findings.sarif").read_text(encoding="utf-8"))["runs"][0][
	"results"
	]
	hashes = [r["properties"].get("strix_vuln_class_hash") for r in results]
	assert all(hashes), "expected a class fingerprint on both findings"
	assert hashes[0] != hashes[1], (
	"CSPT finding with a 'path traversal' title collapsed onto the "
	"server-side traversal fingerprint; finding_class must take precedence"
	)

) asserts only the custom property. It does not inspect partialFingerprints, so the primary collision remains undetected.

Required changes

• Introduce a shared class-token helper that prefers a non-default structured finding_class.
• Use that helper in both _class_fingerprint() and the synthetic branch of _primary_fingerprint().
• Add a regression assertion that CSPT and server-side traversal receive different partialFingerprints.primaryLocationLineHash values.
• Require CSPT reports to populate endpoint/method or a physical source location when available. Otherwise, multiple locationless CSPT findings can still collapse together.
• Include finding_class in the deduplication candidate and _prepare_report_for_comparison(). Currently, reporting deduplication does not receive the new structured discriminator.

---

4. The confirmation model is too restrictive and will cause false negatives

The skill says a case is CSPT only when all of the following are present:

• A traversed path.
• Victim credentials.
• A benign control.
• Successful reach to an unintended endpoint.

It later classifies requests without credentials or successful reach as “not CSPT.” See the [confirmation and false-positive sections](

strix/strix/skills/vulnerabilities/client_side_path_traversal.md

Lines 168 to 220 in c9cbfa7

	**Confirmation criteria** — you have a real CSPT when *all* hold:
	1. The **outbound request path in the proxy/HAR** is the traversed target
	(e.g. `/admin/keys`), not the intended one (`/api/users/<id>/avatar`).
	2. That request carried the **victim's credentials** (session cookie /
	`Authorization` header present on the request — read it from the HAR).
	3. There is a **benign same-endpoint control**: the same flow with a normal
	value hits the intended path. Show both paths side by side.
	4. The traversed endpoint returned data (or performed an action) the source
	value should not have been able to reach.
	Record the request/response pair (method, full path, relevant headers, status)
	from the HAR as evidence. Prefer reading a low-sensitivity sibling endpoint to
	prove reach; do not exfiltrate real secrets — a status/length/shape delta is
	enough proof.
	## Reporting
	File confirmed CSPT with `create_vulnerability_report` and:
	- `finding_class="client_side_path_traversal"` — **required** so the finding is
	separated from server-side path traversal / LFI / RFI (which share the CWE but
	are a different class). Do not leave it `dynamic`.
	- `cwe="CWE-22"` (Improper Limitation of a Pathname to a Restricted Directory).
	- `title` prefixed **`Client-Side Path Traversal`** (e.g.
	`"Client-Side Path Traversal in order-detail fetch via URL fragment"`), so the
	class is obvious in the report/SARIF even to a human skimming titles.
	- `evidence` — the two request paths (intended vs. traversed) from the proxy/HAR,
	with the credential header present, plus the benign control. Fenced code blocks.
	- `poc_description` — steps to reproduce (navigate/craft source → observe path).
	- `poc_script_code` — the crafted URL / `postMessage` / storage-seeding snippet.
	- `impact` — name the concrete sibling endpoint reached and what it exposes or
	changes; if chained (XSS/CSRF/CORS), state the chain.
	- CVSS — usually `AV:N`, `UI:R` (victim opens a link), `PR:N`; scope/impact
	depends on the reached endpoint.
	## False positives
	Not CSPT — do not report:
	- The source is **validated to the resulting path**: an allowlist of endpoints,
	a strict `^[a-z0-9-]+$` segment check, or `encodeURIComponent` on a value used
	as a **single segment** *and* the app rejects/encodes `/` and `.` so traversal
	can't form. (`encodeURIComponent` alone is only safe when the value is one
	segment and `%2f`/`%2e` aren't decoded downstream — verify the outbound path.)
	- **Safe path construction**: the endpoint path is a constant and the client
	value only ever becomes a **query parameter or request body field**, never part
	of the path. A tainted query value is not CSPT (it may be another bug).
	- The browser **does not normalize** the value into a different path (confirm on
	the wire — no traversal actually occurred in the outbound request).
	- The traversed request is **not** sent with ambient credentials (no session on
	that request), so there's no privilege to abuse — note it, don't over-rate it.
	- Server rejects the traversed path (404/403) with no differential — no reach, no

).

This conflates two separate questions:

1. Does the CSPT primitive exist?
   Attacker-controlled client input changes the final request path.

2. What security impact is currently demonstrated?
   Credentials, sensitive data, state change, response processing, Cross-Site Scripting chains, or another reachable target.

A path manipulation primitive may be valid even when the immediate request is unauthenticated or returns 404. It may still become exploitable when chained with response rendering, a public state-changing endpoint, application-added authorization, or another client-side sink.

Required changes

Separate the validation model into:

• Confirmed CSPT primitive: the final outbound path differs from the intended path because of attacker-controlled client input.
• Confirmed exploit impact: the request reaches a meaningful endpoint or participates in a demonstrated chain.

Credentials and successful reach should affect reportability and severity, but they should not redefine whether path traversal occurred.

---

5. Skill references use a non-loadable name

The runtime-discoverable skill key is:

client_side_path_traversal

However, the reporting tool refers agents to:

client-side-path-traversal

See [the reporting-tool documentation](

strix/strix/tools/reporting/tool.py

Lines 565 to 578 in c9cbfa7

	finding_class: Optional finding sub-class for machine-readable
	categorization. Defaults to ``"dynamic"``. Set to
	``"client_side_path_traversal"`` for a confirmed CSPT finding
	— attacker-controlled client-side input (URL params, path
	segments, fragment, ``document.referrer``, ``postMessage``
	data, browser storage) reaching a browser request sink
	(``fetch`` / ``XMLHttpRequest`` / ``axios`` / ``$.ajax`` /
	``EventSource`` / ``WebSocket``) so the victim's own
	authenticated browser is steered to an unintended same-origin
	endpoint. CSPT shares ``CWE-22`` with server-side path
	traversal / LFI / RFI but is a distinct class; setting this
	keeps it separated in the report artifacts and SARIF. Do NOT
	use it for server-side traversal / LFI / RFI (leave those
	``dynamic``). See the ``client-side-path-traversal`` skill.

).

The hyphenated name is not accepted by load_skill; the loader resolves the filename-based underscore name. The CSPT skill similarly refers to path-traversal-lfi-rfi, while the loadable key is path_traversal_lfi_rfi.

Required changes

• Use the actual loadable keys in all agent-facing references:

  • client_side_path_traversal
  • path_traversal_lfi_rfi

• Alternatively, intentionally implement frontmatter-name aliases in the skill loader and test both forms.

---

6. Documentation and PR state need cleanup

The public [skills documentation](https://github.com/usestrix/strix/blob/c9cbfa76665869b815605aa65751dc1ce78d636b/docs/advanced/skills.mdx) does not list the new CSPT skill, leaving the documented skill catalog incomplete.

The PR description also reports 25 CSPT tests, but the current branch contains 30. The full-suite counts are similarly stale.

At the time of this review, the branch is 11 commits behind main. A virtual merge completed without conflicts and the CSPT-specific tests still passed, but the branch should be updated before final review.

Required changes

• Add CSPT to the public skills catalog.
• Update the PR description with current test counts and commands.
• Rebase or merge the latest main.
• Re-run all checks after updating the branch.
• Ensure CI or another recorded status check is attached to the final commit.

---

Verification performed

Check	Result
CSPT-specific tests on the PR head	30 passed
CSPT-specific tests after a virtual merge with current main	30 passed
Ruff lint and formatting	Passed
mypy	Passed
Bandit	Passed
git diff --check	Passed
Full test suite	270 passed, 1 skipped, 2 failed

The two full-suite failures were reproduced on current main and do not appear to be introduced by this PR. Nevertheless, the passing CSPT tests are primarily structural and do not prove the runtime detection behavior required by the issue.

---

Required before re-review

• Add real automated source-to-sink detection coverage.
• Add browser-based outbound-request assertions for every fixture.
• Make the normalized route fixture executable in the documented environment.
• Correct the %2f normalization guidance and affected examples.
• Fix the primary SARIF fingerprint collision.
• Include finding_class in report deduplication.
• Separate CSPT primitive confirmation from impact validation.
• Correct loadable skill names in cross-references.
• Update the public skills documentation.
• Update/rebase the branch and refresh the PR’s verification results.

Once these blockers are addressed with reproducible evidence, the PR should be in a much stronger position for acceptance.

[afraa786]

@Am0stafa Pushed commits addressing all 6 points from your review, with real
browser-based detection tests (headless Chromium + local capture server) and
a proper source-to-sink dataflow test, rather than the earlier structural
checks. Also corrected the %2f normalization guidance, fixed the primary
SARIF fingerprint collision, separated CSPT primitive confirmation from
exploit impact, fixed the non-loadable skill name references, and updated
the docs. Updated the PR description above with full details and
verification.

One known, explicitly-flagged follow-up: the uv.lock isn't yet updated with
the playwright dev dependency due to a uv version/format-revision mismatch
locally (details in the description) happy to resolve however you'd
prefer. There's also no test CI workflow in this repo currently, so the new
browser tests run locally (they cleanly skip where Playwright/Chromium
aren't installed).

Ready for another look whenever convenient thanks for the detailed review,
it caught some real correctness issues.

[Am0stafa]

Strong, high-quality PR — merge-ready after one packaging fix.

1. uv.lock is out of sync with pyproject.toml. The PR adds playwright>=1.40 to the dev group, but the lockfile has zero playwright entries. In a uv-managed repo, uv sync / uv lock --check will flag this, and any future test-CI using --frozen for dev deps will fail. The author flagged it as a known follow-up, but it should be resolved before merge — run uv lock with the repo's pinned uv (or uv lock --upgrade-package playwright) and commit the result. If the churn from a newer local uv (revision 2→3 rewrite) is the obstacle, pin required-version for uv in pyproject.toml so the lock stays on revision
2. Missing "default wiring." The skill is auto-discoverable (globbed → appears in <available_skills>), so it works, but nothing nudges agents toward CSPT. To make detection explicit rather than merely available, add: a GOOD specialization example in strix/agents/prompts/system_prompt.jinja (e.g. "CSPT Chain Agent" with skills: client_side_path_traversal, csrf, xss); a coverage bullet in strix/skills/scan_modes/deep.md (near prototype pollution / CORS); and a one-line "Related: client_side_path_traversal" cross-reference from csrf.md, xss.md, and open_redirect.md (the new skill already links out to path_traversal_lfi_rfi, but nothing links in).
3. path_traversal_lfi_rfi.md frontmatter name: changed to underscore. This diverges from the kebab-case name: used by the other ~23 skills, and it's a no-op for loading — the loader strips frontmatter and resolves skills by filename stem, so loadability was never affected by the frontmatter value. Either revert it (keep the repo consistent) or, if the intent is a project-wide convention change, do it across all skills in a separate change. The PR body's rationale ("loadable skill names fixed … including skill frontmatter") is slightly misattributed here.
4. New browser-automation stack. The tests introduce Playwright, distinct from the product's own agent-browser. It's test-only and importorskips cleanly, so it's acceptable — just worth a maintainer's explicit nod, since it's a second Chromium driver in the tree.

[Am0stafa]

@0xallam, @bearsyankees
Please review and merge this PR for this useful feature.