From a4d7322127e716ba8602e3ec40a5b321a8ecec77 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:28:33 -0400 Subject: [PATCH] refactor(deploy_keys): judge the public key's base64 blob with zod's z.base64() at parse time The deploy key line was accepted by one hand-written regex built from a hand-written base64 grammar, with a second instance for material GitHub returned, and the rejection helpers ran only after the regex failed, to name a reason. The parse is now one structured walk: the fields, the form checks, the side's algorithm rule, then the blob under zod's z.base64(), which covers the alphabet, the padding, and the multiple-of-four length. materialPattern(), STORED_KEY_PATTERN, declaredRejection(), and storedRejection() are gone; the hand-spelled grammar remains only inside PUBLIC_KEY_PATTERN, the published JSON Schema pattern for the key field, whose capture groups are now non-capturing since nothing reads them. zod's check takes the empty string too; the two-field check ahead of it makes an empty blob a missing field on both sides. zod's own published base64 pattern opens with ^$| and cannot sit inside a larger pattern, so the published grammar stays hand-spelled, and test/sections/deploy_keys/published-pattern.test.ts holds the published pattern and parsePublicKey to one verdict over the generators' keys, a seeded sweep over every axis of the line, and the hand-made invalid set (bad padding, a non-base64 character, an empty blob, a wrong length class). Differential check, the origin/main module against this one: 20,050 distinct key lines (45 hand-made, 5 from the e2e generators, 20,000 seeded random), parsePublicKey and parseStoredKey compared on verdict, message, and material: 0 mismatches. The published pattern against the new parsePublicKey: 0 mismatches. Blob-level control: zod's check and the hand grammar diverge on exactly one blob, the empty string. Every refusal message is unchanged. --- src/sections/deploy_keys/schema.ts | 143 +++++++++--------- .../deploy_keys/published-pattern.test.ts | 143 ++++++++++++++++++ test/sections/refusal-messages.test.ts | 2 +- 3 files changed, 219 insertions(+), 69 deletions(-) create mode 100644 test/sections/deploy_keys/published-pattern.test.ts diff --git a/src/sections/deploy_keys/schema.ts b/src/sections/deploy_keys/schema.ts index a78c8117..1a971c26 100644 --- a/src/sections/deploy_keys/schema.ts +++ b/src/sections/deploy_keys/schema.ts @@ -2,6 +2,11 @@ * The `deploy_keys:` section's schema slice; root src/schema.ts composes the SettingsFile property from it. * PUBLIC_KEY_ALGORITHMS is listed by hand in the docs field note and leaves out DSA (ssh-dss): GitHub stopped * accepting it on 2022-03-15, so a DSA key passing the parse would only fail at the create. + * + * PUBLIC_KEY_PATTERN is the published JSON Schema twin of parsePublicKey, which judges the blob with zod's base64 + * check; test/sections/deploy_keys/published-pattern.test.ts holds the two to one verdict. Its base64 grammar is + * spelled by hand because zod's own published pattern opens with `^$|`, which no larger pattern can embed. + * PRIVATE_KEY_FRAMING is assembled at runtime so no source line holds the string a secret scanner flags. */ import { err, ok, type Result } from "neverthrow"; @@ -18,44 +23,24 @@ export const PUBLIC_KEY_ALGORITHMS = [ "sk-ecdsa-sha2-nistp256@openssh.com", ] as const; const ALGORITHMS = new Set(PUBLIC_KEY_ALGORITHMS); - -const BASE64_QUARTET = "[A-Za-z0-9+/]{4}"; -const BASE64_BLOB = `(?:${BASE64_QUARTET})*(?:${BASE64_QUARTET}|[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)`; +const ALGORITHM_ALTERNATION = PUBLIC_KEY_ALGORITHMS.map((algorithm) => + algorithm.replaceAll(".", "\\."), +).join("|"); const FIELD_SEPARATOR = String.raw`[ \t]`; - -/** A second line is refused rather than dropped, so nothing pasted after a valid key is silently normalized away. */ -function materialPattern(algorithms: string): RegExp { - return new RegExp( - `^${FIELD_SEPARATOR}*(${algorithms})${FIELD_SEPARATOR}+(${BASE64_BLOB})(?:${FIELD_SEPARATOR}[^\\r\\n]*)?$`, - ); -} - const FIELDS = new RegExp(`${FIELD_SEPARATOR}+`); const LINE_BREAK = /[\r\n]/; const ODD_WHITESPACE = /[^\S \t\r\n]/; -/** Splits on FIELD_SEPARATOR, so the rejection helpers see the fields the pattern saw. */ -function fieldsOf(raw: string): string[] { - return raw.split(FIELDS).filter((field) => field !== ""); -} - -// The ONE acceptance rule for DECLARED material: the runtime parses through it and the published JSON schema carries its source as the field's pattern, so an editor and the run agree. -const PUBLIC_KEY_PATTERN = materialPattern( - PUBLIC_KEY_ALGORITHMS.map((algorithm) => algorithm.replaceAll(".", "\\.")).join("|"), +const BASE64_QUARTET = "[A-Za-z0-9+/]{4}"; +const BASE64_PATTERN = `(?:${BASE64_QUARTET})*(?:${BASE64_QUARTET}|[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)`; +const PUBLIC_KEY_PATTERN = new RegExp( + `^${FIELD_SEPARATOR}*(?:${ALGORITHM_ALTERNATION})${FIELD_SEPARATOR}+${BASE64_PATTERN}(?:${FIELD_SEPARATOR}[^\\r\\n]*)?$`, ); +const base64Blob = z.base64(); -// The rule for material GitHub STORED, any algorithm token: GitHub validated the key when it accepted it, so a live key under an algorithm PUBLIC_KEY_ALGORITHMS lacks must not abort the section. -const STORED_KEY_PATTERN = materialPattern(String.raw`\S+`); - -// The header and footer framing of every PEM and OpenSSH private key, assembled at runtime so no source line holds the string a secret scanner flags. const PRIVATE_KEY_FRAMING = ["PRIVATE", "KEY-----"].join(" "); -/** Whether the settings file can declare a key under this algorithm; a live key under any other is outside the section. */ -export function declaresAlgorithm(algorithm: string): boolean { - return ALGORITHMS.has(algorithm); -} - const PUBLIC_KEY_FORM = `one line, " [comment]", with the algorithm one of ${PUBLIC_KEY_ALGORITHMS.join(", ")}`; const FEWER_THAN_TWO_FIELDS = "the key has fewer than two fields separated by a space or tab"; const HAS_LINE_BREAK = "the key contains a line break"; @@ -70,38 +55,19 @@ export interface PublicKeyMaterial { readonly material: string; } -/** - * Names what PUBLIC_KEY_PATTERN refused. The reason never quotes the input: it may be a pasted PRIVATE key, and a - * refusal that echoed it would put it in the log and the step summary. - */ -function declaredRejection(raw: string): string { - if (raw.trimStart().startsWith("-----BEGIN")) { - return raw.includes(PRIVATE_KEY_FRAMING) - ? "this is a private key; a deploy key takes the public half (the .pub file)" - : 'this is a PEM block; a deploy key takes the OpenSSH one-line public form, "ssh-ed25519 AAAA... comment" (the .pub file; ssh-keygen -i converts a PEM public key)'; - } - if (LINE_BREAK.test(raw)) { - return `${HAS_LINE_BREAK} (a YAML | block keeps its trailing newline; |- drops it); a public SSH key reads ${PUBLIC_KEY_FORM}`; - } - const fields = fieldsOf(raw); - const form = formRejection(raw, fields); - if (form !== undefined) { - return `${form}; a public SSH key reads ${PUBLIC_KEY_FORM}`; - } - const [algorithm = ""] = fields; - if (algorithm === "ssh-dss") { - return `the key is DSA, which GitHub no longer accepts (since 2022-03-15); a public SSH key reads ${PUBLIC_KEY_FORM}`; - } - if (!ALGORITHMS.has(algorithm)) { - return `the key's first field is not an algorithm GitHub accepts; a public SSH key reads ${PUBLIC_KEY_FORM}`; - } - // A known algorithm with two fields on one line fails the pattern on its blob alone. - return `${BLOB_NOT_BASE64}; a public SSH key reads ${PUBLIC_KEY_FORM}`; +/** Splits on FIELD_SEPARATOR, the separator the published pattern spells, so the parse and an editor see the same fields. */ +function fieldsOf(raw: string): string[] { + return raw.split(FIELDS).filter((field) => field !== ""); +} + +/** Whether the settings file can declare a key under this algorithm; a live key under any other is outside the section. */ +export function declaresAlgorithm(algorithm: string): boolean { + return ALGORITHMS.has(algorithm); } /** * The shape rules both readings share, in the order a reader fixes them; undefined once the value is two or more - * fields on one line, so that only the algorithm or the blob can have failed the pattern. Only those two fields are + * fields on one line, so that only the algorithm or the blob can still be refused. Only those two fields are * tested for odd whitespace (a non-breaking space, for one), since a comment may hold it. */ function formRejection( @@ -117,30 +83,71 @@ function formRejection( return blob === "" ? FEWER_THAN_TWO_FIELDS : undefined; } -/** Names what STORED_KEY_PATTERN refused; the algorithm is never the reason on the live side. */ -function storedRejection(raw: string): string { - return formRejection(raw, fieldsOf(raw)) ?? BLOB_NOT_BASE64; +/** Why the settings file cannot declare a key under `algorithm`, or undefined when it can. */ +function declaredAlgorithmRejection(algorithm: string): string | undefined { + if (algorithm === "ssh-dss") { + return "the key is DSA, which GitHub no longer accepts (since 2022-03-15)"; + } + return ALGORITHMS.has(algorithm) + ? undefined + : "the key's first field is not an algorithm GitHub accepts"; } +/** + * The reading both sides share: the form, the algorithm under the side's rule, then the blob under zod's base64 + * check, which takes the empty string too; the form check ahead of it is what makes an empty blob a missing field. + * A reason never quotes the input: it may be a pasted PRIVATE key, and a refusal that echoed it would put it in + * the log and the step summary. + */ function parseMaterial( - pattern: RegExp, raw: string, - rejection: (raw: string) => string, + algorithmRejection: (algorithm: string) => string | undefined, ): Result { - const match = pattern.exec(raw); - return match === null - ? err(rejection(raw)) - : ok({ algorithm: match[1] ?? "", material: `${match[1]} ${match[2]}` }); + const fields = fieldsOf(raw); + const [algorithm = "", blob = ""] = fields; + + const form = formRejection(raw, fields); + if (form !== undefined) { + return err(form); + } + + const refusedAlgorithm = algorithmRejection(algorithm); + if (refusedAlgorithm !== undefined) { + return err(refusedAlgorithm); + } + + if (!base64Blob.safeParse(blob).success) { + return err(BLOB_NOT_BASE64); + } + + return ok({ algorithm, material: `${algorithm} ${blob}` }); } -/** The one reading of DECLARED deploy key material, shared by the schema and the planner so the two cannot disagree. */ +/** + * The one reading of DECLARED deploy key material, shared by the schema and the planner so the two cannot disagree. + * A second line is refused rather than dropped, so nothing pasted after a valid key is silently normalized away. + */ export function parsePublicKey(raw: string): Result { - return parseMaterial(PUBLIC_KEY_PATTERN, raw, declaredRejection); + if (raw.trimStart().startsWith("-----BEGIN")) { + return err( + raw.includes(PRIVATE_KEY_FRAMING) + ? "this is a private key; a deploy key takes the public half (the .pub file)" + : 'this is a PEM block; a deploy key takes the OpenSSH one-line public form, "ssh-ed25519 AAAA... comment" (the .pub file; ssh-keygen -i converts a PEM public key)', + ); + } + if (LINE_BREAK.test(raw)) { + return err( + `${HAS_LINE_BREAK} (a YAML | block keeps its trailing newline; |- drops it); a public SSH key reads ${PUBLIC_KEY_FORM}`, + ); + } + return parseMaterial(raw, declaredAlgorithmRejection).mapErr( + (reason) => `${reason}; a public SSH key reads ${PUBLIC_KEY_FORM}`, + ); } /** The reading of material GitHub returned: the same comparable form, with GitHub the authority on the algorithm. */ export function parseStoredKey(raw: string): Result { - return parseMaterial(STORED_KEY_PATTERN, raw, storedRejection); + return parseMaterial(raw, () => undefined); } /** The check is entry-level so the refusal can name the entry by title; a field-level one would know only the index. */ diff --git a/test/sections/deploy_keys/published-pattern.test.ts b/test/sections/deploy_keys/published-pattern.test.ts new file mode 100644 index 00000000..530e1dd4 --- /dev/null +++ b/test/sections/deploy_keys/published-pattern.test.ts @@ -0,0 +1,143 @@ +/** + * The published schema's `pattern` for a deploy key line and parsePublicKey are two spellings of one acceptance + * rule: the pattern is a hand-spelled grammar for editors, the parse judges the blob with zod's base64 check. + * Nothing but this test ties them, so a zod release that moved its base64 semantics, or an edit to one spelling + * alone, would otherwise split what an editor accepts from what the run accepts. The corpus is the keys the e2e + * generators declare, a seeded sweep over every axis of the line, and the hand-made invalid set the pattern must + * refuse the way the parse does: bad padding, a non-base64 character, an empty blob, a wrong length class. + * The pattern is read from the built lib/settings.schema.json and compiled by ajv, the way an editor or CI linter judges it. + */ + +import { describe, expect, test } from "bun:test"; +import { Ajv } from "ajv"; +import { PUBLIC_KEY_ALGORITHMS, parsePublicKey } from "../../../src/sections/deploy_keys/schema.js"; +import { Rng } from "../../e2e/generators/prng.js"; +import { readSettingsSchema } from "../../settings-schema.js"; +import { genDeployKeys } from "./generators.js"; + +const SEED = 20261026; +const RANDOM_KEYS = 5000; +const ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + +const LEADING = ["", "", " ", "\t", " \t", "\u00a0"]; +const ALGORITHMS = [ + ...PUBLIC_KEY_ALGORITHMS, + "ssh-dss", + "ssh-ed448", + "SSH-ED25519", + "sk-ssh-ed25519@opensshXcom", + "", + "-----BEGIN", +]; +const SEPARATORS = [" ", " ", "\t", " ", " \t", "", "\u00a0", "\f", "\n"]; +const COMMENTS = ["", "", " deploy@host", "\tc d", " c\u00a0d", " c\n", " AAAA", " two spaces "]; +const TRAILING = ["", "", " ", "\t", "\n", "\r\n", "\u00a0"]; +const BAD_CHARS = ["*", "-", "_", "=", ".", "\u00ff", " ", "\f", "\u00a0", "\n"]; + +const NOT_BASE64 = /^the key's second field is not base64/; +const FEWER_FIELDS = /^the key has fewer than two fields/; +const ODD_WHITESPACE = /^the key contains whitespace other than a space or tab/; + +const INVALID: readonly [what: string, key: string, reason: RegExp][] = [ + ["bad padding, = after a full quartet", "ssh-ed25519 AAAAAAAA=", NOT_BASE64], + ["bad padding, three =", "ssh-ed25519 AAAAA===", NOT_BASE64], + ["bad padding, = mid-way", "ssh-ed25519 AAAA=AAA", NOT_BASE64], + ["bad padding, padding alone", "ssh-ed25519 ====", NOT_BASE64], + ["a non-base64 character", "ssh-ed25519 AAAA*AAA", NOT_BASE64], + ["a base64url character", "ssh-ed25519 AAAA-AAA", NOT_BASE64], + ["a non-ASCII character", "ssh-ed25519 AAAA\u00ffAAA", NOT_BASE64], + ["an empty blob after a space", "ssh-ed25519 ", FEWER_FIELDS], + ["an empty blob after a tab", "ssh-ed25519\t", FEWER_FIELDS], + ["a wrong length class, one character", "ssh-ed25519 A", NOT_BASE64], + ["a wrong length class, three", "ssh-ed25519 AAA", NOT_BASE64], + ["a wrong length class, five", "ssh-ed25519 AAAAA", NOT_BASE64], + ["a wrong length class, six unpadded", "ssh-ed25519 AAAAAA", NOT_BASE64], + ["whitespace inside the blob", "ssh-ed25519 AAA\fAAAA", ODD_WHITESPACE], +]; + +const patternAccepts = (() => { + const definition = readSettingsSchema().definitions.DeployKeyConfig as { + properties: { key: { pattern: string } }; + }; + const ajv = new Ajv({ strict: false }); + return ajv.compile({ type: "string", pattern: definition.properties.key.pattern }); +})(); + +/** A base64 run of `quartets` full groups, closed by the padding class. */ +function validBlob(rng: Rng, quartets: number, padding: "" | "=" | "=="): string { + const bodyLength = quartets * 4 + (padding === "" ? 0 : 4 - padding.length); + const body = Array.from({ length: bodyLength }, () => ALPHABET[rng.int(ALPHABET.length)]); + return body.join("") + padding; +} + +/** The blob edited one way, its length class, a stray character, its padding, or emptied; an edit may land back on valid base64. */ +function mutated(rng: Rng, blob: string): string { + const at = blob.length === 0 ? 0 : rng.int(blob.length); + switch (rng.int(8)) { + case 0: + return blob.slice(0, -1); + case 1: + return `${blob}A`; + case 2: + return blob.slice(0, at) + rng.pick(BAD_CHARS) + blob.slice(at + 1); + case 3: + return blob.slice(0, at) + rng.pick(BAD_CHARS) + blob.slice(at); + case 4: + return `${blob.replace(/=+$/, "")}===`; + case 5: + return `${blob.replace(/=+$/, "")}=`; + case 6: + return ""; + default: + return `${blob.slice(0, at)}=${blob.slice(at)}`; + } +} + +/** One line drawn per axis; a value listed twice in an axis weights the draw toward the plain form, so most keys reach the blob. */ +function randomKey(rng: Rng): string { + const blob = validBlob(rng, rng.int(25), rng.pick(["", "", "=", "=="])); + return [ + rng.pick(LEADING), + rng.pick(ALGORITHMS), + rng.pick(SEPARATORS), + rng.bool(0.4) ? mutated(rng, blob) : blob, + rng.pick(COMMENTS), + rng.pick(TRAILING), + ].join(""); +} + +/** The generators' keys, the invalid set, and the seeded sweep, distinct. */ +function keyCorpus(): string[] { + const rng = new Rng(SEED); + const generated = Array.from({ length: 20 }, (_, seed) => genDeployKeys(new Rng(seed + 1))) + .flat() + .map((entry) => String((entry as { key: unknown }).key)); + const random = Array.from({ length: RANDOM_KEYS }, () => randomKey(rng)); + return [...new Set([...generated, ...INVALID.map(([, key]) => key), ...random])]; +} + +describe("the published key pattern and parsePublicKey are one acceptance rule", () => { + test("every key of the corpus gets the same verdict from the published pattern and from the parse (control: both verdicts occur)", () => { + const verdicts = keyCorpus().map( + (key) => [key, patternAccepts(key), parsePublicKey(key).isOk()] as const, + ); + expect(verdicts.filter(([, pattern, parse]) => pattern !== parse).map(([key]) => key)).toEqual( + [], + ); + expect(new Set(verdicts.map(([, , parse]) => parse))).toEqual(new Set([true, false])); + }); + + // zod's base64 check takes the empty string; the two-field check ahead of it is what makes the empty blob a missing field on both sides. + test.each(INVALID)( + "%s is refused by the published pattern and by the parse, which says why", + (_what, key, reason) => { + expect(patternAccepts(key)).toBe(false); + expect( + parsePublicKey(key).match( + () => "accepted", + (error) => error, + ), + ).toMatch(reason); + }, + ); +}); diff --git a/test/sections/refusal-messages.test.ts b/test/sections/refusal-messages.test.ts index 8b0eb80e..66aa34b5 100644 --- a/test/sections/refusal-messages.test.ts +++ b/test/sections/refusal-messages.test.ts @@ -171,7 +171,7 @@ const DATA_CONSTANTS: ReadonlySet = new Set([ // deploy_keys/schema.ts "PUBLIC_KEY_ALGORITHMS", "BASE64_QUARTET", - "BASE64_BLOB", + "BASE64_PATTERN", "FIELD_SEPARATOR", "PRIVATE_KEY_FRAMING", // repository/schema.ts