fix(security): stabilize imported finding identities

packages/coding-agent/scripts/security-compare.ts

@@ -1,10 +1,17 @@
 #!/usr/bin/env bun
 import * as path from "node:path";
-import { compareSecurityProducers, parseSecurityScanBundle } from "../src/security";
+import { isEnoent } from "@oh-my-pi/pi-utils";
+import { compareSecurityProducers, importCodexSecurityBundle, parseSecurityScanBundle } from "../src/security";
 
 async function readBundle(directory: string) {
 	const root = path.resolve(directory);
-	const scan = JSON.parse(await Bun.file(path.join(root, "scan.json")).text()) as unknown;
+	let scan: unknown;
+	try {
+		scan = JSON.parse(await Bun.file(path.join(root, "scan.json")).text()) as unknown;
+	} catch (error) {
+		if (!isEnoent(error)) throw error;
+		return importCodexSecurityBundle(root, { repositoryRoot: root });
+	}
 	const findings = JSON.parse(await Bun.file(path.join(root, "findings.json")).text()) as unknown;
 	const report = await Bun.file(path.join(root, "report.md"))
 		.text()

packages/coding-agent/src/security/auth.ts

@@ -8,9 +8,24 @@ export interface ExactSecurityOAuthOptions {
 	account: SecurityAccountRef;
 }
 
-function assertIdentityMatches(account: SecurityAccountRef, resolvedAccountId: string | undefined): void {
-	if (account.accountId !== undefined && account.accountId !== resolvedAccountId) {
-		throw new Error("Security scan account mismatch: the pinned workspace identity changed during authentication");
+function assertIdentityMatches(
+	account: SecurityAccountRef,
+	resolution: {
+		credentialId?: number;
+		accountId?: string;
+		email?: string;
+		orgId?: string;
+		orgName?: string;
+	},
+): void {
+	if (
+		account.credentialId !== resolution.credentialId ||
+		(account.accountId !== undefined && account.accountId !== resolution.accountId) ||
+		(account.email !== undefined && account.email !== resolution.email) ||
+		(account.organizationId !== undefined && account.organizationId !== resolution.orgId) ||
+		(account.organizationName !== undefined && account.organizationName !== resolution.orgName)
+	) {
+		throw new Error("Security scan authentication identity mismatch");
 	}
 }
 
@@ -27,9 +42,7 @@ export function createExactSecurityOAuthResolver(
 	const { account, authStorage } = options;
 	return model => {
 		if (model.provider !== account.provider) {
-			throw new Error(
-				`Security scan model provider ${model.provider} does not match pinned account provider ${account.provider}`,
-			);
+			throw new Error("Security scan authentication provider mismatch");
 		}
 		const resolver: ApiKeyResolver = async context => {
 			if (context.lastChance) return undefined;
@@ -40,10 +53,10 @@ export function createExactSecurityOAuthResolver(
 			if (!resolution) {
 				throw new Error("The pinned security OAuth credential is unavailable");
 			}
+			assertIdentityMatches(account, resolution);
 			if (!resolution.ok) {
 				throw new Error("The pinned security OAuth credential could not be resolved");
 			}
-			assertIdentityMatches(account, resolution.accountId);
 			return resolution.accessToken;
 		};
 		return resolver;

packages/coding-agent/src/security/contracts/ids.ts

@@ -24,6 +24,24 @@ function normalizeFingerprintPath(value: string): string {
 	return value.replaceAll("\\", "/").replace(/^\.\//, "");
 }
 
+function compareNormalizedLocationValues(
+	left: string | number | undefined,
+	right: string | number | undefined,
+): number {
+	if (left === right) return 0;
+	if (left === undefined) return -1;
+	if (right === undefined) return 1;
+	if (typeof left === "number" && typeof right === "number") {
+		const leftNaN = Number.isNaN(left);
+		const rightNaN = Number.isNaN(right);
+		if (leftNaN || rightNaN) return leftNaN ? (rightNaN ? 0 : -1) : 1;
+		return left - right;
+	}
+	return String(left) < String(right) ? -1 : 1;
+}
+
+const NORMALIZED_LOCATION_SORT_KEYS = ["path", "startLine", "endLine", "startColumn", "endColumn", "role"] as const;
+
 function normalizedLocations(
 	locations: readonly SecurityLocation[],
 ): Array<Record<string, string | number | undefined>> {
@@ -37,9 +55,11 @@ function normalizedLocations(
 			role: location.role,
 		}))
 		.sort((left, right) => {
-			const byPath = String(left.path).localeCompare(String(right.path));
-			if (byPath !== 0) return byPath;
-			return Number(left.startLine) - Number(right.startLine);
+			for (const key of NORMALIZED_LOCATION_SORT_KEYS) {
+				const comparison = compareNormalizedLocationValues(left[key], right[key]);
+				if (comparison !== 0) return comparison;
+			}
+			return 0;
 		});
 }
 

packages/coding-agent/src/security/importers/codex-security.ts

@@ -123,6 +123,22 @@ function locationsForFinding(finding: CodexFinding): SecurityLocation[] {
 	}
 	return locations.length > 0 ? locations : [{ path: "unknown", startLine: 1, role: "unknown" }];
 }
+const canonicalValidationStatuses: Record<string, true> = {
+	unvalidated: true,
+	validated: true,
+	rejected: true,
+	partial: true,
+	error: true,
+};
+
+function validationStatus(
+	validation: Record<string, unknown> | null | undefined,
+): SecurityFinding["validation"]["status"] {
+	const status = validation?.status;
+	return typeof status === "string" && canonicalValidationStatuses[status] === true
+		? (status as SecurityFinding["validation"]["status"])
+		: "unvalidated";
+}
 
 function mapCoverage(document: CodexCoverageDocument): SecurityCoverage {
 	const allowedModes = new Set(["repository", "scoped_path", "diff", "working_tree", "deep_repository"]);
@@ -205,11 +221,18 @@ export async function importCodexSecurityBundle(
 	for (const source of findingsDocument.findings ?? []) {
 		const ruleId = source.ruleId || "codex-security.unknown";
 		const category = source.taxonomy?.category || ruleId.split(/[./-]/)[0] || "security";
+		const hasSourceLocations = (source.locations ?? []).some(
+			location => typeof location.path === "string" && typeof location.startLine === "number",
+		);
 		const locations = locationsForFinding(source);
+		const anchor =
+			source.identity?.anchor ||
+			source.fingerprints?.primary ||
+			(!hasSourceLocations ? source.findingId : undefined);
 		const fingerprint = createSecurityFindingFingerprint({
 			ruleId,
 			category,
-			anchor: source.identity?.anchor,
+			anchor,
 			locations,
 		});
 		const evidence: SecurityEvidence[] = (source.codeEvidence ?? []).map((item, index) => {
@@ -272,13 +295,13 @@ export async function importCodexSecurityBundle(
 			],
 			evidence,
 			validation: {
-				status: source.validation ? "validated" : "unvalidated",
+				status: validationStatus(source.validation),
 				evidenceIds: [],
 			},
 			disposition: { status: "open" },
 			provenance,
 		};
-		if (source.identity?.anchor !== undefined) finding.anchor = source.identity.anchor;
+		if (anchor !== undefined) finding.anchor = anchor;
 		if (source.severity?.score !== undefined) finding.severity.score = source.severity.score;
 		if (source.severity?.scoringSystem !== undefined) finding.severity.scoringSystem = source.severity.scoringSystem;
 		if (source.severity?.vector !== undefined) finding.severity.vector = source.severity.vector;

packages/coding-agent/src/security/importers/sarif.ts

@@ -10,6 +10,7 @@ import type {
 	SecuritySeverityLevel,
 } from "../contracts";
 import {
+	canonicalSecurityJson,
 	createSecurityFindingFingerprint,
 	createSecurityFindingId,
 	createSecurityOccurrenceId,
@@ -119,6 +120,57 @@ function tagsForRule(rule: SarifRule | undefined): string[] {
 	const tags = rule?.properties?.tags;
 	return Array.isArray(tags) ? tags.filter((tag): tag is string => typeof tag === "string") : [];
 }
+function selectFirstVendorFingerprint(vendorFingerprints: Record<string, string>): string | undefined {
+	for (const [, value] of Object.entries(vendorFingerprints).sort(([left], [right]) =>
+		left < right ? -1 : left > right ? 1 : 0,
+	)) {
+		if (value) return value;
+	}
+	return undefined;
+}
+
+function semanticResultAnchor(
+	ruleId: string,
+	category: string,
+	message: string,
+	locations: readonly SecurityLocation[],
+): string {
+	return `sarif-result/v1:sha256:${securitySha256(
+		canonicalSecurityJson({
+			ruleId,
+			category,
+			message,
+			locations,
+		}),
+	)}`;
+}
+const canonicalValidationStatuses: Record<string, true> = {
+	unvalidated: true,
+	validated: true,
+	rejected: true,
+	partial: true,
+	error: true,
+};
+
+const canonicalDispositionStatuses: Record<string, true> = {
+	open: true,
+	false_positive: true,
+	accepted_risk: true,
+	fixed: true,
+	wont_fix: true,
+};
+
+function importedValidationStatus(value: unknown): SecurityFinding["validation"]["status"] {
+	return typeof value === "string" && canonicalValidationStatuses[value] === true
+		? (value as SecurityFinding["validation"]["status"])
+		: "unvalidated";
+}
+
+function importedDispositionStatus(value: unknown): SecurityFinding["disposition"]["status"] {
+	return typeof value === "string" && canonicalDispositionStatuses[value] === true
+		? (value as SecurityFinding["disposition"]["status"])
+		: "open";
+}
 
 export async function importSarif(input: unknown, options: SarifImportOptions): Promise<SecurityScanBundle> {
 	const sarif = input as SarifLog;
@@ -129,6 +181,7 @@ export async function importSarif(input: unknown, options: SarifImportOptions):
 	const scanId = options.createScanId?.() ?? createSecurityScanId();
 	const createdAt = options.createdAt ?? new Date().toISOString();
 	const findings: SecurityFinding[] = [];
+	const seenFingerprints = new Set<string>();
 	let producerName = "SARIF importer";
 	let producerVersion: string | undefined;
 
@@ -145,17 +198,21 @@ export async function importSarif(input: unknown, options: SarifImportOptions):
 				...stringRecord(result.fingerprints),
 				...stringRecord(result.partialFingerprints),
 			};
-			const firstVendorFingerprint = Object.values(vendorFingerprints)[0];
+			const firstVendorFingerprint = selectFirstVendorFingerprint(vendorFingerprints);
 			const category =
 				typeof result.properties?.category === "string"
 					? result.properties.category
 					: ruleId.split(/[./-]/)[0] || "security";
+			const message = result.message?.text ?? result.message?.markdown ?? rule?.shortDescription?.text ?? ruleId;
+			const anchor = firstVendorFingerprint ?? semanticResultAnchor(ruleId, category, message, locations);
 			const fingerprint = createSecurityFindingFingerprint({
 				ruleId,
 				category,
-				anchor: firstVendorFingerprint,
+				anchor,
 				locations,
 			});
+			if (seenFingerprints.has(fingerprint)) continue;
+			seenFingerprints.add(fingerprint);
 			const tags = tagsForRule(rule);
 			const provenance: SecurityProvenance = {
 				producer: { kind: "sarif-import", name: producerName },
@@ -165,7 +222,6 @@ export async function importSarif(input: unknown, options: SarifImportOptions):
 			};
 			if (producerVersion !== undefined) provenance.producer.version = producerVersion;
 			if (options.sourcePath) provenance.metadata = { sourcePath: options.sourcePath };
-			const message = result.message?.text ?? result.message?.markdown ?? rule?.shortDescription?.text ?? ruleId;
 			const finding: SecurityFinding = {
 				id: createSecurityFindingId(fingerprint),
 				scanId,
@@ -182,11 +238,11 @@ export async function importSarif(input: unknown, options: SarifImportOptions):
 				},
 				occurrences: [{ id: createSecurityOccurrenceId(fingerprint, locations), locations, evidenceIds: [] }],
 				evidence: [],
-				validation: { status: "unvalidated", evidenceIds: [] },
-				disposition: { status: "open" },
+				validation: { status: importedValidationStatus(result.properties?.validation), evidenceIds: [] },
+				disposition: { status: importedDispositionStatus(result.properties?.disposition) },
 				provenance,
 			};
-			if (firstVendorFingerprint !== undefined) finding.anchor = firstVendorFingerprint;
+			finding.anchor = anchor;
 			const score = Number(result.properties?.["security-severity"]);
 			if (Number.isFinite(score)) finding.severity.score = score;
 			findings.push(finding);

packages/coding-agent/src/security/sarif.ts

@@ -1,3 +1,4 @@
+import { pathToFileURL } from "node:url";
 import type { SecurityFinding, SecurityScanBundle } from "./contracts";
 
 function sarifLevel(finding: SecurityFinding): "error" | "warning" | "note" | "none" {
@@ -69,7 +70,7 @@ export function exportSecurityBundleToSarif(bundle: SecurityScanBundle): Record<
 					},
 				})),
 				originalUriBaseIds: {
-					"%SRCROOT%": { uri: `file://${bundle.scan.target.repositoryRoot.replaceAll("\\", "/")}/` },
+					"%SRCROOT%": { uri: pathToFileURL(bundle.scan.target.repositoryRoot).href.replace(/\/?$/, "/") },
 				},
 			},
 		],

packages/coding-agent/test/security/auth.test.ts

@@ -32,6 +32,60 @@ describe("exact security OAuth resolver", () => {
 		expect(getOAuthAccessByCredentialId.mock.calls.map(call => call[1])).toEqual([42, 42]);
 	});
 
+	test("rejects a model whose provider crosses the pinned OAuth boundary", async () => {
+		const getOAuthAccessByCredentialId = vi.fn(async () => ({
+			ok: true as const,
+			accessToken: "must-not-be-requested",
+			credentialId: 42,
+			accountId: "workspace-a",
+		}));
+		const authStorage = { getOAuthAccessByCredentialId } as unknown as AuthStorage;
+		const resolver = createExactSecurityOAuthResolver({
+			authStorage,
+			account: { provider: "openai-codex", credentialId: 42, accountId: "workspace-a" },
+		});
+		const wrongProviderModel = { ...model(), provider: "anthropic" } as unknown as Parameters<typeof resolver>[0];
+		expect(() => resolver(wrongProviderModel)).toThrow("provider mismatch");
+		expect(getOAuthAccessByCredentialId).not.toHaveBeenCalled();
+	});
+
+	test("fails closed when any durable account identity changes", async () => {
+		const account = {
+			provider: "openai-codex",
+			credentialId: 42,
+			accountId: "workspace-a",
+			email: "owner@example.com",
+			organizationId: "org-a",
+			organizationName: "Workspace A",
+		};
+		const resolved = {
+			credentialId: 42,
+			accountId: "workspace-a",
+			email: "owner@example.com",
+			orgId: "org-a",
+			orgName: "Workspace A",
+		};
+		for (const mismatch of [
+			{ credentialId: 99 },
+			{ accountId: "workspace-b" },
+			{ email: "other@example.com" },
+			{ orgId: "org-b" },
+			{ orgName: "Workspace B" },
+		]) {
+			const authStorage = {
+				getOAuthAccessByCredentialId: async () => ({
+					ok: true as const,
+					accessToken: "token",
+					...resolved,
+					...mismatch,
+				}),
+			} as unknown as AuthStorage;
+			const resolver = createExactSecurityOAuthResolver({ authStorage, account });
+			const exact = resolver(model()) as ApiKeyResolver;
+			await expect(exact({ lastChance: false, error: undefined })).rejects.toThrow("identity mismatch");
+		}
+	});
+
 	test("fails closed when the refreshed row loses its workspace identity", async () => {
 		const authStorage = {
 			getOAuthAccessByCredentialId: async () => ({
@@ -53,8 +107,8 @@ describe("exact security OAuth resolver", () => {
 			caught = error;
 		}
 		expect(caught).toBeInstanceOf(Error);
-		if (!(caught instanceof Error)) throw new Error("expected account mismatch");
-		expect(caught.message).toContain("account mismatch");
+		if (!(caught instanceof Error)) throw new Error("expected identity mismatch");
+		expect(caught.message).toContain("identity mismatch");
 		expect(caught.message).not.toContain("workspace-a");
 		expect(caught.message).not.toContain("undefined");
 	});

packages/coding-agent/test/security/contracts.test.ts

@@ -66,6 +66,29 @@ describe("security contracts", () => {
 		expect(createSecurityFindingId(first)).toBe(createSecurityFindingId(second));
 	});
 
+	test("finding fingerprints are stable across every location ordering", () => {
+		const locations = [
+			{ path: "src/entry.ts", startLine: 4, endLine: 8, startColumn: 2, endColumn: 4, role: "source" },
+			{ path: "src/entry.ts", startLine: 4, endLine: 8, startColumn: 2, endColumn: 4, role: "sink" },
+			{ path: "src/entry.ts", startLine: 4, endLine: 9, startColumn: 1, endColumn: 3, role: "propagation" },
+			{ path: "src/entry.ts", startLine: 4, endLine: 10, startColumn: 1, endColumn: 3, role: "source" },
+		] as const;
+		const baseline = createSecurityFindingFingerprint({
+			ruleId: "fixture.rule",
+			category: "fixture",
+			locations,
+		});
+		for (const ordered of [locations.toReversed(), [locations[2], locations[0], locations[3], locations[1]]]) {
+			expect(
+				createSecurityFindingFingerprint({
+					ruleId: "fixture.rule",
+					category: "fixture",
+					locations: ordered,
+				}),
+			).toBe(baseline);
+		}
+	});
+
 	test("scan IDs remain OMP-owned", () => {
 		expect(createSecurityScanId(() => "018f0000-0000-7000-8000-000000000001")).toBe(
 			"secscan_018f0000000070008000000000000001",

packages/coding-agent/test/security/importers-store.test.ts

@@ -2,7 +2,8 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test";
 import * as fs from "node:fs/promises";
 import * as os from "node:os";
 import * as path from "node:path";
-import { importCodexSecurityBundle, importSarifFile, SecurityStore } from "../../src/security";
+import { $ } from "bun";
+import { importCodexSecurityBundle, importSarif, importSarifFile, SecurityStore } from "../../src/security";
 
 const FIXTURE_ROOT = path.join(import.meta.dir, "..", "fixtures", "security");
 let temporaryRoot = "";
@@ -41,6 +42,37 @@ describe("security importers and store", () => {
 		expect(sarif.scan.producer.kind).toBe("sarif-import");
 	});
 
+	test("resolves one canonical store for a nested repository cwd", async () => {
+		const nestedCwd = path.join(repositoryRoot, "packages", "app");
+		await fs.mkdir(nestedCwd, { recursive: true });
+		const initialized = await $`git init --initial-branch=main`.cwd(repositoryRoot).quiet().nothrow();
+		if (initialized.exitCode !== 0) throw new Error("git init failed");
+		const store = await SecurityStore.openForCwd(nestedCwd, { stateRoot: path.join(temporaryRoot, "state") });
+		expect(store.repositoryRoot).toBe(await fs.realpath(repositoryRoot));
+	});
+
+	test("locationless SARIF keeps distinct results while deduplicating repeats", async () => {
+		const input = {
+			version: "2.1.0",
+			runs: [
+				{
+					tool: { driver: { name: "Fixture scanner" } },
+					results: [
+						{ ruleId: "fixture.rule", message: { text: "first result" } },
+						{ ruleId: "fixture.rule", message: { text: "second result" } },
+						{ ruleId: "fixture.rule", message: { text: "second result" } },
+					],
+				},
+			],
+		};
+		const bundle = await importSarif(input, {
+			repositoryRoot,
+			createScanId: () => "secscan_locationless",
+		});
+		expect(bundle.findings.map(finding => finding.summary)).toEqual(["first result", "second result"]);
+		expect(new Set(bundle.findings.map(finding => finding.id)).size).toBe(2);
+	});
+
 	test("serializes concurrent index updates without losing scans", async () => {
 		const store = await SecurityStore.open(repositoryRoot, { stateRoot: path.join(temporaryRoot, "state") });
 		const bundles = await Promise.all(

packages/coding-agent/test/security/seeded-fixture.test.ts

@@ -15,9 +15,19 @@ describe("security seeded validation repository", () => {
 		expect(manifest.schemaVersion).toBe(1);
 		expect(manifest.nonProduction).toBeTrue();
 		expect(manifest.seeds).toHaveLength(8);
+		const expectedClasses: Record<string, string> = {
+			"command-injection": "command-injection",
+			"path-traversal": "path-traversal",
+			"sql-injection": "sql-injection",
+			ssrf: "ssrf",
+			"authorization-bypass": "authorization",
+			"unsafe-deserialization": "unsafe-deserialization",
+			"fake-secret": "hard-coded-secret",
+			"safe-lookalike": "path-traversal",
+		};
 		for (const seed of manifest.seeds) {
-			expect(seed.id.length).toBeGreaterThan(0);
-			expect(seed.expectedClass.length).toBeGreaterThan(0);
+			expect(seed.id in expectedClasses).toBeTrue();
+			expect(expectedClasses[seed.id]).toBe(seed.expectedClass);
 			expect(["finding", "no-finding"]).toContain(seed.expectedDisposition);
 			expect(await Bun.file(path.join(ROOT, seed.path)).exists()).toBeTrue();
 		}