tests(sarif): schema-validation suite never exercises a path needing uri encoding, can't catch a path_to_uri_reference regression

[dekobon]

Summary

The SARIF schema-validation integration suite in tests/sarif_test.rs
never feeds a path that exercises path_to_uri_reference's encoding
branches (spaces, backslashes, non-ASCII, colons), so it cannot catch a
regression that makes the writer emit an artifactLocation.uri the
vendored SARIF schema's uri-reference format constraint rejects —
the exact failure that encoding exists to prevent.

Location

• tests/sarif_test.rs:124-207 (the five *_validates_against_schema
  tests + the canary), all using clean ASCII paths such as
  src/foo.rs, a.rs, src/alpha.rs.
• tests/sarif_test.rs:116-122 (the section doc claiming these tests
  catch "a wrong-typed value, an extra property the schema rejects").
• Guards production code at src/output/sarif.rs:58-95
  (path_to_uri_reference), whose doc (lines 48-52) states it
  percent-encodes "so spaces and other path characters survive
  validation" under "the uri-reference format ... GitHub Code
  Scanning uses."

Evidence

The vendored schema constrains the URI field with a real assertion the
validator enforces:

tests/fixtures/sarif-2.1.0.json:211-214
  "uri": { ... "format": "uri-reference" }

jsonschema::draft7::new(&schema) (tests/common/validators.rs:55)
validates format for Draft-07 by default (the crate's format
validation is draft-dependent; Draft-07 treats format as an
assertion). A raw space is not a legal uri-reference character, so a
document whose uri is src/my file.rs (unencoded) would fail
validate_sarif.

Production deliberately percent-encodes to avoid exactly this
(src/output/sarif.rs:84-92). But every offender fed to the
schema-validation tests has a path that is already a clean
uri-reference requiring no transformation:

• sarif_single_offender_validates_against_schema → src/foo.rs
• sarif_multi_offender_validates_against_schema → src/alpha.rs, src/beta.rs
• sarif_error_severity_validates_against_schema → src/foo.rs
• sarif_omitted_optional_fields_validates_against_schema → a.rs

The encoding branch is asserted only at the unit level in
src/output/sarif.rs:634 (space_is_percent_encoded), which checks the
string output of path_to_uri_reference directly but never runs the
result through schema validation. So the one place that proves
schema-conformance (the integration suite) and the one place that
exercises encoding (the unit test) never meet.

Concretely: if path_to_uri_reference regressed to stop
percent-encoding spaces (or emitted a malformed escape), the unit test
space_is_percent_encoded would catch the exact-string break, but the
integration schema suite — the test that exists to certify
GitHub-Code-Scanning-ingestible output — would stay green, because no
fixture carries a character that needs encoding.

Expected Behavior

At least one *_validates_against_schema test should use an offender
path that forces a non-trivial path_to_uri_reference transformation
(e.g. a path with an embedded space, and ideally a Windows-style
backslash path), so the suite proves the encoded URI still satisfies
the schema's uri-reference format — not merely that an
already-clean path does.

Actual Behavior

All schema-validation fixtures use paths that pass through
path_to_uri_reference unchanged, so the suite would still pass even if
the encoding logic were removed entirely.

Impact

A regression in path_to_uri_reference that produces schema-invalid
uri-reference values for real-world paths (spaces are common in
fixture/temp paths; backslashes on Windows runners) would ship
undetected by the integration suite whose stated purpose is to certify
schema conformance. This is the lib-side analogue of the SARIF
URI-correctness concerns already tracked in #798 (colon scheme
ambiguity), but it is a distinct test-quality gap, not a duplicate:
#798 is a production bug in the colon branch; this is the absence of any
schema-validation fixture that exercises any encoding branch.

[dekobon]

General Plan for Fix

Root Cause

The schema-validation integration tests were written to certify that
emitted SARIF satisfies the vendored Draft-07 schema (including its
uri-reference format assertion on artifactLocation.uri), but every
fixture path is already a valid uri-reference, so path_to_uri_reference
is a no-op for them. The encoding logic — the only thing standing between
a real OS path and a schema-invalid uri — is exercised only by a unit
test that asserts the raw string, never by a test that runs the encoded
output through the schema validator.

Implementation Steps

1. Add one schema-validation test that constructs an OffenderRecord
   with a space-bearing relative path (e.g. src/my dir/my file.rs),
   renders it, and calls assert_valid_sarif — proving the
   percent-encoded uri (src/my%20dir/my%20file.rs) still validates.
2. Optionally assert the produced uri value is the encoded form (so
   the test fails loud if encoding is dropped and if the schema were
   ever relaxed), keeping the existing well-formedness assertion as the
   primary guard.
3. Consider a second fixture with a Windows-style backslash path
   (src\\a b\\c.rs) to cover the \ -> / plus encoding path, which
   the Windows runners actually produce.

These are additive test changes only; no production code changes are
required (the encoding behavior is already correct — this closes the
detection gap, not a live bug).

Tests to Add/Update

• New: sarif_space_in_path_validates_against_schema (space path ->
  encoded uri -> assert_valid_sarif).
• Optional: sarif_backslash_path_validates_against_schema.

Verification

• cargo test -p big-code-analysis --test sarif_test passes.
• Revert-check: temporarily neuter the percent-encoding arm in
  path_to_uri_reference (push the raw byte instead of %XX) and
  confirm the new test FAILS on the schema uri-reference violation
  while the old clean-path tests still pass — proving the new test
  guards the encoding. Restore afterward.
• cargo clippy -p big-code-analysis clean.

[dekobon]

Added sarif_space_in_path_validates_against_schema in
tests/sarif_test.rs. It renders an OffenderRecord whose path is
src/my dir/my file.rs, asserts the emitted
artifactLocation.uri is the percent-encoded
src/my%20dir/my%20file.rs, and runs it through
assert_valid_sarif (the Draft-07 uri-reference format
assertion). This is the first schema-validation fixture whose path
actually needs path_to_uri_reference to transform it.

Verified via test-via-revert: neutering the percent-encoding arm in
path_to_uri_reference (src/output/sarif.rs) to push the raw byte
instead of %XX makes the new test fail (the emitted uri carries a
literal space); restoring the encoding passes, and the existing
clean-path tests stay green throughout. Production code unchanged.
Committed test(sarif): validate a uri-encoded path against the schema (Fixes #949); not closed.