fix(native): keep signature failure evidence instead of collapsing it (#1084)
A regular CodeResources file replaced by a directory or FIFO before open was reported absent. That hid the replacement; it is now unreadable with reason changed. A failed anchor apple probe kept only an exit code. The caller-visible limitation now includes the probe's diagnostic. Definitive codesign rejections are a phrase list, including unrecognized bundle format, instead of one growing regular expression. Unlisted nonzero diagnostics stay unknown. Co-authored-by: Cursor Agent <[email protected]>
This commit is contained in:
1 parent
1325bf936f
commit
d46449e54b
5 files changed
+97
-12
No files matched your search
@@ -44,6 +44,33 @@ const operationalReason = (reason: string): boolean =>
|
||||
reason,
|
||||
);
|
||||
|
||||
/**
|
||||
* Definitive codesign rejections. A nonzero exit whose text is not in this
|
||||
* list, and is not an operational failure, stays `unknown` so an unrecognized
|
||||
* diagnostic is not reported as a broken signature.
|
||||
*/
|
||||
const INVALID_SIGNATURE_REASONS = [
|
||||
"a resource envelope is obsolete",
|
||||
"a sealed resource is missing or invalid",
|
||||
"bundle format unrecognized, invalid, or unsuitable",
|
||||
"code or signature modified",
|
||||
"file added:",
|
||||
"file modified:",
|
||||
"invalid or unsupported format",
|
||||
"invalid signature",
|
||||
"resource envelope is obsolete",
|
||||
"resource fork, Finder information, or similar detritus not allowed",
|
||||
"signature is invalid",
|
||||
"unsealed contents present",
|
||||
] as const;
|
||||
|
||||
const invalidSignatureReason = (reason: string): boolean => {
|
||||
const text = reason.toLowerCase();
|
||||
return INVALID_SIGNATURE_REASONS.some((phrase) =>
|
||||
text.startsWith(phrase.toLowerCase()),
|
||||
);
|
||||
};
|
||||
|
||||
/** Operational diagnostics can coexist with an independently proven invalid component. */
|
||||
export const codesignOperationalFailure = (
|
||||
capture: NativeCommandCapture,
|
||||
@@ -61,14 +88,7 @@ const verificationStatus = (
|
||||
reason,
|
||||
),
|
||||
);
|
||||
if (
|
||||
reasons.some((reason) =>
|
||||
/^(?:a sealed resource is missing or invalid|code or signature modified|invalid signature|invalid or unsupported format|resource envelope is obsolete|signature is invalid|unsealed contents present|file modified:|file added:|a resource envelope is obsolete|resource fork, Finder information, or similar detritus not allowed)/iu.test(
|
||||
reason,
|
||||
),
|
||||
)
|
||||
)
|
||||
return "invalid";
|
||||
if (reasons.some(invalidSignatureReason)) return "invalid";
|
||||
if (unsignedReason && !unsigned) return "invalid";
|
||||
if (reasons.some(operationalReason)) return "unknown";
|
||||
if (unsignedReason) return "unsigned";
|
||||
|
||||
@@ -23,6 +23,7 @@ import type { NativeObservation } from "./NativeObservation.js";
|
||||
import {
|
||||
signatureVerification,
|
||||
codesignOperationalFailure,
|
||||
codesignReasons,
|
||||
} from "./CodesignVerification.js";
|
||||
import { parseCodeSignature } from "./parsers/codesign.js";
|
||||
import { parseSignatureEntitlements } from "./NativeSignatureEntitlements.js";
|
||||
@@ -119,11 +120,18 @@ export const inspectNativeSignature = async (options: {
|
||||
if (!verified.ok) return verified;
|
||||
const captures = [display, requirements, entitlements, verified.value];
|
||||
let appleOrigin = false;
|
||||
let appleOriginFailure: string | undefined;
|
||||
if (!unsigned && (parsed.code_directory?.platform_identifier ?? 0) !== 0) {
|
||||
const anchor = await capture(["--verify", "-R=anchor apple", code], signal);
|
||||
if (!anchor.ok) return anchor;
|
||||
captures.push(anchor.value);
|
||||
appleOrigin = anchor.value.exitCode === 0;
|
||||
if (!appleOrigin) {
|
||||
const reason = codesignReasons(anchor.value, code).find(
|
||||
(line) => line.trim().length > 0,
|
||||
);
|
||||
appleOriginFailure = reason ?? `codesign exited ${anchor.value.exitCode}`;
|
||||
}
|
||||
}
|
||||
const slices = await inspectSignatureSlices(
|
||||
target,
|
||||
@@ -152,6 +160,8 @@ export const inspectNativeSignature = async (options: {
|
||||
limitations.push(
|
||||
`Entitlements: ${omittedPrototypeKeysLimitation(entitlementValue.omittedPrototypeKeys)}`,
|
||||
);
|
||||
if (appleOriginFailure !== undefined)
|
||||
limitations.push(`Apple-origin check failed: ${appleOriginFailure}`);
|
||||
if (verification.status === "unknown")
|
||||
limitations.push(
|
||||
"Signature verification is inconclusive (permission, I/O or unrecognized diagnostic); this is not a proven broken signature.",
|
||||
|
||||
@@ -89,7 +89,16 @@ export const stapledTicket = async (
|
||||
);
|
||||
try {
|
||||
const opened = await handle.stat();
|
||||
if (!opened.isFile()) return absent;
|
||||
// The path was a regular file at lstat. A directory or FIFO now means
|
||||
// the ticket was replaced; absence would hide that change.
|
||||
if (!opened.isFile())
|
||||
return {
|
||||
status: "unreadable",
|
||||
path: relative,
|
||||
sha256: null,
|
||||
size: null,
|
||||
reason: "changed",
|
||||
};
|
||||
// If the path was replaced between lstat and open, or mutated while
|
||||
// hashing, the digest would describe the wrong bytes: dev and ino catch
|
||||
// replacement, while size and mtime catch same-inode writes. Report any
|
||||
|
||||
@@ -43,6 +43,9 @@ it("keeps operational, unsigned, definite invalid and unrecognized failures dist
|
||||
expect(status("permission denied")).toBe("unknown");
|
||||
expect(status("I/O error")).toBe("unknown");
|
||||
expect(status("invalid signature")).toBe("invalid");
|
||||
expect(status("bundle format unrecognized, invalid, or unsuitable")).toBe(
|
||||
"invalid",
|
||||
);
|
||||
expect(
|
||||
status(
|
||||
"resource fork, Finder information, or similar detritus not allowed",
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { chmod, lstat, mkdir, writeFile } from "node:fs/promises";
|
||||
import { chmod, lstat, mkdir, rm, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
@@ -244,6 +244,16 @@ describe("native signature posture boundaries", () => {
|
||||
expect(result.provenance.map(({ command }) => command.at(-2))).toContain(
|
||||
"-R=anchor apple",
|
||||
);
|
||||
if (anchorExit === 0)
|
||||
expect(
|
||||
result.limitations.some((line) =>
|
||||
line.startsWith("Apple-origin check failed"),
|
||||
),
|
||||
).toBe(false);
|
||||
else
|
||||
expect(result.limitations).toContain(
|
||||
"Apple-origin check failed: code failed to satisfy specified code requirement(s)",
|
||||
);
|
||||
return Object.fromEntries(
|
||||
result.security_facets.map(({ facet, state }) => [facet, state]),
|
||||
);
|
||||
@@ -495,6 +505,28 @@ describe("signature architecture coverage", () => {
|
||||
});
|
||||
|
||||
describe("signature target version binding", () => {
|
||||
it("reports a regular ticket replaced by a directory as changed", async () => {
|
||||
const { app } = await fixtureApp(notarizationTicketFixture());
|
||||
const ticket = await stapledTicket(
|
||||
{ sourcePath: app, bundleInfoPlist: join(app, "Contents/Info.plist") },
|
||||
undefined,
|
||||
async (path, phase) => {
|
||||
const metadata = await lstat(path);
|
||||
if (phase === "before") {
|
||||
await rm(path);
|
||||
await mkdir(path);
|
||||
}
|
||||
return metadata;
|
||||
},
|
||||
);
|
||||
expect(ticket).toMatchObject({
|
||||
status: "unreadable",
|
||||
path: "Contents/CodeResources",
|
||||
reason: "changed",
|
||||
sha256: null,
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["EACCES", "EPERM", "EIO"])(
|
||||
"preserves final ticket-path failure %s instead of inventing mutation",
|
||||
async (code) => {
|
||||
@@ -612,8 +644,19 @@ class PlatformRunner extends FixtureRunner {
|
||||
override async run(tool: string, arguments_: readonly string[]) {
|
||||
const result = await super.run(tool, arguments_);
|
||||
if (!result.ok || tool !== "codesign") return result;
|
||||
if (arguments_.includes("-R=anchor apple"))
|
||||
return ok({ ...result.value, exitCode: this.anchorExit });
|
||||
if (arguments_.includes("-R=anchor apple")) {
|
||||
const target = arguments_.at(-1) ?? "";
|
||||
const stderr =
|
||||
this.anchorExit === 0
|
||||
? ""
|
||||
: `${target}: code failed to satisfy specified code requirement(s)\n`;
|
||||
return ok({
|
||||
...result.value,
|
||||
exitCode: this.anchorExit,
|
||||
stderr,
|
||||
stderrBytes: Buffer.byteLength(stderr),
|
||||
});
|
||||
}
|
||||
if (!arguments_.includes("--verbose=4")) return result;
|
||||
const stderr = `${result.value.stderr}Platform identifier=26\n`;
|
||||
return ok({
|
||||
|
||||
Reference in new issue
Block a user