Skip to content

Commit b2ed711

Browse files
VivswanVivswan
authored
refactor(deploy_keys): validate the key's base64 blob with z.base64(), the published pattern held in lockstep (#565)
1 parent 3b774f6 commit b2ed711

3 files changed

Lines changed: 219 additions & 69 deletions

File tree

‎src/sections/deploy_keys/schema.ts‎

Lines changed: 75 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
* The `deploy_keys:` section's schema slice; root src/schema.ts composes the SettingsFile property from it.
33
* PUBLIC_KEY_ALGORITHMS is listed by hand in the docs field note and leaves out DSA (ssh-dss): GitHub stopped
44
* accepting it on 2022-03-15, so a DSA key passing the parse would only fail at the create.
5+
*
6+
* PUBLIC_KEY_PATTERN is the published JSON Schema twin of parsePublicKey, which judges the blob with zod's base64
7+
* check; test/sections/deploy_keys/published-pattern.test.ts holds the two to one verdict. Its base64 grammar is
8+
* spelled by hand because zod's own published pattern opens with `^$|`, which no larger pattern can embed.
9+
* PRIVATE_KEY_FRAMING is assembled at runtime so no source line holds the string a secret scanner flags.
510
*/
611

712
import { err, ok, type Result } from "neverthrow";
@@ -18,44 +23,24 @@ export const PUBLIC_KEY_ALGORITHMS = [
1823
"sk-ecdsa-sha2-nistp256@openssh.com",
1924
] as const;
2025
const ALGORITHMS = new Set<string>(PUBLIC_KEY_ALGORITHMS);
21-
22-
const BASE64_QUARTET = "[A-Za-z0-9+/]{4}";
23-
const BASE64_BLOB = `(?:${BASE64_QUARTET})*(?:${BASE64_QUARTET}|[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)`;
26+
const ALGORITHM_ALTERNATION = PUBLIC_KEY_ALGORITHMS.map((algorithm) =>
27+
algorithm.replaceAll(".", "\\."),
28+
).join("|");
2429

2530
const FIELD_SEPARATOR = String.raw`[ \t]`;
26-
27-
/** A second line is refused rather than dropped, so nothing pasted after a valid key is silently normalized away. */
28-
function materialPattern(algorithms: string): RegExp {
29-
return new RegExp(
30-
`^${FIELD_SEPARATOR}*(${algorithms})${FIELD_SEPARATOR}+(${BASE64_BLOB})(?:${FIELD_SEPARATOR}[^\\r\\n]*)?$`,
31-
);
32-
}
33-
3431
const FIELDS = new RegExp(`${FIELD_SEPARATOR}+`);
3532
const LINE_BREAK = /[\r\n]/;
3633
const ODD_WHITESPACE = /[^\S \t\r\n]/;
3734

38-
/** Splits on FIELD_SEPARATOR, so the rejection helpers see the fields the pattern saw. */
39-
function fieldsOf(raw: string): string[] {
40-
return raw.split(FIELDS).filter((field) => field !== "");
41-
}
42-
43-
// 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.
44-
const PUBLIC_KEY_PATTERN = materialPattern(
45-
PUBLIC_KEY_ALGORITHMS.map((algorithm) => algorithm.replaceAll(".", "\\.")).join("|"),
35+
const BASE64_QUARTET = "[A-Za-z0-9+/]{4}";
36+
const BASE64_PATTERN = `(?:${BASE64_QUARTET})*(?:${BASE64_QUARTET}|[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)`;
37+
const PUBLIC_KEY_PATTERN = new RegExp(
38+
`^${FIELD_SEPARATOR}*(?:${ALGORITHM_ALTERNATION})${FIELD_SEPARATOR}+${BASE64_PATTERN}(?:${FIELD_SEPARATOR}[^\\r\\n]*)?$`,
4639
);
40+
const base64Blob = z.base64();
4741

48-
// 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.
49-
const STORED_KEY_PATTERN = materialPattern(String.raw`\S+`);
50-
51-
// 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.
5242
const PRIVATE_KEY_FRAMING = ["PRIVATE", "KEY-----"].join(" ");
5343

54-
/** Whether the settings file can declare a key under this algorithm; a live key under any other is outside the section. */
55-
export function declaresAlgorithm(algorithm: string): boolean {
56-
return ALGORITHMS.has(algorithm);
57-
}
58-
5944
const PUBLIC_KEY_FORM = `one line, "<algorithm> <base64> [comment]", with the algorithm one of ${PUBLIC_KEY_ALGORITHMS.join(", ")}`;
6045
const FEWER_THAN_TWO_FIELDS = "the key has fewer than two fields separated by a space or tab";
6146
const HAS_LINE_BREAK = "the key contains a line break";
@@ -70,38 +55,19 @@ export interface PublicKeyMaterial {
7055
readonly material: string;
7156
}
7257

73-
/**
74-
* Names what PUBLIC_KEY_PATTERN refused. The reason never quotes the input: it may be a pasted PRIVATE key, and a
75-
* refusal that echoed it would put it in the log and the step summary.
76-
*/
77-
function declaredRejection(raw: string): string {
78-
if (raw.trimStart().startsWith("-----BEGIN")) {
79-
return raw.includes(PRIVATE_KEY_FRAMING)
80-
? "this is a private key; a deploy key takes the public half (the .pub file)"
81-
: '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)';
82-
}
83-
if (LINE_BREAK.test(raw)) {
84-
return `${HAS_LINE_BREAK} (a YAML | block keeps its trailing newline; |- drops it); a public SSH key reads ${PUBLIC_KEY_FORM}`;
85-
}
86-
const fields = fieldsOf(raw);
87-
const form = formRejection(raw, fields);
88-
if (form !== undefined) {
89-
return `${form}; a public SSH key reads ${PUBLIC_KEY_FORM}`;
90-
}
91-
const [algorithm = ""] = fields;
92-
if (algorithm === "ssh-dss") {
93-
return `the key is DSA, which GitHub no longer accepts (since 2022-03-15); a public SSH key reads ${PUBLIC_KEY_FORM}`;
94-
}
95-
if (!ALGORITHMS.has(algorithm)) {
96-
return `the key's first field is not an algorithm GitHub accepts; a public SSH key reads ${PUBLIC_KEY_FORM}`;
97-
}
98-
// A known algorithm with two fields on one line fails the pattern on its blob alone.
99-
return `${BLOB_NOT_BASE64}; a public SSH key reads ${PUBLIC_KEY_FORM}`;
58+
/** Splits on FIELD_SEPARATOR, the separator the published pattern spells, so the parse and an editor see the same fields. */
59+
function fieldsOf(raw: string): string[] {
60+
return raw.split(FIELDS).filter((field) => field !== "");
61+
}
62+
63+
/** Whether the settings file can declare a key under this algorithm; a live key under any other is outside the section. */
64+
export function declaresAlgorithm(algorithm: string): boolean {
65+
return ALGORITHMS.has(algorithm);
10066
}
10167

10268
/**
10369
* The shape rules both readings share, in the order a reader fixes them; undefined once the value is two or more
104-
* fields on one line, so that only the algorithm or the blob can have failed the pattern. Only those two fields are
70+
* fields on one line, so that only the algorithm or the blob can still be refused. Only those two fields are
10571
* tested for odd whitespace (a non-breaking space, for one), since a comment may hold it.
10672
*/
10773
function formRejection(
@@ -117,30 +83,71 @@ function formRejection(
11783
return blob === "" ? FEWER_THAN_TWO_FIELDS : undefined;
11884
}
11985

120-
/** Names what STORED_KEY_PATTERN refused; the algorithm is never the reason on the live side. */
121-
function storedRejection(raw: string): string {
122-
return formRejection(raw, fieldsOf(raw)) ?? BLOB_NOT_BASE64;
86+
/** Why the settings file cannot declare a key under `algorithm`, or undefined when it can. */
87+
function declaredAlgorithmRejection(algorithm: string): string | undefined {
88+
if (algorithm === "ssh-dss") {
89+
return "the key is DSA, which GitHub no longer accepts (since 2022-03-15)";
90+
}
91+
return ALGORITHMS.has(algorithm)
92+
? undefined
93+
: "the key's first field is not an algorithm GitHub accepts";
12394
}
12495

96+
/**
97+
* The reading both sides share: the form, the algorithm under the side's rule, then the blob under zod's base64
98+
* check, which takes the empty string too; the form check ahead of it is what makes an empty blob a missing field.
99+
* A reason never quotes the input: it may be a pasted PRIVATE key, and a refusal that echoed it would put it in
100+
* the log and the step summary.
101+
*/
125102
function parseMaterial(
126-
pattern: RegExp,
127103
raw: string,
128-
rejection: (raw: string) => string,
104+
algorithmRejection: (algorithm: string) => string | undefined,
129105
): Result<PublicKeyMaterial, string> {
130-
const match = pattern.exec(raw);
131-
return match === null
132-
? err(rejection(raw))
133-
: ok({ algorithm: match[1] ?? "", material: `${match[1]} ${match[2]}` });
106+
const fields = fieldsOf(raw);
107+
const [algorithm = "", blob = ""] = fields;
108+
109+
const form = formRejection(raw, fields);
110+
if (form !== undefined) {
111+
return err(form);
112+
}
113+
114+
const refusedAlgorithm = algorithmRejection(algorithm);
115+
if (refusedAlgorithm !== undefined) {
116+
return err(refusedAlgorithm);
117+
}
118+
119+
if (!base64Blob.safeParse(blob).success) {
120+
return err(BLOB_NOT_BASE64);
121+
}
122+
123+
return ok({ algorithm, material: `${algorithm} ${blob}` });
134124
}
135125

136-
/** The one reading of DECLARED deploy key material, shared by the schema and the planner so the two cannot disagree. */
126+
/**
127+
* The one reading of DECLARED deploy key material, shared by the schema and the planner so the two cannot disagree.
128+
* A second line is refused rather than dropped, so nothing pasted after a valid key is silently normalized away.
129+
*/
137130
export function parsePublicKey(raw: string): Result<PublicKeyMaterial, string> {
138-
return parseMaterial(PUBLIC_KEY_PATTERN, raw, declaredRejection);
131+
if (raw.trimStart().startsWith("-----BEGIN")) {
132+
return err(
133+
raw.includes(PRIVATE_KEY_FRAMING)
134+
? "this is a private key; a deploy key takes the public half (the .pub file)"
135+
: '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)',
136+
);
137+
}
138+
if (LINE_BREAK.test(raw)) {
139+
return err(
140+
`${HAS_LINE_BREAK} (a YAML | block keeps its trailing newline; |- drops it); a public SSH key reads ${PUBLIC_KEY_FORM}`,
141+
);
142+
}
143+
return parseMaterial(raw, declaredAlgorithmRejection).mapErr(
144+
(reason) => `${reason}; a public SSH key reads ${PUBLIC_KEY_FORM}`,
145+
);
139146
}
140147

141148
/** The reading of material GitHub returned: the same comparable form, with GitHub the authority on the algorithm. */
142149
export function parseStoredKey(raw: string): Result<PublicKeyMaterial, string> {
143-
return parseMaterial(STORED_KEY_PATTERN, raw, storedRejection);
150+
return parseMaterial(raw, () => undefined);
144151
}
145152

146153
/** The check is entry-level so the refusal can name the entry by title; a field-level one would know only the index. */

‎test/sections/deploy_keys/published-pattern.test.ts‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/**
2+
* The published schema's `pattern` for a deploy key line and parsePublicKey are two spellings of one acceptance
3+
* rule: the pattern is a hand-spelled grammar for editors, the parse judges the blob with zod's base64 check.
4+
* Nothing but this test ties them, so a zod release that moved its base64 semantics, or an edit to one spelling
5+
* alone, would otherwise split what an editor accepts from what the run accepts. The corpus is the keys the e2e
6+
* generators declare, a seeded sweep over every axis of the line, and the hand-made invalid set the pattern must
7+
* refuse the way the parse does: bad padding, a non-base64 character, an empty blob, a wrong length class.
8+
* The pattern is read from the built lib/settings.schema.json and compiled by ajv, the way an editor or CI linter judges it.
9+
*/
10+
11+
import { describe, expect, test } from "bun:test";
12+
import { Ajv } from "ajv";
13+
import { PUBLIC_KEY_ALGORITHMS, parsePublicKey } from "../../../src/sections/deploy_keys/schema.js";
14+
import { Rng } from "../../e2e/generators/prng.js";
15+
import { readSettingsSchema } from "../../settings-schema.js";
16+
import { genDeployKeys } from "./generators.js";
17+
18+
const SEED = 20261026;
19+
const RANDOM_KEYS = 5000;
20+
const ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
21+
22+
const LEADING = ["", "", " ", "\t", " \t", "\u00a0"];
23+
const ALGORITHMS = [
24+
...PUBLIC_KEY_ALGORITHMS,
25+
"ssh-dss",
26+
"ssh-ed448",
27+
"SSH-ED25519",
28+
"sk-ssh-ed25519@opensshXcom",
29+
"",
30+
"-----BEGIN",
31+
];
32+
const SEPARATORS = [" ", " ", "\t", " ", " \t", "", "\u00a0", "\f", "\n"];
33+
const COMMENTS = ["", "", " deploy@host", "\tc d", " c\u00a0d", " c\n", " AAAA", " two spaces "];
34+
const TRAILING = ["", "", " ", "\t", "\n", "\r\n", "\u00a0"];
35+
const BAD_CHARS = ["*", "-", "_", "=", ".", "\u00ff", " ", "\f", "\u00a0", "\n"];
36+
37+
const NOT_BASE64 = /^the key's second field is not base64/;
38+
const FEWER_FIELDS = /^the key has fewer than two fields/;
39+
const ODD_WHITESPACE = /^the key contains whitespace other than a space or tab/;
40+
41+
const INVALID: readonly [what: string, key: string, reason: RegExp][] = [
42+
["bad padding, = after a full quartet", "ssh-ed25519 AAAAAAAA=", NOT_BASE64],
43+
["bad padding, three =", "ssh-ed25519 AAAAA===", NOT_BASE64],
44+
["bad padding, = mid-way", "ssh-ed25519 AAAA=AAA", NOT_BASE64],
45+
["bad padding, padding alone", "ssh-ed25519 ====", NOT_BASE64],
46+
["a non-base64 character", "ssh-ed25519 AAAA*AAA", NOT_BASE64],
47+
["a base64url character", "ssh-ed25519 AAAA-AAA", NOT_BASE64],
48+
["a non-ASCII character", "ssh-ed25519 AAAA\u00ffAAA", NOT_BASE64],
49+
["an empty blob after a space", "ssh-ed25519 ", FEWER_FIELDS],
50+
["an empty blob after a tab", "ssh-ed25519\t", FEWER_FIELDS],
51+
["a wrong length class, one character", "ssh-ed25519 A", NOT_BASE64],
52+
["a wrong length class, three", "ssh-ed25519 AAA", NOT_BASE64],
53+
["a wrong length class, five", "ssh-ed25519 AAAAA", NOT_BASE64],
54+
["a wrong length class, six unpadded", "ssh-ed25519 AAAAAA", NOT_BASE64],
55+
["whitespace inside the blob", "ssh-ed25519 AAA\fAAAA", ODD_WHITESPACE],
56+
];
57+
58+
const patternAccepts = (() => {
59+
const definition = readSettingsSchema().definitions.DeployKeyConfig as {
60+
properties: { key: { pattern: string } };
61+
};
62+
const ajv = new Ajv({ strict: false });
63+
return ajv.compile({ type: "string", pattern: definition.properties.key.pattern });
64+
})();
65+
66+
/** A base64 run of `quartets` full groups, closed by the padding class. */
67+
function validBlob(rng: Rng, quartets: number, padding: "" | "=" | "=="): string {
68+
const bodyLength = quartets * 4 + (padding === "" ? 0 : 4 - padding.length);
69+
const body = Array.from({ length: bodyLength }, () => ALPHABET[rng.int(ALPHABET.length)]);
70+
return body.join("") + padding;
71+
}
72+
73+
/** The blob edited one way, its length class, a stray character, its padding, or emptied; an edit may land back on valid base64. */
74+
function mutated(rng: Rng, blob: string): string {
75+
const at = blob.length === 0 ? 0 : rng.int(blob.length);
76+
switch (rng.int(8)) {
77+
case 0:
78+
return blob.slice(0, -1);
79+
case 1:
80+
return `${blob}A`;
81+
case 2:
82+
return blob.slice(0, at) + rng.pick(BAD_CHARS) + blob.slice(at + 1);
83+
case 3:
84+
return blob.slice(0, at) + rng.pick(BAD_CHARS) + blob.slice(at);
85+
case 4:
86+
return `${blob.replace(/=+$/, "")}===`;
87+
case 5:
88+
return `${blob.replace(/=+$/, "")}=`;
89+
case 6:
90+
return "";
91+
default:
92+
return `${blob.slice(0, at)}=${blob.slice(at)}`;
93+
}
94+
}
95+
96+
/** 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. */
97+
function randomKey(rng: Rng): string {
98+
const blob = validBlob(rng, rng.int(25), rng.pick(["", "", "=", "=="]));
99+
return [
100+
rng.pick(LEADING),
101+
rng.pick(ALGORITHMS),
102+
rng.pick(SEPARATORS),
103+
rng.bool(0.4) ? mutated(rng, blob) : blob,
104+
rng.pick(COMMENTS),
105+
rng.pick(TRAILING),
106+
].join("");
107+
}
108+
109+
/** The generators' keys, the invalid set, and the seeded sweep, distinct. */
110+
function keyCorpus(): string[] {
111+
const rng = new Rng(SEED);
112+
const generated = Array.from({ length: 20 }, (_, seed) => genDeployKeys(new Rng(seed + 1)))
113+
.flat()
114+
.map((entry) => String((entry as { key: unknown }).key));
115+
const random = Array.from({ length: RANDOM_KEYS }, () => randomKey(rng));
116+
return [...new Set([...generated, ...INVALID.map(([, key]) => key), ...random])];
117+
}
118+
119+
describe("the published key pattern and parsePublicKey are one acceptance rule", () => {
120+
test("every key of the corpus gets the same verdict from the published pattern and from the parse (control: both verdicts occur)", () => {
121+
const verdicts = keyCorpus().map(
122+
(key) => [key, patternAccepts(key), parsePublicKey(key).isOk()] as const,
123+
);
124+
expect(verdicts.filter(([, pattern, parse]) => pattern !== parse).map(([key]) => key)).toEqual(
125+
[],
126+
);
127+
expect(new Set(verdicts.map(([, , parse]) => parse))).toEqual(new Set([true, false]));
128+
});
129+
130+
// 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.
131+
test.each(INVALID)(
132+
"%s is refused by the published pattern and by the parse, which says why",
133+
(_what, key, reason) => {
134+
expect(patternAccepts(key)).toBe(false);
135+
expect(
136+
parsePublicKey(key).match(
137+
() => "accepted",
138+
(error) => error,
139+
),
140+
).toMatch(reason);
141+
},
142+
);
143+
});

‎test/sections/refusal-messages.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ const DATA_CONSTANTS: ReadonlySet<string> = new Set([
171171
// deploy_keys/schema.ts
172172
"PUBLIC_KEY_ALGORITHMS",
173173
"BASE64_QUARTET",
174-
"BASE64_BLOB",
174+
"BASE64_PATTERN",
175175
"FIELD_SEPARATOR",
176176
"PRIVATE_KEY_FRAMING",
177177
// repository/schema.ts

0 commit comments

Comments
 (0)