diff --git a/src/lab/public/signature.ts b/src/lab/public/signature.ts index c88c903a0d..802ea10e82 100644 --- a/src/lab/public/signature.ts +++ b/src/lab/public/signature.ts @@ -52,6 +52,22 @@ function publisherForPrivateKey(privateKeyPem: string): PublicPublisherV1 { }; } +/** + * The bounded errno-shaped code from a failed ACL harden, or null when the cause + * carries none. + * + * Only the code is allowed into the message. `hardenSecretPath` already sanitizes + * its own diagnostic prose, but this error is what reaches a CI log, so what + * crosses that boundary is re-checked here rather than trusted: an errno code has + * no separator, no lowercase and a bounded length, and therefore cannot carry the + * key pathname or the username component inside it. + */ +function aclFailureCode(error: unknown): string | null { + if (!(error instanceof Error) || !("code" in error)) return null; + const code = (error as NodeJS.ErrnoException).code; + return typeof code === "string" && /^[A-Z][A-Z0-9_]{1,30}$/.test(code) ? code : null; +} + function requirePublisherKeyAcl(path: string, timeoutMemoKey = path): void { privateRegularFileSize(path, PRIVATE_KEY_FILE_OPTIONS); let hardened: { ok: boolean }; @@ -63,9 +79,17 @@ function requirePublisherKeyAcl(path: string, timeoutMemoKey = path): void { hardened = { ok: false }; } if (!hardened.ok) { + // Name the cause in the message, not only on `cause`. Every harden failure + // reaches a CI log as this one string, and the three that occur there need + // different fixes: ETIMEDOUT is the budget, EACLIDENTITY is the effective-SID + // lookup, EICACLS is icacls refusing the path. A message identical across all + // three cannot be acted on without a Windows box to re-run it under (#2152). + const code = aclFailureCode(hardeningError); const failure = new PublicEvidenceValidationError( "public_publisher_key_unsafe", - "public publisher key ACL hardening did not complete", + code + ? `public publisher key ACL hardening did not complete (${code})` + : "public publisher key ACL hardening did not complete", ); if (hardeningError !== undefined) { (failure as Error & { cause?: unknown }).cause = hardeningError; diff --git a/tests/lab-public-security-regressions.test.ts b/tests/lab-public-security-regressions.test.ts index ee86428655..4c45cc28c3 100644 --- a/tests/lab-public-security-regressions.test.ts +++ b/tests/lab-public-security-regressions.test.ts @@ -15,11 +15,13 @@ import { setIcaclsRunnerForTests, setPlatformForTests, } from "../src/lib/windows-secret-acl"; +import { setWindowsPrincipalRunnerForTests } from "../src/lib/windows-user-principal"; const roots: string[] = []; afterEach(() => { setIcaclsRunnerForTests(null); + setWindowsPrincipalRunnerForTests(null); setPlatformForTests(null); resetHardenedStateForTests(); for (const root of roots.splice(0)) { @@ -186,3 +188,62 @@ test("publisher key ACL failures preserve their underlying cause", () => { expect(caught).toBeInstanceOf(Error); expect((caught as Error & { cause?: unknown }).cause).toBeInstanceOf(Error); }); + +function publisherKeyAclFailureMessage(prefix: string): string { + const home = configDir(prefix); + try { + getOrCreatePublicPublisher(home); + } catch (error) { + return (error as Error).message; + } + throw new Error("expected required publisher key ACL hardening to fail"); +} + +// #2152: on the Windows CI leg every publisher-key harden failure arrived as one +// fixed string. The three causes that occur there need different fixes -- the +// budget, the effective-SID lookup, or icacls itself -- and the code lived only on +// `cause`, which the reporter does not print. Diagnosing it needed a Windows box. +test("a required publisher key ACL timeout names ETIMEDOUT in its message", () => { + resetHardenedStateForTests(); + setPlatformForTests("win32"); + setIcaclsRunnerForTests(() => ({ success: false, exitCode: null, timedOut: true, stdout: "" })); + + expect(publisherKeyAclFailureMessage("ocx-cl10-acl-code-timeout-")).toContain("ETIMEDOUT"); +}); + +test("a required publisher key icacls refusal names EICACLS in its message", () => { + resetHardenedStateForTests(); + setPlatformForTests("win32"); + setIcaclsRunnerForTests(() => ({ success: false, exitCode: 5, timedOut: false, stdout: "" })); + + expect(publisherKeyAclFailureMessage("ocx-cl10-acl-code-icacls-")).toContain("EICACLS"); +}); + +// The identity code is raised by windows-user-principal, one module further out +// than the icacls runner, so this also pins that it survives the hand-off. +test("a required publisher key SID lookup failure names EACLIDENTITY in its message", () => { + resetHardenedStateForTests(); + setPlatformForTests("win32"); + setIcaclsRunnerForTests(() => ({ success: true, exitCode: 0, timedOut: false, stdout: "" })); + setWindowsPrincipalRunnerForTests(() => ({ + success: false, + exitCode: null, + timedOut: true, + stdout: "", + })); + + expect(publisherKeyAclFailureMessage("ocx-cl10-acl-code-identity-")).toContain("EACLIDENTITY"); +}); + +// A cause with no errno-shaped code must leave the message alone rather than +// print an empty parenthetical, and nothing but the bounded code may be appended. +test("a publisher key ACL failure without a bounded code keeps the plain message", () => { + resetHardenedStateForTests(); + setPlatformForTests("win32"); + setIcaclsRunnerForTests(() => { + throw new Error("synthetic icacls runner failure"); + }); + + const message = publisherKeyAclFailureMessage("ocx-cl10-acl-code-plain-"); + expect(message).toBe("public publisher key ACL hardening did not complete"); +});