From b36496fa944c4f87fe8c4bf6a957c39f1e9b0972 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 18:54:32 +0000 Subject: [PATCH 1/2] feat(identity): freeze finding-pattern/v1 with canonical vectors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 0 of the identity/provenance work (Own.NET#266): make the pattern-level identity a contract with one implementation and executable vectors, WITHOUT changing a single emitted byte. The recipe — sha1(path \x1f rule \x1f message)[:16] — moves from viz/apply_verdicts into identity/pattern.py unchanged, and finding_id stays as the overlay-facing alias. Nothing about the output moves, which is the entire point: every stored fp-verdicts.json is keyed by those bytes, the freshness guard only proves the verdicts were judged against the same findings file, and an overlay that stops matching yields an EMPTY REPORT rather than an error. A silent failure mode earns a test, not a comment. contracts/finding-pattern-v1.json is the executable half. Its vectors pin what is deliberately NOT normalized — path separator, path case, digits and whitespace in the message — each as its own distinctness pair, so a well-meant cleanup fails a test instead of quietly re-keying every verdict in existence. Also pinned: the intentional collision (two physical findings sharing (path, rule, message) share an id, because one judged verdict covers the repeated pattern), UTF-8 handling for non-ASCII paths and messages, and that a MISSING field raises while an EMPTY one hashes. The expected ids were not generated by the implementation under test. They were computed from an independent transcription of the recipe and cross-checked against the shipped apply_verdicts.finding_id before being frozen; the test keeps that outside opinion as a live check, because vectors regenerated from their own implementation agree with each other and with nothing else. Recorded in the domain contract and AGENTS.md, because it has already misled once: SARIF's partialFingerprints["ownAudit/v1"] is a DIFFERENT value — it lower-cases the message, collapses digits and spaces, keeps the full digest and appends a /ordinal suffix for repeats. It is a legacy GitHub-correlation key. The ordinal alone disqualifies it as identity: presentation order must never change an id. New fingerprints will go alongside it, never in place of it, or the migration would reset every alert's history while claiming to improve correlation. Verified: new contract tests pass under python3 and -O; apply_verdicts selftest unchanged; the full Linux-safe sweep from AGENTS.md — all 15 suites — passes. Refs PhysShell/Own.NET#266. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M --- .github/workflows/ci.yml | 2 + AGENTS.md | 1 + contracts/finding-pattern-v1.json | 98 ++++++++++++++++++ docs/fp-judge/verdict-contract.md | 35 +++++++ identity/pattern.py | 159 ++++++++++++++++++++++++++++++ identity/tests/test_pattern.py | 114 +++++++++++++++++++++ viz/apply_verdicts.py | 16 ++- 7 files changed, 422 insertions(+), 3 deletions(-) create mode 100644 contracts/finding-pattern-v1.json create mode 100644 identity/pattern.py create mode 100644 identity/tests/test_pattern.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2de75b9..86b4c74 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,7 @@ jobs: PYTHONPATH=fix python3 fix/tests/test_ai_fix.py PYTHONPATH=fix python3 fix/tests/test_own_fix.py PYTHONPATH=fix python3 fix/tests/test_orchestrate.py + PYTHONPATH=. python3 identity/tests/test_pattern.py PYTHONPATH=. python3 report/tests/test_sarif.py PYTHONPATH=. python3 report/tests/test_baseline.py PYTHONPATH=. python3 report/tests/test_rules_map.py @@ -53,6 +54,7 @@ jobs: PYTHONPATH=fix python3 -O fix/tests/test_ai_fix.py PYTHONPATH=fix python3 -O fix/tests/test_own_fix.py PYTHONPATH=fix python3 -O fix/tests/test_orchestrate.py + PYTHONPATH=. python3 -O identity/tests/test_pattern.py PYTHONPATH=. python3 -O report/tests/test_sarif.py PYTHONPATH=. python3 -O report/tests/test_baseline.py PYTHONPATH=. python3 -O report/tests/test_rules_map.py diff --git a/AGENTS.md b/AGENTS.md index 3cece7e..5f13ee3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,6 +5,7 @@ This file provides guidance to agents when working with code in this repository. - **OwnAudit is the canonical audit repo** (since 2026-07-18): the lift-out from `Own.NET/audit/` happened and the direction reversed — `Own.NET/audit/` is deprecated and Own.NET stays a pure SAST engine that emits findings. New aggregation/reporting/runtime/fix work lands here, not there. (Older docs in both repos may still state the pre-reversal boundary; `docs/collector-plan.md` is the current word.) - `src/OwnAudit.Cli` and `src/OwnAudit.Runtime` intentionally target `net8.0-windows` because runtime work depends on FlaUI/ClrMD; Linux work here is mainly the Python tooling/tests. `OwnAudit.Runtime` is the home of the ClrMD heap collector (`docs/collector-plan.md`). - Python tests are designed to run as bare scripts because `pytest` is not guaranteed in the dev shell: `PYTHONUTF8=1 python3 arch/tests/test_arch.py` or any `*/tests/test_*.py` file directly. +- **Finding identity is a frozen contract, not a helper.** `identity/pattern.py` implements `finding-pattern/v1` and `contracts/finding-pattern-v1.json` holds its canonical vectors; `viz/apply_verdicts.finding_id` is an alias for it. Every stored `fp-verdicts.json` overlay is keyed by those exact bytes, so changing the algorithm, separator, truncation or field order orphans the verdicts — and the result is an *empty report*, not an error. Do not "clean up" the hash or normalize paths/messages. The SARIF `partialFingerprints["ownAudit/v1"]` is a **different, legacy GitHub-correlation key** (normalized message, full digest, `/ordinal` suffix) and must never be used as an identity. - Full Linux-safe regression sweep: `PYTHONUTF8=1 python3 arch/tests/test_arch.py && PYTHONUTF8=1 python3 fix/tests/test_orchestrate.py && PYTHONUTF8=1 python3 fix/tests/test_own_fix.py && PYTHONUTF8=1 python3 fix/tests/test_ai_fix.py && PYTHONUTF8=1 python3 leakmine/tests/test_leakmine.py && PYTHONUTF8=1 python3 report/tests/test_baseline.py && PYTHONUTF8=1 python3 report/tests/test_sarif.py && PYTHONUTF8=1 python3 report/tests/test_rules_map.py && PYTHONUTF8=1 python3 report/tests/test_annotate.py && PYTHONUTF8=1 python3 report/tests/test_exec.py && PYTHONUTF8=1 python3 viz/tests/test_dashboard_rules.py && PYTHONUTF8=1 python3 runtime/tests/test_runtime.py && PYTHONUTF8=1 python3 runtime/tests/test_member_aware.py && PYTHONUTF8=1 python3 oracle/fixtures/test_oracle_arch.py && PYTHONUTF8=1 python3 oracle/fixtures/test_oracle_runtime.py`. - To run the collector against real STS documents, build the shipping stand first — `docs/sts-stand-build.md` (worktree build into `Setup`, `SERIALIZERSIM_SETUP`, `leaktest --hold`). - `Run-Audit.ps1` is Windows-stand oriented and drives external Own.NET worktrees; it sets `PYTHONUTF8=1`, emits SARIF into `artifacts/`, and clusters optional Infer#/Roslyn results only if their SARIF files already exist. diff --git a/contracts/finding-pattern-v1.json b/contracts/finding-pattern-v1.json new file mode 100644 index 0000000..6611fd8 --- /dev/null +++ b/contracts/finding-pattern-v1.json @@ -0,0 +1,98 @@ +{ + "contract": "finding-pattern/v1", + "owner": "OwnAudit (domain)", + "issue": "PhysShell/Own.NET#266", + "recipe": "sha1(path + \"\\u001f\" + rule + \"\\u001f\" + message).hexdigest()[:16]", + "id_length": 16, + "encoding": "utf-8", + "separator": "\u001f", + "notes": [ + "These vectors are the executable half of the contract. A change to any expected id is a BREAKING change: every stored fp-verdicts.json overlay is keyed by these bytes, and an overlay that stops matching produces an empty report, not an error.", + "Fields are joined VERBATIM. No case folding, no path-separator rewriting, no digit or whitespace collapsing. The vectors below pin each of those individually so that a well-meant normalization fails a test instead of silently re-keying every stored verdict.", + "`line` is deliberately absent from the recipe: it drifts. Two physical findings sharing (path, rule, message) share a pattern_id on purpose — one judged verdict covers the repeated pattern. Distinguishing the physical occurrences is occurrence_id's job, not this one.", + "NOT the same value as report/sarif.py's partialFingerprints['ownAudit/v1'], which normalizes the message, joins with newlines, keeps the full digest and appends an ordinal for repeats. That one is a legacy GitHub-correlation key; identity must not depend on presentation order." + ], + "vectors": [ + { + "name": "windows-style-path", + "why": "a producer that emitted backslashes keeps them; the separator is part of the identity", + "path": "src\\App\\DocumentWindow.xaml.cs", + "rule": "OWN001", + "message": "event 'AppSettings.PropertyChanged' subscribed, handler never released", + "pattern_id": "9323408c303bd2cf" + }, + { + "name": "posix-style-path", + "why": "the same file spelled with forward slashes is a DIFFERENT id — identity follows the emitted record, not a guess about the filesystem", + "path": "src/App/DocumentWindow.xaml.cs", + "rule": "OWN001", + "message": "event 'AppSettings.PropertyChanged' subscribed, handler never released", + "pattern_id": "bf5dae4e2ebff293" + }, + { + "name": "path-case-upper", + "why": "no case folding, even though Windows filesystems are case-insensitive", + "path": "Src/App/DocumentWindow.xaml.cs", + "rule": "OWN001", + "message": "event 'AppSettings.PropertyChanged' subscribed, handler never released", + "pattern_id": "8125b290fa225510" + }, + { + "name": "unicode-path-and-message", + "why": "non-ASCII paths and messages hash as UTF-8, not as escapes or a lossy encoding", + "path": "src/Документы/Окно.cs", + "rule": "OWN001", + "message": "событие 'Настройки.Изменено' подписано, обработчик не освобождён", + "pattern_id": "99ecde19649f7ae8" + }, + { + "name": "message-with-digits", + "why": "digits are NOT collapsed (the SARIF fingerprint does collapse them; this contract must not)", + "path": "src/Pool.cs", + "rule": "OWN003", + "message": "buffer of 4096 bytes rented but never returned", + "pattern_id": "f98bf75335d7458d" + }, + { + "name": "message-with-other-digits", + "why": "same sentence, different number: a different pattern, therefore a different id", + "path": "src/Pool.cs", + "rule": "OWN003", + "message": "buffer of 8192 bytes rented but never returned", + "pattern_id": "dbeece167c805d43" + }, + { + "name": "whitespace-differs", + "why": "whitespace is NOT collapsed either — the message is taken as emitted", + "path": "src/Pool.cs", + "rule": "OWN003", + "message": "buffer of 4096 bytes rented but never returned", + "pattern_id": "58ad38785c931022" + }, + { + "name": "repeated-identical", + "why": "a second physical finding with identical (path, rule, message) — the intentional collision one verdict is meant to cover", + "path": "src/App/DocumentWindow.xaml.cs", + "rule": "OWN001", + "message": "event 'AppSettings.PropertyChanged' subscribed, handler never released", + "pattern_id": "bf5dae4e2ebff293" + }, + { + "name": "empty-message", + "why": "an empty field is hashed as empty rather than rejected; only a MISSING key is an error", + "path": "src/Empty.cs", + "rule": "OWN050", + "message": "", + "pattern_id": "ab023777f5fe8ea7" + } + ], + "same_pattern": [ + ["posix-style-path", "repeated-identical"] + ], + "distinct_patterns": [ + ["windows-style-path", "posix-style-path"], + ["posix-style-path", "path-case-upper"], + ["message-with-digits", "message-with-other-digits"], + ["message-with-digits", "whitespace-differs"] + ] +} diff --git a/docs/fp-judge/verdict-contract.md b/docs/fp-judge/verdict-contract.md index 0b38a11..3130d66 100644 --- a/docs/fp-judge/verdict-contract.md +++ b/docs/fp-judge/verdict-contract.md @@ -42,6 +42,41 @@ identical. So a `finding_id` may map to **≥1 physical findings**: No ordinal / tiebreaker — that would reintroduce line-order fragility for zero gain. +### Where the recipe lives now (`finding-pattern/v1`) + +The implementation is [`identity/pattern.py`](../../identity/pattern.py) and the +canonical vectors are +[`contracts/finding-pattern-v1.json`](../../contracts/finding-pattern-v1.json). +`viz/apply_verdicts.finding_id` imports it and survives as the overlay-facing +alias; the bytes are unchanged, which is the point — every stored +`fp-verdicts.json` is keyed by them, and an overlay that stops matching +produces an **empty report rather than an error**. That failure is silent, so +it is pinned by a test (`identity/tests/test_pattern.py`), not by a comment. + +The vectors pin what is deliberately *not* normalized: path separator, path +case, digits and whitespace in the message. A future cleanup that folds any of +them fails a test instead of quietly re-keying every stored verdict. + +### `ownAudit/v1` in SARIF is a different value + +`report/sarif.py` emits `partialFingerprints["ownAudit/v1"]`. It is **not** +`finding-pattern/v1` and must not be used as one: + +| | `finding-pattern/v1` | SARIF `ownAudit/v1` | +|---|---|---| +| basis | `path \x1f rule \x1f message` | `rule \n path \n normalize(message)` | +| message | verbatim | lower-cased, digits → `#`, spaces collapsed | +| digest | SHA-1, first 16 hex | SHA-1, full | +| repeats | share an id (intentional) | disambiguated with a `/ordinal` suffix | + +`ownAudit/v1` is a **legacy GitHub-correlation key**: it keeps code-scanning +alerts stitched together across commits. Its ordinal suffix alone disqualifies +it as an identity — the value would depend on the order results happen to be +emitted in, and presentation order must never change identity. When +occurrence/pattern fingerprints are exported to SARIF they go in *alongside* +this key, never in place of it, or the migration would reset every alert's +history while claiming to improve correlation. + --- ## 2. `fp-verdicts.json` — required fields diff --git a/identity/pattern.py b/identity/pattern.py new file mode 100644 index 0000000..d6e6f7a --- /dev/null +++ b/identity/pattern.py @@ -0,0 +1,159 @@ +#!/usr/bin/env python3 +""" +`finding-pattern/v1` — the ONE implementation of Own's pattern-level finding +identity, and the domain's canonical definition of it (issue Own.NET#266). + + pattern_id = sha1( path + "\\x1f" + rule + "\\x1f" + message )[:16] + +This is the recipe `viz/apply_verdicts.finding_id` has always used +(`docs/fp-judge/verdict-contract.md` §1); it moves here unchanged so that every +producer and consumer computes identity from one place instead of from one +place each. `apply_verdicts` now imports it, and its `finding_id` name survives +as an alias — the bytes are identical, so existing overlays keep matching. + +WHY IT IS NOT "IMPROVED" HERE +----------------------------- +SHA-1 is not a security choice in this role; it is a *compatibility* one. Every +`fp-verdicts.json` overlay in existence is keyed by these exact 16 hex +characters, and the overlay's freshness guard only proves the verdicts were +judged against the same findings file — not that the ids can be recomputed +differently. Changing the algorithm, the separator, the truncation, or the +field order silently orphans every stored verdict: the overlay would simply +stop matching, and a clean report would be indistinguishable from a correct +one. Compatibility outranks a tidier hash. + +WHAT IS DELIBERATELY *NOT* NORMALIZED +------------------------------------- +`path`, `rule` and `message` are joined **verbatim**, as UTF-8: + + * no case folding — `Src\\App.cs` and `src\\app.cs` are different findings; + * no path-separator rewriting — a Windows-style and a POSIX-style spelling of + the same file are different ids, because the producer chose the spelling + and identity follows the emitted record, not a guess about the filesystem; + * no digit or whitespace collapsing in `message` — two findings whose + messages differ only by a number are different patterns. + +The `contracts/finding-pattern-v1.json` vectors pin each of those, so a future +"harmless cleanup" fails a test instead of quietly re-keying the world. + +NOT TO BE CONFUSED WITH THE SARIF FINGERPRINT +--------------------------------------------- +`report/sarif.py` emits `partialFingerprints["ownAudit/v1"]`, which is a +DIFFERENT value computed a DIFFERENT way (rule/path/normalized-message joined +with newlines, full SHA-1, plus a `/ordinal` suffix for repeats). It exists to +keep GitHub code-scanning alerts correlated across commits and it is a legacy +GitHub-correlation key — **it is not `finding-pattern/v1`**. Its ordinal suffix +alone disqualifies it: identity must not depend on presentation order. + +Line is never part of identity: it drifts, and it travels as data for locating +a finding, never for keying one. Two physical findings that share +`(path, rule, message)` share a `pattern_id` ON PURPOSE — one judged verdict +covers the repeated pattern. Telling those physical occurrences apart is the +job of `occurrence_id`, a different identity with different rules. +""" + +from __future__ import annotations + +import hashlib +import json +from pathlib import Path +from typing import Any, Mapping + +#: The contract this module implements. Bump only with a synchronized change to +#: `contracts/finding-pattern-v1.json` and every consumer that pins it. +CONTRACT = "finding-pattern/v1" + +#: Length of the hex digest kept, in characters. Part of the contract. +ID_LENGTH = 16 + +#: Unit separator: unambiguous join, cannot occur in a path/rule/message. +SEP = "\x1f" + +CONTRACT_FILE = Path(__file__).resolve().parent.parent / "contracts" / "finding-pattern-v1.json" + + +def pattern_id(path: str, rule: str, message: str) -> str: + """The `finding-pattern/v1` id for one finding's verbatim fields. + + Takes the three fields directly rather than a dict, so a caller cannot + accidentally feed a *differently shaped* record and get a plausible-looking + id from the wrong keys. + """ + key = f"{path}{SEP}{rule}{SEP}{message}" + return hashlib.sha1(key.encode("utf-8")).hexdigest()[:ID_LENGTH] + + +def pattern_id_of(finding: Mapping[str, Any]) -> str: + """`pattern_id` for a normalized finding record. + + The record must carry `path`, `rule` and `message`; a missing one is an + error rather than an empty string, because silently hashing `""` would mint + a stable-looking id for an incomplete record and merge unrelated findings + under it. + """ + missing = [k for k in ("path", "rule", "message") if k not in finding] + if missing: + raise KeyError( + f"{CONTRACT}: cannot compute pattern_id, record is missing {missing}" + ) + return pattern_id(finding["path"], finding["rule"], finding["message"]) + + +def load_vectors(path: str | Path | None = None) -> dict[str, Any]: + """The canonical vector file — the contract's executable half.""" + return json.loads(Path(path or CONTRACT_FILE).read_text(encoding="utf-8")) + + +def _selftest() -> int: + """Run the canonical vectors. Bare `python3 identity/pattern.py --selftest`.""" + fails: list[str] = [] + doc = load_vectors() + + if doc.get("contract") != CONTRACT: + fails.append(f"contract mismatch: file says {doc.get('contract')!r}, module says {CONTRACT!r}") + if doc.get("id_length") != ID_LENGTH: + fails.append(f"id_length mismatch: file {doc.get('id_length')!r} vs module {ID_LENGTH}") + + for vec in doc.get("vectors", []): + got = pattern_id(vec["path"], vec["rule"], vec["message"]) + if got != vec["pattern_id"]: + fails.append(f"{vec['name']}: got {got}, want {vec['pattern_id']}") + if len(got) != ID_LENGTH or any(c not in "0123456789abcdef" for c in got): + fails.append(f"{vec['name']}: {got!r} is not {ID_LENGTH} lower-case hex chars") + + # The grouping vectors state, as data, which pairs must (not) collide. + by_name = {v["name"]: v for v in doc.get("vectors", [])} + for a, b in doc.get("same_pattern", []): + if pattern_id_of(by_name[a]) != pattern_id_of(by_name[b]): + fails.append(f"{a} and {b} must share a pattern_id") + for a, b in doc.get("distinct_patterns", []): + if pattern_id_of(by_name[a]) == pattern_id_of(by_name[b]): + fails.append(f"{a} and {b} must NOT share a pattern_id") + + try: + pattern_id_of({"path": "a", "rule": "b"}) + except KeyError: + pass + else: + fails.append("a record missing `message` must raise, not hash an empty field") + + for f in fails: + print(f"FAIL: {f}") + n = len(doc.get("vectors", [])) + print(f"identity/pattern selftest: {'OK' if not fails else 'FAIL'} " + f"({n} vectors, {len(doc.get('same_pattern', []))} grouping pairs, " + f"{len(doc.get('distinct_patterns', []))} distinctness pairs)") + return 1 if fails else 0 + + +if __name__ == "__main__": + import sys + + if "--selftest" in sys.argv: + raise SystemExit(_selftest()) + if len(sys.argv) == 4: + print(pattern_id(sys.argv[1], sys.argv[2], sys.argv[3])) + raise SystemExit(0) + print(__doc__) + print("usage: pattern.py --selftest | pattern.py ") + raise SystemExit(2) diff --git a/identity/tests/test_pattern.py b/identity/tests/test_pattern.py new file mode 100644 index 0000000..18795c7 --- /dev/null +++ b/identity/tests/test_pattern.py @@ -0,0 +1,114 @@ +"""`finding-pattern/v1` contract tests. Bare python3 or pytest: + + PYTHONPATH=. python3 identity/tests/test_pattern.py + +Proves the identity recipe is frozen: the canonical vectors reproduce exactly, +the deliberate non-normalizations hold (path separator, path case, digits, +whitespace), the intentional pattern collision still collides, a record missing +a field raises instead of hashing an empty one, and `apply_verdicts.finding_id` +still returns the same bytes it always did. + +That last one is the compatibility guarantee in executable form: every stored +`fp-verdicts.json` is keyed by those bytes, and an overlay that stops matching +yields an empty report rather than an error — a silent failure, so it gets a +test rather than a comment. -O-safe (explicit raises, no bare assert). +""" +import hashlib +import json +import os +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) +ROOT = os.path.dirname(os.path.dirname(HERE)) +sys.path.insert(0, ROOT) + +from identity.pattern import ( # noqa: E402 + CONTRACT, ID_LENGTH, SEP, CONTRACT_FILE, load_vectors, pattern_id, pattern_id_of, +) +from viz.apply_verdicts import finding_id # noqa: E402 + +fails: list[str] = [] + + +def check(cond: bool, msg: str) -> None: + if not cond: + fails.append(msg) + + +def main() -> int: + doc = load_vectors() + vectors = doc["vectors"] + by_name = {v["name"]: v for v in vectors} + + check(doc["contract"] == CONTRACT, f"contract name drifted: {doc['contract']!r}") + check(doc["id_length"] == ID_LENGTH, f"id_length drifted: {doc['id_length']!r}") + check(doc["separator"] == SEP, "separator drifted from the unit separator") + + # 1. Every canonical vector reproduces, exactly. + for v in vectors: + got = pattern_id(v["path"], v["rule"], v["message"]) + check(got == v["pattern_id"], + f"{v['name']}: got {got}, contract says {v['pattern_id']}") + check(len(got) == ID_LENGTH and all(c in "0123456789abcdef" for c in got), + f"{v['name']}: {got!r} is not {ID_LENGTH} lower-case hex characters") + + # 2. An independent transcription of the recipe agrees. Written out here on + # purpose: if pattern.py and the vectors were ever regenerated together + # from a changed implementation, both would agree with each other and + # with nothing else. This line is the outside opinion. + for v in vectors: + ref = hashlib.sha1( + (v["path"] + "\x1f" + v["rule"] + "\x1f" + v["message"]).encode("utf-8") + ).hexdigest()[:16] + check(ref == v["pattern_id"], f"{v['name']}: recipe transcription disagrees ({ref})") + + # 3. The deliberate non-normalizations. + for a, b in doc["distinct_patterns"]: + check(pattern_id_of(by_name[a]) != pattern_id_of(by_name[b]), + f"{a} and {b} must NOT share a pattern_id — a normalization crept in") + for a, b in doc["same_pattern"]: + check(pattern_id_of(by_name[a]) == pattern_id_of(by_name[b]), + f"{a} and {b} must share a pattern_id — the repeated-pattern collision is intentional") + + # 4. A missing field raises; an EMPTY field does not. + for missing in ("path", "rule", "message"): + rec = {"path": "p", "rule": "r", "message": "m"} + del rec[missing] + try: + pattern_id_of(rec) + except KeyError: + pass + else: + fails.append(f"a record missing {missing!r} must raise, not hash an empty field") + check(pattern_id_of({"path": "src/Empty.cs", "rule": "OWN050", "message": ""}) + == by_name["empty-message"]["pattern_id"], + "an empty message must hash as empty, not raise") + + # 5. Extra keys on the record change nothing — identity reads three fields. + check(pattern_id_of({**by_name["posix-style-path"], "line": 42, "tool": "own-check"}) + == by_name["posix-style-path"]["pattern_id"], + "line/tool leaked into the identity") + + # 6. THE compatibility guarantee: the overlay-facing alias is byte-identical. + for v in vectors: + check(finding_id(v) == v["pattern_id"], + f"{v['name']}: apply_verdicts.finding_id drifted from the contract") + + # 7. The SARIF fingerprint is a DIFFERENT value and must not be mistaken for + # this one — it normalizes the message and appends an ordinal. + from report.sarif import _fingerprint # noqa: E402 + v = by_name["message-with-digits"] + check(_fingerprint(v) != v["pattern_id"], + "the SARIF ownAudit/v1 fingerprint must not equal pattern_id — it is a legacy " + "GitHub-correlation key with different rules") + + for f in fails: + print(f"FAIL: {f}") + print(f"identity/pattern: {'OK' if not fails else 'FAIL'} — {len(vectors)} vectors, " + f"{len(doc['distinct_patterns'])} distinctness pairs, " + f"{len(doc['same_pattern'])} collision pairs, alias byte-identical") + return 1 if fails else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/viz/apply_verdicts.py b/viz/apply_verdicts.py index 0e87191..9dc4f21 100644 --- a/viz/apply_verdicts.py +++ b/viz/apply_verdicts.py @@ -28,6 +28,10 @@ import os import sys +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from identity.pattern import pattern_id_of # noqa: E402 + # A false_positive at >= this confidence is dropped from the primary triage into the # counted "judged-FP" section. A LOWER-confidence FP stays visible as `uncertain` — we # only retire a CONFIDENT false positive. @@ -44,9 +48,15 @@ class StaleOverlayError(RuntimeError): def finding_id(f: dict) -> str: """Line-independent fingerprint (verdict-contract.md §1): sha1(path\\x1f rule\\x1f message)[:16]. NOT keyed on `line` (it drifts). Identical (path,rule,message) share - an id on purpose — one verdict covers a repeated pattern.""" - key = f"{f['path']}\x1f{f['rule']}\x1f{f['message']}" - return hashlib.sha1(key.encode("utf-8")).hexdigest()[:16] + an id on purpose — one verdict covers a repeated pattern. + + The recipe now lives in `identity/pattern.py` as `finding-pattern/v1`, with + canonical vectors in `contracts/finding-pattern-v1.json`; this name stays as + the overlay-facing alias. The bytes are unchanged, so every existing + `fp-verdicts.json` keeps matching — that is the whole point of moving it + rather than reimplementing it. + """ + return pattern_id_of(f) def sha256_file(path: str) -> str: From 366a0c2c2f157728f40288068c9653c7b9e24a26 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 19:58:08 +0000 Subject: [PATCH 2/2] fix(identity): make the package unshadowable and its output encoding-proof Two review findings, both reproduced before fixing. 1. AN INSTALLED `identity` PACKAGE SHADOWED OURS (Codex P2). Without `__init__.py` the directory is only a NAMESPACE portion: during the sys.path scan a namespace portion is remembered and the scan CONTINUES, so a regular package found later still wins -- inserting the repo root first does not help. `identity` is a real name on PyPI, so this is not hypothetical. Reproduced with a stand-in installed package: `ModuleNotFoundError: No module named 'identity.pattern'`, resolving to the other distribution. Adding the initializer fixes it; verified against the same stand-in. 2. THE TEST'S OWN OUTPUT COULD CRASH IT (CodeRabbit, with a correction). The claim was a missing PYTHONUTF8=1 prefix. Reading the vectors was never at risk -- every read passes `encoding="utf-8"` explicitly -- but the printed summary contained an em-dash, and under a legacy stdout encoding that raises UnicodeEncodeError. Reproduced with `PYTHONUTF8=0 LC_ALL=C`: the contract verified correctly and then the process died printing the result. A failing test that fails on its own output is worse than a failing test. Fixed at the source rather than documented around: the identity module and its test are now pure ASCII, including the docstring that the CLI branch prints. Both now pass under `PYTHONUTF8=0 LC_ALL=C`, so they no longer depend on the ambient encoding at all. The documented invocations do carry PYTHONUTF8=1, matching AGENTS.md. The CI lines deliberately do NOT: this workflow has never used the prefix on any of its sixteen suites, and adding it to only the two new lines would make them inconsistent with the rest while fixing nothing that (2) has not already fixed. If the runners' locale were ever non-UTF-8 that would be a repo-wide problem, not this slice's. Verified: both hostile scenarios now pass; the full Linux-safe sweep -- now 16 suites -- passes, plus -O. Refs PhysShell/Own.NET#266. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M --- identity/__init__.py | 6 ++++++ identity/pattern.py | 28 ++++++++++++++++------------ identity/tests/test_pattern.py | 16 ++++++++-------- 3 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 identity/__init__.py diff --git a/identity/__init__.py b/identity/__init__.py new file mode 100644 index 0000000..6fd8b8e --- /dev/null +++ b/identity/__init__.py @@ -0,0 +1,6 @@ +# a package, so an installed "identity" distribution cannot shadow ours. +# +# Without this file the directory is a NAMESPACE portion: during the sys.path +# scan a namespace portion is only remembered, and a regular package found +# LATER on the path still wins -- even though the repo root was inserted first. +# `identity` is a real name on PyPI, so this is not hypothetical (Codex, PR #55). diff --git a/identity/pattern.py b/identity/pattern.py index d6e6f7a..2f92aa2 100644 --- a/identity/pattern.py +++ b/identity/pattern.py @@ -1,22 +1,22 @@ #!/usr/bin/env python3 """ -`finding-pattern/v1` — the ONE implementation of Own's pattern-level finding +`finding-pattern/v1` - the ONE implementation of Own's pattern-level finding identity, and the domain's canonical definition of it (issue Own.NET#266). pattern_id = sha1( path + "\\x1f" + rule + "\\x1f" + message )[:16] This is the recipe `viz/apply_verdicts.finding_id` has always used -(`docs/fp-judge/verdict-contract.md` §1); it moves here unchanged so that every +(`docs/fp-judge/verdict-contract.md` section 1); it moves here unchanged so that every producer and consumer computes identity from one place instead of from one place each. `apply_verdicts` now imports it, and its `finding_id` name survives -as an alias — the bytes are identical, so existing overlays keep matching. +as an alias - the bytes are identical, so existing overlays keep matching. WHY IT IS NOT "IMPROVED" HERE ----------------------------- SHA-1 is not a security choice in this role; it is a *compatibility* one. Every `fp-verdicts.json` overlay in existence is keyed by these exact 16 hex characters, and the overlay's freshness guard only proves the verdicts were -judged against the same findings file — not that the ids can be recomputed +judged against the same findings file - not that the ids can be recomputed differently. Changing the algorithm, the separator, the truncation, or the field order silently orphans every stored verdict: the overlay would simply stop matching, and a clean report would be indistinguishable from a correct @@ -26,11 +26,11 @@ ------------------------------------- `path`, `rule` and `message` are joined **verbatim**, as UTF-8: - * no case folding — `Src\\App.cs` and `src\\app.cs` are different findings; - * no path-separator rewriting — a Windows-style and a POSIX-style spelling of + * no case folding - `Src\\App.cs` and `src\\app.cs` are different findings; + * no path-separator rewriting - a Windows-style and a POSIX-style spelling of the same file are different ids, because the producer chose the spelling and identity follows the emitted record, not a guess about the filesystem; - * no digit or whitespace collapsing in `message` — two findings whose + * no digit or whitespace collapsing in `message` - two findings whose messages differ only by a number are different patterns. The `contracts/finding-pattern-v1.json` vectors pin each of those, so a future @@ -42,12 +42,12 @@ DIFFERENT value computed a DIFFERENT way (rule/path/normalized-message joined with newlines, full SHA-1, plus a `/ordinal` suffix for repeats). It exists to keep GitHub code-scanning alerts correlated across commits and it is a legacy -GitHub-correlation key — **it is not `finding-pattern/v1`**. Its ordinal suffix +GitHub-correlation key - **it is not `finding-pattern/v1`**. Its ordinal suffix alone disqualifies it: identity must not depend on presentation order. Line is never part of identity: it drifts, and it travels as data for locating a finding, never for keying one. Two physical findings that share -`(path, rule, message)` share a `pattern_id` ON PURPOSE — one judged verdict +`(path, rule, message)` share a `pattern_id` ON PURPOSE - one judged verdict covers the repeated pattern. Telling those physical occurrences apart is the job of `occurrence_id`, a different identity with different rules. """ @@ -100,12 +100,15 @@ def pattern_id_of(finding: Mapping[str, Any]) -> str: def load_vectors(path: str | Path | None = None) -> dict[str, Any]: - """The canonical vector file — the contract's executable half.""" + """The canonical vector file - the contract's executable half.""" return json.loads(Path(path or CONTRACT_FILE).read_text(encoding="utf-8")) def _selftest() -> int: - """Run the canonical vectors. Bare `python3 identity/pattern.py --selftest`.""" + """Run the canonical vectors. + + `PYTHONUTF8=1 PYTHONPATH=. python3 identity/pattern.py --selftest` + """ fails: list[str] = [] doc = load_vectors() @@ -155,5 +158,6 @@ def _selftest() -> int: print(pattern_id(sys.argv[1], sys.argv[2], sys.argv[3])) raise SystemExit(0) print(__doc__) - print("usage: pattern.py --selftest | pattern.py ") + print("usage: PYTHONUTF8=1 PYTHONPATH=. python3 identity/pattern.py --selftest") + print(" PYTHONUTF8=1 PYTHONPATH=. python3 identity/pattern.py ") raise SystemExit(2) diff --git a/identity/tests/test_pattern.py b/identity/tests/test_pattern.py index 18795c7..9905d49 100644 --- a/identity/tests/test_pattern.py +++ b/identity/tests/test_pattern.py @@ -1,6 +1,6 @@ """`finding-pattern/v1` contract tests. Bare python3 or pytest: - PYTHONPATH=. python3 identity/tests/test_pattern.py + PYTHONUTF8=1 PYTHONPATH=. python3 identity/tests/test_pattern.py Proves the identity recipe is frozen: the canonical vectors reproduce exactly, the deliberate non-normalizations hold (path separator, path case, digits, @@ -10,7 +10,7 @@ That last one is the compatibility guarantee in executable form: every stored `fp-verdicts.json` is keyed by those bytes, and an overlay that stops matching -yields an empty report rather than an error — a silent failure, so it gets a +yields an empty report rather than an error - a silent failure, so it gets a test rather than a comment. -O-safe (explicit raises, no bare assert). """ import hashlib @@ -65,10 +65,10 @@ def main() -> int: # 3. The deliberate non-normalizations. for a, b in doc["distinct_patterns"]: check(pattern_id_of(by_name[a]) != pattern_id_of(by_name[b]), - f"{a} and {b} must NOT share a pattern_id — a normalization crept in") + f"{a} and {b} must NOT share a pattern_id - a normalization crept in") for a, b in doc["same_pattern"]: check(pattern_id_of(by_name[a]) == pattern_id_of(by_name[b]), - f"{a} and {b} must share a pattern_id — the repeated-pattern collision is intentional") + f"{a} and {b} must share a pattern_id - the repeated-pattern collision is intentional") # 4. A missing field raises; an EMPTY field does not. for missing in ("path", "rule", "message"): @@ -84,7 +84,7 @@ def main() -> int: == by_name["empty-message"]["pattern_id"], "an empty message must hash as empty, not raise") - # 5. Extra keys on the record change nothing — identity reads three fields. + # 5. Extra keys on the record change nothing - identity reads three fields. check(pattern_id_of({**by_name["posix-style-path"], "line": 42, "tool": "own-check"}) == by_name["posix-style-path"]["pattern_id"], "line/tool leaked into the identity") @@ -95,16 +95,16 @@ def main() -> int: f"{v['name']}: apply_verdicts.finding_id drifted from the contract") # 7. The SARIF fingerprint is a DIFFERENT value and must not be mistaken for - # this one — it normalizes the message and appends an ordinal. + # this one - it normalizes the message and appends an ordinal. from report.sarif import _fingerprint # noqa: E402 v = by_name["message-with-digits"] check(_fingerprint(v) != v["pattern_id"], - "the SARIF ownAudit/v1 fingerprint must not equal pattern_id — it is a legacy " + "the SARIF ownAudit/v1 fingerprint must not equal pattern_id - it is a legacy " "GitHub-correlation key with different rules") for f in fails: print(f"FAIL: {f}") - print(f"identity/pattern: {'OK' if not fails else 'FAIL'} — {len(vectors)} vectors, " + print(f"identity/pattern: {'OK' if not fails else 'FAIL'} - {len(vectors)} vectors, " f"{len(doc['distinct_patterns'])} distinctness pairs, " f"{len(doc['same_pattern'])} collision pairs, alias byte-identical") return 1 if fails else 0