diff --git a/bun.lock b/bun.lock index 9c43e8ba..953fec84 100644 --- a/bun.lock +++ b/bun.lock @@ -37,6 +37,7 @@ "ajv": "8.20.0", "ajv-formats": "3.0.1", "entities": "8.1.0", + "estree-walker": "3.0.3", "graphql": "17.0.2", "knip": "6.34.0", "lefthook": "2.1.12", @@ -307,6 +308,8 @@ "@types/bun": ["@types/bun@1.4.0", "", { "dependencies": { "bun-types": "1.4.0" } }, "sha512-K+lZULY23vRgK/CfTjFIV+tyifaNdSMlPh9j+6mQ/cLfpOznLyAuzgV/JQysyECpkBQLVMSyvjlr2fBUSA9wFQ=="], + "@types/estree": ["@types/estree@1.0.9", "", {}, "sha512-GhdPgy1el4/ImP05X05Uw4cw2/M93BCUmnEvWZNStlCzEKME4Fkk+YpoA5OiHNQmoS7Cafb8Xa3Pya8m1Qrzeg=="], + "@types/node": ["@types/node@26.4.1", "", { "dependencies": { "undici-types": "~8.3.0" } }, "sha512-k97ENvZWtvA6yqz5/FS6a7duDgOPEeOQOc2iKS/nY6mX6qJUKtLnWzQS+Xj6tXweyj6ZcTAK2Qecetnvi9nCLA=="], "@typescript/typescript-aix-ppc64": ["@typescript/typescript-aix-ppc64@7.0.2", "", { "os": "aix", "cpu": "ppc64" }, "sha512-MTKKkWB7p/0E9xi1d1tHtZ5PiLkGEMIq88pK2CubZjOsLtYTLqhgIgi6zepFa+9GHZ6h05NMCkQxGKiPXMxXtQ=="], @@ -519,6 +522,8 @@ "escalade": ["escalade@3.2.0", "", {}, "sha512-WUj2qlxaQtO4g6Pq5c29GTcWGDyd8itL8zTlipgECz3JesAiiOKotd8JU6otB3PACgG6xkJUyVhboMS+bje/jA=="], + "estree-walker": ["estree-walker@3.0.3", "", { "dependencies": { "@types/estree": "^1.0.0" } }, "sha512-7RUKfXgSMMkzt6ZuXmqapOurLGPPfgj6l9uRZ7lRGolvk0y2yocc35LdcxKC5PQZdn2DMqioAQ2NoWcrTKmm6g=="], + "event-target-shim": ["event-target-shim@5.0.1", "", {}, "sha512-i/2XbnSz/uxRCU6+NdVJgKWDTM427+MqYbkQzD321DuCQJUqOuJKIA0IM2+W2xtYHdKOmZ4dR6fExsd4SXL+WQ=="], "events": ["events@3.3.0", "", {}, "sha512-mQw+2fkQbALzQ7V0MY0IqdnXNOeTtP4r0lN9z7AAawCXgqea7bDii20AYrIBrFd/Hx0M2Ocz6S111CaFkUcb0Q=="], diff --git a/package.json b/package.json index b6b9164e..bc2c5584 100644 --- a/package.json +++ b/package.json @@ -94,6 +94,7 @@ "ajv": "8.20.0", "ajv-formats": "3.0.1", "entities": "8.1.0", + "estree-walker": "3.0.3", "graphql": "17.0.2", "knip": "6.34.0", "lefthook": "2.1.12", diff --git a/test/problem.test.ts b/test/problem.test.ts index 5ff96d21..a1552b16 100644 --- a/test/problem.test.ts +++ b/test/problem.test.ts @@ -8,11 +8,15 @@ import { describe, expect, test } from "bun:test"; import { + badDirectiveIssue, describeProblem, type Problem, type ProblemOf, quoteList, type SettingsProblem, + singleDocumentRemovalIssue, + unknownDirectivesIssue, + unknownSectionsIssue, } from "../src/problem.js"; import { SECTION_KEYS } from "../src/schema.js"; @@ -495,3 +499,61 @@ describe("quoteList", () => { expect(quoteList([])).toBe(""); }); }); + +describe("the document-level issue builders render the lines a user reads", () => { + const DIRECTIVES = + "The underscore marks this action's directives, \"_layering\" (a file's top level or a list " + + "section's {entries} wrapper) and \"_undeclared\" (a file's top level or a wrapper), and " + + "nothing else; there are no private-note keys. Remove the key, or keep the note as a YAML " + + "comment"; + + test.each<[what: string, line: string, expected: string]>([ + [ + "one unknown underscore key", + unknownDirectivesIssue(["_notes"]), + `unknown underscore key: _notes. ${DIRECTIVES}`, + ], + [ + "two unknown underscore keys", + unknownDirectivesIssue(["_notes", "_owner"]), + `unknown underscore keys: _notes, _owner. ${DIRECTIVES}`, + ], + [ + "a file-wide policy outside the two values", + badDirectiveIssue("sometimes", ["keep", "delete"]), + '_undeclared must be one of "keep", "delete"; got a string that is none of them. Write _undeclared: keep or _undeclared: delete at the top of the file, or remove the key so each list\'s own policy applies', + ], + [ + "a file-wide policy that is a list", + badDirectiveIssue(["keep"], ["keep", "delete"]), + '_undeclared must be one of "keep", "delete"; got a list. Write _undeclared: keep or _undeclared: delete at the top of the file, or remove the key so each list\'s own policy applies', + ], + [ + "a file-wide policy that is a mapping", + badDirectiveIssue({ keep: true }, ["keep", "delete"]), + '_undeclared must be one of "keep", "delete"; got a mapping. Write _undeclared: keep or _undeclared: delete at the top of the file, or remove the key so each list\'s own policy applies', + ], + [ + "a file-wide policy that is not a string", + badDirectiveIssue(null, ["keep", "delete"]), + '_undeclared must be one of "keep", "delete"; got null. Write _undeclared: keep or _undeclared: delete at the top of the file, or remove the key so each list\'s own policy applies', + ], + [ + "a removal marker in a document that is not a layer", + singleDocumentRemovalIssue("labels[2]"), + "labels[2]: a single document has no lower layer to remove from; _remove: true belongs in a higher layer of a fold (mode: render)", + ], + [ + "one unknown section", + unknownSectionsIssue(["tags"], ["labels", "teams"]), + 'unknown top-level section: tags (known: labels, teams). Fix the typo, or set the "sections" input to limit processing', + ], + [ + "two unknown sections", + unknownSectionsIssue(["tags", "issues"], ["labels", "teams"]), + 'unknown top-level sections: tags, issues (known: labels, teams). Fix the typo, or set the "sections" input to limit processing', + ], + ])("%s", (_what, line, expected) => { + expect(line).toBe(expected); + }); +}); diff --git a/test/sections/actions/schema.test.ts b/test/sections/actions/schema.test.ts new file mode 100644 index 00000000..72d712a6 --- /dev/null +++ b/test/sections/actions/schema.test.ts @@ -0,0 +1,100 @@ +/** + * The actions section's parse refusals, each pinned as the problem line a user reads: GitHub's GET-only fields + * (declared, they would re-PUT forever), the OIDC claim-key rules, and an allowlist declared under a policy that + * ignores it. Parsed through the loosened document shape, so a rule that survives here reaches the run. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(actions: Record): readonly string[] | null { + return validateSectionShapes({ actions }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +const REPORTED_ONLY = (field: string) => + `${field} is a value GitHub reports, not a setting it accepts (the GET returns it, the PUT does not take it), so a declared value could never be applied; remove it from the settings file`; + +describe("an actions setting GitHub would ignore or 422 is refused at parse, naming the key and the fix", () => { + test.each<[what: string, actions: Record, expected: string[]]>([ + [ + "the GET-only allowlist link at the top level", + { selected_actions_url: "https://api.github.com/x" }, + [`actions.selected_actions_url: ${REPORTED_ONLY("selected_actions_url")}`], + ], + [ + "the GET-only retention ceiling", + { artifact_and_log_retention: { days: 30, maximum_allowed_days: 90 } }, + [ + `actions.artifact_and_log_retention.maximum_allowed_days: ${REPORTED_ONLY("maximum_allowed_days")}`, + ], + ], + [ + "the GET-only subject prefix", + { oidc_customization_sub: { use_default: true, sub_claim_prefix: "repo:" } }, + [`actions.oidc_customization_sub.sub_claim_prefix: ${REPORTED_ONLY("sub_claim_prefix")}`], + ], + [ + "a claim key with a character GitHub refuses", + { oidc_customization_sub: { use_default: false, include_claim_keys: ["repo", "job-ref"] } }, + [ + 'actions.oidc_customization_sub.include_claim_keys[1]: a claim key holds only letters, digits, and underscores (such as "repo" or "job_workflow_ref")', + ], + ], + [ + "a repeated claim key", + { oidc_customization_sub: { use_default: false, include_claim_keys: ["repo", "repo"] } }, + [ + 'actions.oidc_customization_sub.include_claim_keys[1]: "repo" repeats an earlier claim key; GitHub requires the keys to be unique', + ], + ], + [ + "a claim-key list beside the default template, which GitHub ignores", + { oidc_customization_sub: { use_default: true, include_claim_keys: ["repo"] } }, + [ + "actions.oidc_customization_sub.include_claim_keys: GitHub ignores include_claim_keys under use_default: true, so the declared list could never take; set use_default: false for a custom template, or remove the list", + ], + ], + [ + "an allowlist under a policy that allows every action", + { allowed_actions: "all", selected_actions: { github_owned_allowed: true } }, + [ + 'actions.selected_actions: selected_actions is declared together with allowed_actions: "all", but an allowlist only applies under allowed_actions: "selected". Set allowed_actions to "selected", or remove selected_actions', + ], + ], + [ + "an allowlist beside a policy written as a list", + { allowed_actions: [], selected_actions: { github_owned_allowed: true } }, + [ + 'actions.allowed_actions: Invalid option: expected one of "all"|"local_only"|"selected"', + 'actions.selected_actions: selected_actions is declared together with allowed_actions: a list, but an allowlist only applies under allowed_actions: "selected". Set allowed_actions to "selected", or remove selected_actions', + ], + ], + [ + "an allowlist beside a policy written as a mapping", + { allowed_actions: {}, selected_actions: { github_owned_allowed: true } }, + [ + 'actions.allowed_actions: Invalid option: expected one of "all"|"local_only"|"selected"', + 'actions.selected_actions: selected_actions is declared together with allowed_actions: a mapping, but an allowlist only applies under allowed_actions: "selected". Set allowed_actions to "selected", or remove selected_actions', + ], + ], + ])("%s", (_what, actions, expected) => { + expect(issues(actions)).toEqual(expected); + }); + + test("the same keys in the forms GitHub accepts parse", () => { + expect( + issues({ + allowed_actions: "selected", + selected_actions: { github_owned_allowed: true, patterns_allowed: ["actions/*"] }, + artifact_and_log_retention: { days: 30 }, + oidc_customization_sub: { + use_default: false, + include_claim_keys: ["repo", "job_workflow_ref"], + }, + }), + ).toBeNull(); + }); +}); diff --git a/test/sections/branches/schema.test.ts b/test/sections/branches/schema.test.ts index 73c0bb93..487608a3 100644 --- a/test/sections/branches/schema.test.ts +++ b/test/sections/branches/schema.test.ts @@ -9,43 +9,49 @@ function issues(entries: unknown[]): string[] { : parsed.error.issues.map((issue) => `${issue.path.join(".")}: ${issue.message}`); } +const WILDCARD_KEYS = + "enforce_admins, required_linear_history, allow_force_pushes, allow_deletions, " + + "block_creations, required_conversation_resolution, lock_branch, allow_fork_syncing, " + + "required_signatures, required_status_checks, required_pull_request_reviews, " + + "force_push_bypassers, required_deployments"; + const cases: Array<{ refused: string; protection: Record; name?: string; paths: string[]; - /** Substrings every issue's message must carry (the path never stands in for them). */ + /** The message of every issue in full (one per path when they differ): the path never stands in for it. */ fix: string | string[]; }> = [ { refused: "a status-check requirement without strict (the PUT 422s on it)", protection: { required_status_checks: { contexts: ["ci"] } }, paths: ["0.protection.required_status_checks.strict"], - fix: "up to date with its base", + fix: "required_status_checks.strict must be an unquoted true or false (GitHub's protection PUT rejects the requirement without it): true also requires the branch to be up to date with its base before merging, false only requires the checks to pass", }, { refused: "a status-check requirement without a check list (the PUT 422s on it)", protection: { required_status_checks: { strict: true } }, paths: ["0.protection.required_status_checks"], - fix: "contexts: [] requires none", + fix: "required_status_checks must list the required checks as contexts: [names] or checks: [{context, app_id}] (GitHub's protection PUT rejects the requirement without them); contexts: [] requires none", }, { refused: "a restrictions holder without its users and teams lists (the PUT 422s on it)", protection: { restrictions: {} }, paths: ["0.protection.restrictions.users", "0.protection.restrictions.teams"], - fix: "restrictions: null lifts the push restriction", + fix: "protection.restrictions must carry both users and teams ([] when none; apps is optional), since GitHub's protection PUT requires the two lists; restrictions: null lifts the push restriction", }, { refused: "a restrictions holder naming only apps (the PUT 422s without users and teams)", protection: { restrictions: { apps: ["deploy-gate"] } }, paths: ["0.protection.restrictions.users", "0.protection.restrictions.teams"], - fix: "must carry both users and teams", + fix: "protection.restrictions must carry both users and teams ([] when none; apps is optional), since GitHub's protection PUT requires the two lists; restrictions: null lifts the push restriction", }, { refused: "a restrictions holder naming users but no teams", protection: { restrictions: { users: ["octocat"] } }, paths: ["0.protection.restrictions.teams"], - fix: "[] when none", + fix: "protection.restrictions must carry both users and teams ([] when none; apps is optional), since GitHub's protection PUT requires the two lists; restrictions: null lifts the push restriction", }, { refused: "a check item carrying a key the PUT has no word for", @@ -53,13 +59,13 @@ const cases: Array<{ required_status_checks: { strict: true, checks: [{ context: "ci", app: "ci-bot" }] }, }, paths: ["0.protection.required_status_checks.checks.0"], - fix: 'remove "app"', + fix: 'a required_status_checks.checks item takes only context and app_id (GitHub\'s protection PUT has no other field there); remove "app"', }, { refused: "a check item without its context", protection: { required_status_checks: { strict: true, checks: [{ app_id: 15368 }] } }, paths: ["0.protection.required_status_checks.checks.0.context"], - fix: "the check's name", + fix: "required_status_checks.checks[].context must be the check's name, as a string", }, { refused: "a fractional app_id on a check item", @@ -67,26 +73,26 @@ const cases: Array<{ required_status_checks: { strict: true, checks: [{ context: "ci", app_id: 1.5 }] }, }, paths: ["0.protection.required_status_checks.checks.0.app_id"], - fix: "-1", + fix: "required_status_checks.checks[].app_id must be a whole number: the id of the GitHub App that must report the check, or -1 to let any App report it; omit it to pin whichever App reported it last", }, { refused: "a bare name where a check item goes", protection: { required_status_checks: { strict: true, checks: ["ci"] } }, paths: ["0.protection.required_status_checks.checks.0"], - fix: "contexts: [names]", + fix: "each required_status_checks.checks item is a {context, app_id} mapping naming one required check; a bare name goes under contexts: [names]", }, { refused: "a scalar where the status-check mapping goes, on a literal branch", protection: { required_status_checks: true }, paths: ["0.protection.required_status_checks"], - fix: "or null to turn the requirement off", + fix: "required_status_checks must be a mapping of its keys (strict, then contexts or checks), or null to turn the requirement off", }, { refused: "a scalar where the review mapping goes, on a wildcard rule", name: "release/*", protection: { required_pull_request_reviews: 5 }, paths: ["0.protection.required_pull_request_reviews"], - fix: "or null to turn the requirement off", + fix: "required_pull_request_reviews must be a mapping of its keys (required_approving_review_count and the other review settings), or null to turn the requirement off", }, { refused: @@ -99,25 +105,28 @@ const cases: Array<{ "0.protection.required_status_checks.enabled", "0.protection.required_pull_request_reviews.enabled", ], - fix: "remove it (the control's own key carries the toggle)", + fix: ["required_status_checks", "required_pull_request_reviews"].map( + (control) => + `protection.${control}.enabled is GitHub's GET-only echo, which the protection PUT has no word for; remove it (the control's own key carries the toggle)`, + ), }, { refused: "a review count of 7 (GitHub 422s above 6)", protection: { required_pull_request_reviews: { required_approving_review_count: 7 } }, paths: ["0.protection.required_pull_request_reviews.required_approving_review_count"], - fix: "from 0 to 6", + fix: "required_pull_request_reviews.required_approving_review_count must be a whole number from 0 to 6 (GitHub accepts 1 to 6, or 0 to require no approvals)", }, { refused: "a fractional review count", protection: { required_pull_request_reviews: { required_approving_review_count: 1.5 } }, paths: ["0.protection.required_pull_request_reviews.required_approving_review_count"], - fix: "from 0 to 6", + fix: "required_pull_request_reviews.required_approving_review_count must be a whole number from 0 to 6 (GitHub accepts 1 to 6, or 0 to require no approvals)", }, { refused: "a negative review count", protection: { required_pull_request_reviews: { required_approving_review_count: -1 } }, paths: ["0.protection.required_pull_request_reviews.required_approving_review_count"], - fix: "from 0 to 6", + fix: "required_pull_request_reviews.required_approving_review_count must be a whole number from 0 to 6 (GitHub accepts 1 to 6, or 0 to require no approvals)", }, { // Typed in the zod shape so document validation rejects it before any section writes, not as a @@ -125,7 +134,7 @@ const cases: Array<{ refused: 'a quoted "true" for the signatures toggle, with the YAML gotcha named', protection: { enforce_admins: true, required_signatures: "true" }, paths: ["0.protection.required_signatures"], - fix: "unquoted true or false", + fix: 'required_signatures must be an unquoted true or false (YAML parses "no"/"off"/"yes" as strings, not booleans), so the toggle direction is unambiguous', }, { // The same key on a LITERAL entry stays a passthrough (the parses-clean case below). @@ -133,32 +142,44 @@ const cases: Array<{ name: "release/*", protection: { enforce_admins: true, restrictions: { users: [], teams: [] } }, paths: ["0.protection.restrictions"], - fix: ["protection.restrictions", "rulesets section"], + fix: + 'the wildcard entry "release/*" declares protection.restrictions, which this section does ' + + "not manage on wildcard rules; only the keys it can round-trip through the GraphQL rule " + + "mutations apply here: [" + + String(WILDCARD_KEYS) + + "]. For actor lists and richer controls, prefer the rulesets section (the modern successor " + + "of classic protection)", }, { refused: "a wildcard sub-key the rule mutation has no word for", name: "release/*", protection: { required_status_checks: { strict: true, checks: [] } }, paths: ["0.protection.required_status_checks.checks"], - fix: "does not manage on wildcard rules", + fix: + 'the wildcard entry "release/*" declares protection.required_status_checks.checks, which ' + + "this section does not manage on wildcard rules; only the keys it can round-trip through " + + "the GraphQL rule mutations apply here: [" + + String(WILDCARD_KEYS) + + "]. For actor lists and richer controls, prefer the rulesets section (the modern successor " + + "of classic protection)", }, { refused: "a malformed actor in a routed list", protection: { force_push_bypassers: ["a/b/c"] }, paths: ["0.protection.force_push_bypassers.0"], - fix: "bare user login", + fix: 'each force_push_bypassers actor must be a bare user login ("octocat"), "org/team-slug" for a team, or "app/slug" for a GitHub App', }, { refused: "case-insensitive duplicates in a routed actor list", protection: { force_push_bypassers: ["octocat", "OctoCat"] }, paths: ["0.protection.force_push_bypassers"], - fix: "more than once", + fix: 'force_push_bypassers lists "OctoCat" more than once (actor names are case-insensitive); keep one entry per actor', }, { refused: "case-insensitive duplicates in the required environments", protection: { required_deployments: { environments: ["prod", "Prod"] } }, paths: ["0.protection.required_deployments.environments"], - fix: "more than once", + fix: 'required_deployments.environments lists "Prod" more than once (environment names are case-insensitive); keep one entry per environment', }, ]; @@ -169,11 +190,9 @@ describe("branches protection parse rules", () => { return { path: issue.slice(0, colon), message: issue.slice(colon + 2) }; }); expect(found.map((issue) => issue.path)).toEqual(paths); - for (const issue of found) { - for (const wording of [fix].flat()) { - expect(issue.message).toContain(wording); - } - } + expect(found.map((issue) => issue.message)).toEqual( + Array.isArray(fix) ? fix : paths.map(() => fix), + ); }); // The GET expands each actor into an object; the PUT takes the login/slug string, so a copied diff --git a/test/sections/check_suite_preferences/schema.test.ts b/test/sections/check_suite_preferences/schema.test.ts index 4df5126d..3f2698dd 100644 --- a/test/sections/check_suite_preferences/schema.test.ts +++ b/test/sections/check_suite_preferences/schema.test.ts @@ -82,10 +82,8 @@ describe("a check suite preference GitHub would reject or silently overwrite nev ]), ).toEqual({ issues: [ - expect.stringMatching(APP_ID_RULE), - expect.stringMatching( - /auto_trigger_checks\[2\]\.app_id: repeats app_id 15368 from auto_trigger_checks\[0\]/, - ), + "check_suite_preferences.auto_trigger_checks[1].app_id: a GitHub App id is a positive integer (the App's settings page shows it); GitHub has no app 0 and rejects fractions", + "check_suite_preferences.auto_trigger_checks[2].app_id: repeats app_id 15368 from auto_trigger_checks[0]; GitHub would keep whichever entry it reads last and nothing reads the result back, so declare one entry per app", ], }); }); diff --git a/test/sections/collaborators/schema.test.ts b/test/sections/collaborators/schema.test.ts new file mode 100644 index 00000000..38e0d1fe --- /dev/null +++ b/test/sections/collaborators/schema.test.ts @@ -0,0 +1,35 @@ +/** + * The collaborators section's own parse refusal, pinned as the problem line a user reads: a key outside the grant + * PUT's two fields. The permission vocabulary is shared with teams and pinned once in ../roles.test.ts. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(collaborators: unknown): readonly string[] | null { + return validateSectionShapes({ collaborators }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +describe("a collaborator entry the grant PUT would silently misread is refused at parse", () => { + test.each<[what: string, entry: Record, expected: string[]]>([ + [ + "a misspelled permission key, which would grant the default role instead", + { username: "octocat", permissions: "admin" }, + [ + 'collaborators[0] (username "octocat"): declares "permissions", which this section does not ' + + 'recognize (known keys: username, permission) - a misspelled "permission" key would ' + + 'silently grant the default "push" role instead of the intended one. Fix the key name, or ' + + "remove it", + ], + ], + ])("%s", (_what, entry, expected) => { + expect(issues([entry])).toEqual(expected); + }); + + test("the two fields the PUT takes parse", () => { + expect(issues([{ username: "octocat", permission: "admin" }])).toBeNull(); + }); +}); diff --git a/test/sections/custom_properties/schema.test.ts b/test/sections/custom_properties/schema.test.ts new file mode 100644 index 00000000..68ae15f9 --- /dev/null +++ b/test/sections/custom_properties/schema.test.ts @@ -0,0 +1,58 @@ +/** + * The custom_properties section's parse refusals, each pinned as the problem line a user reads: a multi_select value + * that is an empty list (GitHub does not document whether [] stores or unsets), a repeated option (a set comparison + * would hide the typo forever), and a key outside the bulk PATCH's two fields. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(entries: unknown[]): readonly string[] | null { + return validateSectionShapes({ custom_properties: entries }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +describe("a custom property value GitHub would store ambiguously is refused at parse, naming the entry and the fix", () => { + test.each<[what: string, entry: Record, expected: string[]]>([ + [ + "an empty list, whose storage GitHub does not document", + { property_name: "team", value: [] }, + [ + 'custom_properties[0].value: the "team" entry declares an empty list; declare value: null to unset the property instead', + ], + ], + [ + "a repeated option in a multi_select value", + { property_name: "team", value: ["platform", "web", "platform"] }, + [ + 'custom_properties[0].value: the "team" entry lists the value "platform" more than once; a multi_select value is a set, so keep each option exactly once', + ], + ], + [ + "a key outside the bulk PATCH body", + { property_name: "team", value: "platform", values: ["web"] }, + [ + 'custom_properties[0] (property_name "team"): declares "values", which this section does ' + + "not recognize (known keys: property_name, value) - the key would silently never reach " + + "GitHub and the misdeclared property would keep its live value. Fix the key name, or remove " + + "it", + ], + ], + ])("%s", (_what, entry, expected) => { + expect(issues([entry])).toEqual(expected); + }); + + test("every value form GitHub stores parses: a string, a set of options, a boolean, a number, and the null that unsets", () => { + expect( + issues([ + { property_name: "team", value: "platform" }, + { property_name: "tags", value: ["web", "api"] }, + { property_name: "critical", value: true }, + { property_name: "tier", value: 2 }, + { property_name: "owner", value: null }, + ]), + ).toBeNull(); + }); +}); diff --git a/test/sections/deploy_keys/schema.test.ts b/test/sections/deploy_keys/schema.test.ts new file mode 100644 index 00000000..3481807d --- /dev/null +++ b/test/sections/deploy_keys/schema.test.ts @@ -0,0 +1,114 @@ +/** + * The deploy_keys section's parse refusals, each pinned as the problem line a user reads: the forms of key + * material GitHub's create would reject (a private key, a PEM block, a broken one-liner, a retired algorithm), and + * two entries declaring one key, which GitHub attaches to a repository once. The refusal never echoes the material: + * it may be a pasted private key. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(entries: unknown[]): readonly string[] | null { + return validateSectionShapes({ deploy_keys: entries }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +const BOT_KEY = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBotBotBotBotBotBotBotBotBotBotBotBotBotBotB"; + +/** Assembled at runtime so no source line holds the header a secret scanner flags. */ +const PRIVATE_KEY_HEADER = ["-----BEGIN", "OPENSSH PRIVATE", "KEY-----"].join(" "); + +describe("deploy key material GitHub's create would reject is refused at parse, naming the entry and the form to write", () => { + test.each<[what: string, key: string, expected: string]>([ + [ + "a private key", + `${PRIVATE_KEY_HEADER}\nb3BlbnNzaC1rZXktdjEAAAAA\n`, + 'deploy_keys[0].key: entry "ci": this is a private key; a deploy key takes the public half (the .pub file)', + ], + [ + "a PEM public key", + "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAU3ludGhldGljRml4dHVyZUJvZHlQdWJsaWM=\n-----END PUBLIC KEY-----", + 'deploy_keys[0].key: entry "ci": 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)', + ], + [ + "a YAML block scalar's trailing newline", + `${BOT_KEY}\n`, + 'deploy_keys[0].key: entry "ci": the key contains a line break (a YAML | block keeps its ' + + 'trailing newline; |- drops it); a public SSH key reads one line, " ' + + '[comment]", with the algorithm one of ssh-ed25519, ssh-rsa, ecdsa-sha2-nistp256, ' + + "ecdsa-sha2-nistp384, ecdsa-sha2-nistp521, sk-ssh-ed25519@openssh.com, " + + "sk-ecdsa-sha2-nistp256@openssh.com", + ], + [ + "a non-breaking space between the fields", + BOT_KEY.replace(" ", "\u00a0"), + 'deploy_keys[0].key: entry "ci": the key contains whitespace other than a space or tab (a ' + + 'non-breaking space, for one); a public SSH key reads one line, " ' + + '[comment]", with the algorithm one of ssh-ed25519, ssh-rsa, ecdsa-sha2-nistp256, ' + + "ecdsa-sha2-nistp384, ecdsa-sha2-nistp521, sk-ssh-ed25519@openssh.com, " + + "sk-ecdsa-sha2-nistp256@openssh.com", + ], + [ + "the algorithm alone", + "ssh-ed25519", + 'deploy_keys[0].key: entry "ci": the key has fewer than two fields separated by a space or ' + + 'tab; a public SSH key reads one line, " [comment]", with the algorithm ' + + "one of ssh-ed25519, ssh-rsa, ecdsa-sha2-nistp256, ecdsa-sha2-nistp384, " + + "ecdsa-sha2-nistp521, sk-ssh-ed25519@openssh.com, sk-ecdsa-sha2-nistp256@openssh.com", + ], + [ + "a DSA key, which GitHub retired", + "ssh-dss AAAAB3NzaC1kc3MAAACBAP1/U4E=", + 'deploy_keys[0].key: entry "ci": the key is DSA, which GitHub no longer accepts (since ' + + '2022-03-15); a public SSH key reads one line, " [comment]", with the ' + + "algorithm one of ssh-ed25519, ssh-rsa, ecdsa-sha2-nistp256, ecdsa-sha2-nistp384, " + + "ecdsa-sha2-nistp521, sk-ssh-ed25519@openssh.com, sk-ecdsa-sha2-nistp256@openssh.com", + ], + [ + "an algorithm GitHub does not take", + "ssh-ed448 AAAAC3NzaC1lZDI1NTE5AAAAIBotBotBotBotBotBotBotBotBotBotBotBotBotBotB", + 'deploy_keys[0].key: entry "ci": the key\'s first field is not an algorithm GitHub accepts; ' + + 'a public SSH key reads one line, " [comment]", with the algorithm one ' + + "of ssh-ed25519, ssh-rsa, ecdsa-sha2-nistp256, ecdsa-sha2-nistp384, ecdsa-sha2-nistp521, " + + "sk-ssh-ed25519@openssh.com, sk-ecdsa-sha2-nistp256@openssh.com", + ], + [ + "a second field that is not base64", + "ssh-ed25519 not*base64", + 'deploy_keys[0].key: entry "ci": the key\'s second field is not base64 (the alphabet A-Z a-z ' + + "0-9 + / with = padding to a multiple of four); a public SSH key reads one line, " + + '" [comment]", with the algorithm one of ssh-ed25519, ssh-rsa, ' + + "ecdsa-sha2-nistp256, ecdsa-sha2-nistp384, ecdsa-sha2-nistp521, sk-ssh-ed25519@openssh.com, " + + "sk-ecdsa-sha2-nistp256@openssh.com", + ], + ])("%s", (_what, key, expected) => { + expect(issues([{ title: "ci", key }])).toEqual([expected]); + }); + + test("an entry without a string title is named as this entry, beside the title's own type issue", () => { + expect(issues([{ title: 7, key: "ssh-ed25519" }])).toEqual([ + expect.stringContaining("deploy_keys[0].title: "), + "deploy_keys[0].key: this entry: the key has fewer than two fields separated by a space or " + + 'tab; a public SSH key reads one line, " [comment]", with the algorithm ' + + "one of ssh-ed25519, ssh-rsa, ecdsa-sha2-nistp256, ecdsa-sha2-nistp384, " + + "ecdsa-sha2-nistp521, sk-ssh-ed25519@openssh.com, sk-ecdsa-sha2-nistp256@openssh.com", + ]); + }); + + test("two entries declaring one key are refused at the second, since GitHub attaches a key to a repository once", () => { + expect( + issues([ + { title: "ci", key: BOT_KEY }, + { title: "deploy", key: `${BOT_KEY} a comment GitHub may drop` }, + ]), + ).toEqual([ + 'deploy_keys[1].key: the entries "ci" and "deploy" declare the same key material, and GitHub attaches a public key to one repository once, so the second create would be rejected - keep one entry per key', + ]); + }); + + test("a one-line public key with a comment parses", () => { + expect(issues([{ title: "ci", key: `${BOT_KEY} ci@example`, read_only: true }])).toBeNull(); + }); +}); diff --git a/test/sections/environments/schema.test.ts b/test/sections/environments/schema.test.ts new file mode 100644 index 00000000..0b7e3f50 --- /dev/null +++ b/test/sections/environments/schema.test.ts @@ -0,0 +1,124 @@ +/** + * The environments section's parse refusals, each pinned as the problem line a user reads: the PUT body rules + * GitHub 422s on (the wait timer, the reviewer cap, the two branch-policy flags) and the combinations GitHub accepts + * but never reads back (a self-review flag without reviewers, patterns without the flag, a singular `secret`, more + * pinned environments than a repository may hold). + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(entries: unknown[]): readonly string[] | null { + return validateSectionShapes({ environments: entries }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +const reviewers = (count: number) => + Array.from({ length: count }, (_, id) => ({ type: "User", id: id + 1 })); + +describe("an environment GitHub would 422, or could never converge on, is refused at parse, naming the entry and the fix", () => { + test.each<[what: string, entry: Record, expected: string[]]>([ + [ + "both branch-policy flags on", + { + name: "prod", + deployment_branch_policy: { protected_branches: true, custom_branch_policies: true }, + }, + [ + "environments[0].deployment_branch_policy: deployment_branch_policy sets both protected_branches and custom_branch_policies to true, which GitHub rejects: the two flags are mutually exclusive, so set exactly one of them to true", + ], + ], + [ + "both branch-policy flags off, which GitHub spells as null", + { + name: "prod", + deployment_branch_policy: { protected_branches: false, custom_branch_policies: false }, + }, + [ + "environments[0].deployment_branch_policy: deployment_branch_policy sets both protected_branches and custom_branch_policies to false, which GitHub rejects: GitHub spells 'any branch may deploy' as deployment_branch_policy: null, so write null", + ], + ], + [ + "a fractional wait timer", + { name: "prod", wait_timer: 1.5 }, + ["environments[0].wait_timer: wait_timer is a whole number of minutes"], + ], + [ + "a negative wait timer", + { name: "prod", wait_timer: -1 }, + ["environments[0].wait_timer: wait_timer cannot be negative; 0 declares the wait timer off"], + ], + [ + "a wait timer past GitHub's 30-day cap", + { name: "prod", wait_timer: 43_201 }, + ["environments[0].wait_timer: GitHub caps wait_timer at 43200 minutes (30 days)"], + ], + [ + "seven reviewers", + { name: "prod", reviewers: reviewers(7) }, + [ + "environments[0].reviewers: GitHub allows at most 6 required reviewers per environment; keep 6 or fewer entries", + ], + ], + [ + "a singular secret key, which the PUT would carry verbatim", + { name: "prod", secret: { name: "TOKEN", value: "$TOKEN" } }, + [ + "environments[0].secret: environment secrets belong under the entry's `secrets` list, not a singular `secret` key; here it would pass through to the environment PUT verbatim and configure nothing", + ], + ], + [ + "a self-review flag without reviewers, which GitHub drops", + { name: "prod", prevent_self_review: true }, + [ + 'environments[0].prevent_self_review: the "prod" entry declares prevent_self_review: true ' + + "without reviewers; GitHub keeps the flag only on a required-reviewers rule, which needs at " + + "least one reviewer. Declare a reviewer, or write prevent_self_review: false", + ], + ], + [ + "branch patterns without the custom-policies flag", + { name: "prod", deployment_branch_policies: [{ name: "release/*" }] }, + [ + 'environments[0].deployment_branch_policies: the "prod" entry declares deployment_branch_policies, so it must also declare deployment_branch_policy with custom_branch_policies: true - GitHub rejects every pattern write while the flag is off', + ], + ], + ])("%s", (_what, entry, expected) => { + expect(issues([entry])).toEqual(expected); + }); + + test("an eleventh pinned environment is refused at its own entry, naming GitHub's cap", () => { + const pinned = Array.from({ length: 11 }, (_, i) => ({ name: `env-${i}`, pinned: true })); + expect(issues(pinned)).toEqual([ + "environments[10].pinned: the settings file declares 11 environments with pinned: true, but GitHub allows at most 10 pinned environments per repository. Declare pinned: true on at most 10 entries", + ]); + }); + + test("an entry without a string name is named as this entry, beside the name's own type issue", () => { + expect(issues([{ name: 7, prevent_self_review: true }])).toEqual([ + expect.stringContaining("environments[0].name: "), + "environments[0].prevent_self_review: this entry declares prevent_self_review: true without " + + "reviewers; GitHub keeps the flag only on a required-reviewers rule, which needs at least " + + "one reviewer. Declare a reviewer, or write prevent_self_review: false", + ]); + }); + + test("the forms GitHub accepts parse: the caps met exactly, the flags exclusive, patterns under their flag", () => { + expect( + issues([ + { + name: "prod", + wait_timer: 43_200, + reviewers: reviewers(6), + prevent_self_review: true, + deployment_branch_policy: { protected_branches: false, custom_branch_policies: true }, + deployment_branch_policies: [{ name: "release/*" }], + secrets: [{ name: "TOKEN", value: "$TOKEN" }], + }, + { name: "staging", wait_timer: 0, deployment_branch_policy: null, pinned: true }, + ]), + ).toBeNull(); + }); +}); diff --git a/test/sections/interaction_limits/schema.test.ts b/test/sections/interaction_limits/schema.test.ts index e614f325..14e3cd7b 100644 --- a/test/sections/interaction_limits/schema.test.ts +++ b/test/sections/interaction_limits/schema.test.ts @@ -2,8 +2,8 @@ * GitHub's interaction-limit rules the platform does not enforce for us before the wire: limit and expiry are closed * enums (a typo 422s the PUT, and since the PUT re-arms on every apply the typo can never surface earlier), the PUT * body is exactly limit and expiry (a declared origin or expires_at is GitHub's read-back and diffs unequal forever), - * and max_open_pull_requests is a whole number in GitHub's 1 to 1000 range. Parsed through the loosened document - * shape, so a rule that survives here reaches the run. + * and max_open_pull_requests is a whole number in GitHub's 1 to 1000 range. Each refusal is pinned as the problem + * line a user reads. Parsed through the loosened document shape, so a rule that survives here reaches the run. */ import { describe, expect, test } from "bun:test"; @@ -18,20 +18,13 @@ function verdict(interactionLimits: unknown): { ok: true } | { issues: readonly } const LIMIT_RULE = - /^interaction_limits\.limit: limit is one of existing_users, contributors_only, collaborators_only/; + "interaction_limits.limit: limit is one of existing_users, contributors_only, collaborators_only (GitHub's interaction groups)"; const EXPIRY_RULE = - /^interaction_limits\.expiry: expiry is one of one_day, three_days, one_week, one_month, six_months/; + "interaction_limits.expiry: expiry is one of one_day, three_days, one_week, one_month, six_months (GitHub's interaction durations)"; const CAP_RULE = - /^interaction_limits\.pull_request_creation_cap\.max_open_pull_requests: max_open_pull_requests is a whole number from 1 to 1000/; + "interaction_limits.pull_request_creation_cap.max_open_pull_requests: max_open_pull_requests is a whole number from 1 to 1000 (GitHub's range)"; const KNOWN_KEYS = - "interaction_limits takes limit, expiry, pull_request_creation_cap, and pull_request_creation_bypass " + - "\\(origin and expires_at are what GitHub reports, not what it accepts\\); remove the key, or fix its spelling$"; -const unrecognized = (keys: string) => - new RegExp(`^interaction_limits: Unrecognized ${keys}; ${KNOWN_KEYS}`); -const NEEDS_LIMIT = - /^interaction_limits\.limit: expiry rides the base interaction-limits PUT, which requires a limit; declare limit alongside it, or remove expiry$/; -const NEEDS_ONE_GROUP = - /^interaction_limits: declare at least one of limit, pull_request_creation_cap, or pull_request_creation_bypass/; + "interaction_limits takes limit, expiry, pull_request_creation_cap, and pull_request_creation_bypass (origin and expires_at are what GitHub reports, not what it accepts); remove the key, or fix its spelling"; describe("an interaction limit GitHub would 422, or could never converge on, never reaches it", () => { test.each<[what: string, doc: unknown]>([ @@ -52,7 +45,7 @@ describe("an interaction limit GitHub would 422, or could never converge on, nev expect(verdict(doc)).toEqual({ ok: true }); }); - test.each<[what: string, doc: unknown, issues: RegExp[]]>([ + test.each<[what: string, doc: unknown, issues: string[]]>([ [ "a limit GitHub has no group for (422 on the re-arming PUT)", { limit: "collaborators" }, @@ -67,24 +60,34 @@ describe("an interaction limit GitHub would 422, or could never converge on, nev [ "GitHub's computed expires_at, which moves on every re-arm and would drift forever", { limit: "existing_users", expires_at: "2027-01-01T00:00:00Z" }, - [unrecognized('key: "expires_at"')], + [`interaction_limits: Unrecognized key: "expires_at"; ${KNOWN_KEYS}`], ], [ "GitHub's origin, which the PUT never accepts", { limit: "existing_users", origin: "repository" }, - [unrecognized('key: "origin"')], + [`interaction_limits: Unrecognized key: "origin"; ${KNOWN_KEYS}`], ], [ "two misspelled keys, both named in one issue", { limit: "existing_users", expiry_days: 7, pull_request_creation_caps: { enabled: true } }, - [unrecognized('keys: "expiry_days", "pull_request_creation_caps"')], + [ + `interaction_limits: Unrecognized keys: "expiry_days", "pull_request_creation_caps"; ${KNOWN_KEYS}`, + ], ], [ "an expiry without a limit, which would ride a PUT that never fires", { expiry: "one_week", pull_request_creation_cap: { enabled: true } }, - [NEEDS_LIMIT], + [ + "interaction_limits.limit: expiry rides the base interaction-limits PUT, which requires a limit; declare limit alongside it, or remove expiry", + ], + ], + [ + "an object declaring none of the three groups", + {}, + [ + "interaction_limits: declare at least one of limit, pull_request_creation_cap, or pull_request_creation_bypass (or declare interaction_limits: null to clear the base limit)", + ], ], - ["an object declaring none of the three groups", {}, [NEEDS_ONE_GROUP]], [ "a cap of zero", { pull_request_creation_cap: { enabled: true, max_open_pull_requests: 0 } }, @@ -114,27 +117,27 @@ describe("an interaction limit GitHub would 422, or could never converge on, nev 'a YAML-quoted "true" cap flag', { pull_request_creation_cap: { enabled: "true" } }, [ - /^interaction_limits\.pull_request_creation_cap\.enabled: enabled must be an unquoted true or false/, + 'interaction_limits.pull_request_creation_cap.enabled: enabled must be an unquoted true or false (YAML parses "no"/"off"/"yes" as strings, not booleans), so the cap direction is unambiguous', ], ], [ "a bypass list over GitHub's 100-user cap", { pull_request_creation_bypass: Array.from({ length: 101 }, (_, i) => `user-${i}`) }, [ - /^interaction_limits\.pull_request_creation_bypass: GitHub caps the bypass list at 100 users, but 101 logins are declared/, + "interaction_limits.pull_request_creation_bypass: GitHub caps the bypass list at 100 users, but 101 logins are declared; trim the list", ], ], [ "two case-variant spellings of one login", { pull_request_creation_bypass: ["octocat", "Octocat"] }, [ - /^interaction_limits\.pull_request_creation_bypass: "octocat" and "Octocat" name the same login/, + 'interaction_limits.pull_request_creation_bypass: "octocat" and "Octocat" name the same login (logins are case-insensitive); keep exactly one', ], ], ])( "what GitHub rejects fails at parse, naming the key and the rule: %s", (_what, doc, issues) => { - expect(verdict(doc)).toEqual({ issues: issues.map((issue) => expect.stringMatching(issue)) }); + expect(verdict(doc)).toEqual({ issues }); }, ); diff --git a/test/sections/labels/schema.test.ts b/test/sections/labels/schema.test.ts index 2b9a6fc0..b3bf8628 100644 --- a/test/sections/labels/schema.test.ts +++ b/test/sections/labels/schema.test.ts @@ -28,12 +28,12 @@ describe("a label the API would reject never reaches it", () => { ])("a color GitHub 422s fails at parse naming the entry and the rule: %s", (_what, color) => { expect(verdict({ name: "bug", color })).toEqual({ issues: [ - expect.stringMatching(/^labels\[0\]\.color: .*six hex digits.*leading "#" optional/), + 'labels[0].color: a label color is six hex digits, the leading "#" optional ("#d73a4a" or "d73a4a"); color names and three-digit shorthand are not accepted', ], }); }); - test.each<[what: string, description: unknown, issues: RegExp[] | null]>([ + test.each<[what: string, description: unknown, issues: (string | RegExp)[] | null]>([ ["100 ASCII characters, at the cap", "x".repeat(100), null], [ "100 emoji: the cap counts code points, as JSON Schema maxLength does", @@ -43,12 +43,16 @@ describe("a label the API would reject never reaches it", () => { [ "101 ASCII characters", "x".repeat(101), - [/^labels\[0\]\.description: .*100 characters.*this one has 101$/], + [ + "labels[0].description: a label description is at most 100 characters (GitHub's cap); this one has 101", + ], ], [ "101 emoji, the count shown in code points too", "\u{1F600}".repeat(101), - [/^labels\[0\]\.description: .*100 characters.*this one has 101$/], + [ + "labels[0].description: a label description is at most 100 characters (GitHub's cap); this one has 101", + ], ], [ "a mapping with a length key: zod would run the cap check on it, so only the type error may report it", @@ -66,7 +70,11 @@ describe("a label the API would reject never reaches it", () => { expect(verdict({ name: "bug", description })).toEqual( issues === null ? { ok: true } - : { issues: issues.map((issue) => expect.stringMatching(issue)) }, + : { + issues: issues.map((issue) => + typeof issue === "string" ? issue : expect.stringMatching(issue), + ), + }, ); }, ); diff --git a/test/sections/list-wrapper-schema.test.ts b/test/sections/list-wrapper-schema.test.ts new file mode 100644 index 00000000..2b83dc3f --- /dev/null +++ b/test/sections/list-wrapper-schema.test.ts @@ -0,0 +1,98 @@ +/** + * The refusals every list section shares, pinned once as the problem lines a user reads: a wrapper carrying a key + * that is not one of its directives (with the pre-v3 policy spelling naming its rename), a section value that is + * neither a list nor a wrapper, and a YAML-tagged value where a mapping section expects a plain mapping. The + * wording lives in src/sections/shared/schema-helpers.ts, renamed-key.ts, and contract/module.ts. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../src/engine/validate.js"; +import { repositorySection } from "../../src/sections/repository/index.js"; + +function issues(doc: Record): readonly string[] | null { + return validateSectionShapes(doc, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +const KNOBBED_DIRECTIVES = + 'the wrapper\'s directives are "_undeclared" and, on a top-level section, "_layering", and nothing else - there are no private-note keys. Remove the key, or keep the note as a YAML comment'; + +const RENAMED = + 'the wrapper\'s policy key "undeclared" was renamed to "_undeclared" in v3 (a directive, like _layering) - write _undeclared: keep or _undeclared: delete'; + +describe("a list wrapper carrying a key outside its directives is refused at parse, naming the directives and the fix", () => { + test.each<[what: string, doc: Record, expected: string[]]>([ + [ + "a private note on a knobbed section's wrapper", + { labels: { _notes: "owned by platform", entries: [{ name: "bug" }] } }, + [`labels: Unrecognized key: "_notes"; ${KNOBBED_DIRECTIVES}`], + ], + [ + "a private note beside a misspelled entries key: the clause names the underscore key it is about", + { labels: { _notes: "owned by platform", entires: [], entries: [] } }, + [`labels: Unrecognized keys: "_notes", "entires"; "_notes": ${KNOBBED_DIRECTIVES}`], + ], + [ + "the pre-v3 policy spelling", + { labels: { undeclared: "keep", entries: [] } }, + [`labels: Unrecognized key: "undeclared"; ${RENAMED}`], + ], + [ + "the pre-v3 policy spelling beside a private note: both fixes in one run", + { labels: { undeclared: "keep", _owner: "platform", entries: [] } }, + [ + `labels: Unrecognized keys: "undeclared", "_owner"; ${RENAMED}; "_owner": ${KNOBBED_DIRECTIVES}`, + ], + ], + [ + "a policy on a plain list's wrapper, which applies none", + { branches: { _undeclared: "keep", entries: [] } }, + [ + 'branches: Unrecognized key: "_undeclared"; the wrapper\'s directives are "_layering" alone ' + + '(this section applies no undeclared policy, so its wrapper takes no "_undeclared"), and ' + + "nothing else - there are no private-note keys. Remove the key, or keep the note as a YAML " + + "comment", + ], + ], + [ + "the layering directive on a nested list, which has no layers below it", + { environments: [{ name: "prod", variables: { _layering: "deep", entries: [] } }] }, + [`environments[0].variables: Unrecognized key: "_layering"; ${KNOBBED_DIRECTIVES}`], + ], + [ + "a scalar where a knobbed section's list or wrapper goes", + { labels: 5 }, + [ + 'labels: Invalid input: expected a list of entries, or a mapping with "entries" (and an optional "_undeclared" policy), but this section parsed as number', + ], + ], + [ + "a scalar where a plain list section's list or wrapper goes", + { branches: "main" }, + [ + 'branches: Invalid input: expected a list of entries, or a mapping with "entries" (and an optional "_layering" directive), but this section parsed as string', + ], + ], + ])("%s", (_what, doc, expected) => { + expect(issues(doc)).toEqual(expected); + }); + + test("both wrapper forms with their own directives parse", () => { + expect( + issues({ + labels: { _undeclared: "delete", _layering: "shallow", entries: [{ name: "bug" }] }, + branches: { _layering: "replace", entries: [] }, + environments: [{ name: "prod", variables: { _undeclared: "keep", entries: [] } }], + }), + ).toBeNull(); + }); + + test("a mapping section's shape, parsed directly by a library caller, refuses a YAML-tagged value by name; the engine refuses it earlier with its own plainness line", () => { + const parsed = repositorySection.shape.safeParse(new Date(0)); + expect(parsed.success ? [] : parsed.error.issues.map((issue) => issue.message)).toEqual([ + "Invalid input: expected a plain mapping (a YAML-tagged value like !!timestamp parses to another type)", + ]); + }); +}); diff --git a/test/sections/milestones/scenarios/milestones-invalid-state-rejected.yml b/test/sections/milestones/scenarios/milestones-invalid-state-rejected.yml new file mode 100644 index 00000000..37b68bdc --- /dev/null +++ b/test/sections/milestones/scenarios/milestones-invalid-state-rejected.yml @@ -0,0 +1,18 @@ +# A milestone state outside GitHub's two (open, closed) is refused when the +# settings file is parsed, naming the entry and the accepted values, before +# any API contact: the create would 422 at apply time with every other +# milestone unwritten. +name: milestones-invalid-state-rejected +settings: + milestones: + - title: v1.0 + state: paused +inputs: + mode: apply +expect: + exit_code: 1 + result: failed + zero_requests: true + stdout_contains: + - "milestones[0].state" + - '"open"|"closed"' diff --git a/test/sections/milestones/schema.test.ts b/test/sections/milestones/schema.test.ts new file mode 100644 index 00000000..6e82d75b --- /dev/null +++ b/test/sections/milestones/schema.test.ts @@ -0,0 +1,32 @@ +/** + * The milestones section's parse refusal, pinned as the problem line a user reads: a due date that is neither a + * calendar day nor the timestamp GitHub keeps on one. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(entries: unknown[]): readonly string[] | null { + return validateSectionShapes({ milestones: entries }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +describe("a milestone due date GitHub would 422 is refused at parse, naming the key and the forms", () => { + test.each<[what: string, due_on: unknown]>([ + ["a day-month-year spelling", "01-10-2026"], + ["a number", 20261001], + ])("%s", (_what, due_on) => { + expect(issues([{ title: "v1", due_on }])).toEqual([ + "milestones[0].due_on: due_on is a calendar day, YYYY-MM-DD (or an ISO 8601 UTC timestamp, YYYY-MM-DDTHH:MM:SSZ, whose time GitHub discards)", + ]); + }); + + test.each<[what: string, due_on: string]>([ + ["a calendar day", "2026-10-01"], + ["the timestamp GitHub keeps on that day", "2026-10-01T07:00:00Z"], + ])("%s parses", (_what, due_on) => { + expect(issues([{ title: "v1", due_on }])).toBeNull(); + }); +}); diff --git a/test/sections/pages/schema.test.ts b/test/sections/pages/schema.test.ts new file mode 100644 index 00000000..b17b2280 --- /dev/null +++ b/test/sections/pages/schema.test.ts @@ -0,0 +1,70 @@ +/** + * The pages section's parse refusals, each pinned as the problem line a user reads: a field the GET reports that the + * update PUT has no parameter for, so a declared value would ride the PUT ignored and diff on every run. One row per + * read-only field, each naming why removing it loses nothing. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(pages: unknown): readonly string[] | null { + return validateSectionShapes({ pages }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +const REPORTED = (fix: string) => + `GitHub reports this field on the Pages site and the update has no such parameter, so the value would be sent, ignored, and reported as drift on every run (${fix}); remove it`; + +describe("a Pages field the update cannot set is refused at parse, naming the key and why it can go", () => { + test.each<[key: string, value: unknown, fix: string]>([ + [ + "url", + "https://api.github.com/repos/o/r/pages", + "GitHub mints the API address from the repository", + ], + [ + "html_url", + "https://o.github.io/r/", + "GitHub mints the site address from the repository and `cname`; declare `cname` for a custom domain", + ], + ["status", "built", "it reports the latest build's outcome"], + [ + "custom_404", + false, + "it reports whether the published site carries a 404.html; add that file to the source instead", + ], + [ + "protected_domain_state", + "verified", + "it reports the custom domain's verification; verify the domain in the owner's Pages settings", + ], + [ + "pending_domain_unverified_at", + "2026-10-01T00:00:00Z", + "it reports the custom domain's verification deadline", + ], + [ + "https_certificate", + { state: "approved" }, + "GitHub provisions the certificate for `cname`; declare `cname` and `https_enforced`", + ], + ])("%s", (key, value, fix) => { + expect(issues({ build_type: "workflow", [key]: value })).toEqual([ + `pages.${key}: ${REPORTED(fix)}`, + ]); + }); + + test("the fields the update takes parse, and null turns the site off", () => { + expect( + issues({ + build_type: "legacy", + source: { branch: "gh-pages", path: "/docs" }, + cname: "docs.example.com", + https_enforced: true, + }), + ).toBeNull(); + expect(issues(null)).toBeNull(); + }); +}); diff --git a/test/sections/refusal-messages.test.ts b/test/sections/refusal-messages.test.ts new file mode 100644 index 00000000..3477d4dc --- /dev/null +++ b/test/sections/refusal-messages.test.ts @@ -0,0 +1,1183 @@ +/** + * Refusal messages are user experience: every sentence a user can read when the settings file is refused at parse + * time is pinned by a test that spells it, so a wording change is a deliberate test change and a new refusal cannot + * ship unpinned. The message literals are read off the source AST (oxc-parser, walked with estree-walker), never by + * regex over source text, and each must appear, cooked, inside a string of some test under test/ (a .ts literal or + * template, or a scenario's expect lists). The failure names the source file, the line, and the literal. + * + * Every string literal in a section schema slice, a shared schema helper, or compilable-form.ts is a refusal message + * unless its position is in the listed exclusions (DATA_CONSTANTS by name, keys, specifiers, type positions, + * vocabulary calls); in section modules, module.ts, repo-secrets.ts, and problem.ts, every literal inside a + * message-shaped object ({path, message}), an error:/consequence: property, or an issue-builder body is a message. + * No reach or helper analysis: a literal cannot hide from a file scan, so a spelling the census does not follow is + * not a class it has to learn. + */ + +import { describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { walk as walkTree } from "estree-walker"; +import { parseSync } from "oxc-parser"; +import { parse as parseYaml } from "yaml"; +import { ROOT } from "../root.js"; +import { withTempDir } from "../temp-dir.js"; + +/** The shape every oxc node shares; estree-walker finds the children by the fields that carry a `type`. */ +interface Node { + readonly type: string; + readonly start: number; + readonly end: number; +} + +interface TemplateElement extends Node { + readonly value: { readonly cooked: string | null; readonly raw: string }; +} + +/** How a source file is read: every literal (a slice, a helper), or only its message positions (a module). */ +type Sites = "every-literal" | "message-positions"; + +export interface Source { + readonly path: string; + readonly text: string; + readonly sites: Sites; +} + +/** + * One message literal. A whole literal that is not prose (a terse word) is pinned only as the whole message of a + * rendered line; a piece of a template or a `+` chain, and prose, by containment. + */ +interface Fragment { + readonly path: string; + readonly line: number; + readonly text: string; + readonly whole: boolean; +} + +/** Prose: long enough to be a clause and holding a space. A shorter whole literal is a terse message, pinned whole. */ +const MIN_PROSE_LENGTH = 16; + +const INVARIANT_PREFIX = "BUG:"; + +/** + * The exclusions, each a syntactic position a string literal can sit in and not be a message; the planted control + * below carries one literal per position. Every other literal of a literal-first source is a message. + * + * property key `{ "error": ... }`, the key itself + * module specifier `import { z } from "zod"` + * type position a literal type, an annotation + * vocabulary call argument 0 of `z.enum([...])`, `z.literal("web")`, `.default("branch")`, `.includes("")` + * value method `.split(",")`, `.join(", ")`, `.startsWith("-----BEGIN")`, `new RegExp("...")`, `new Set([...])` + * comparison or case `name === ""`, `case "boolean":` + * data field `path: ["key"]`, `code: "custom"`, `id: "LabelConfig"`, `route: "GET ..."` + * as const vocabulary `["a", "b"] as const`, reached through arrays, objects, and properties only + * schema twin the argument of `.meta({...})` or `conditional(...)`, a JSON Schema mirror of a refinement + * regex source a `String.raw` template + * invariant a "BUG:" message for the developer holding the stack + * DATA_CONSTANTS a named constant holding a key, a pattern piece, or a vocabulary; prose inside one is a message + * DATA_FUNCTIONS a function that assembles regex source + * DATA_ARGUMENTS a data argument of a local factory (`bareRule("creation")`) + */ +const VOCABULARY_CALLS: ReadonlySet = new Set([ + "enum", + "discriminatedUnion", + "literal", + "default", + "prefault", + "catch", + "describe", + "meta", + "brand", + "includes", + "startsWith", + "endsWith", +]); + +const VALUE_METHODS: ReadonlySet = new Set([ + "split", + "join", + "replace", + "replaceAll", + "indexOf", + "lastIndexOf", + "padStart", + "padEnd", + "repeat", + "has", + "get", + "hasOwn", + "test", + "exec", + "encode", + "RegExp", + "Set", + "Map", + "Error", +]); + +/** The fields of an issue, a route table, or an actor table that carry data, never text a user reads as a message. */ +const DATA_FIELDS: ReadonlySet = new Set([ + "path", + "key", + "code", + "id", + "kind", + "type", + "route", + "field", + "nameKey", + "example", + "form", + "syntax", + "params", + "statuses", + "phase", + "accessGrade", + "notFound", + "primaryRead", + "check", + "text", +]); + +/** The calls whose object argument is a JSON Schema twin of a refinement: keys and enum values, never text. */ +const SCHEMA_TWIN_CALLS: ReadonlySet = new Set(["meta", "conditional"]); + +/** The functions of compilable-form.ts that assemble regex source: every literal in them is a pattern piece. */ +const DATA_FUNCTIONS: ReadonlySet = new Set(["render", "codePointEscape"]); + +/** + * The local factories and helpers whose string arguments are data (a key name, a rule type, a failure code, a size + * measure, a definition id), by argument index, and those whose string arguments are pieces a longer message is + * composed from (pinned by containment). Auditable by name; a control below fails naming an entry no source declares + * or imports. + */ +const DATA_ARGUMENTS: Readonly> = { + actorList: [0, 1], + reviewActorHolder: [0], + bareRule: [0], + rule: [0], + patternRule: [0], + ruleId: [0], + sectionFailure: [0], + boundedString: [1], + sealedSecretConfig: [0], + variableConfig: [0], +}; + +const PIECE_ARGUMENTS: Readonly> = { + closedKeyError: [0], + identifiedBy: [2], + duplicateFieldIssues: [2], + duplicateIssues: [2], + duplicateVariableNameIssues: [1], + duplicateSecretNameIssues: [1], + holderError: [0], + commitMessageFamily: [0, 1], + githubName: [0], + renamedKeyError: [0, 1, 2, 3], +}; + +/** + * The named constants of the literal-first sources whose string literals are data, not messages: key names, + * vocabularies, and pattern pieces. A prose-length literal inside one is still a message. Auditable: every entry is + * a name a reader can open, and a control below fails naming one no source declares. + */ +const DATA_CONSTANTS: ReadonlySet = new Set([ + // shared/schema-helpers.ts + "UNDECLARED_POLICIES", + "LAYERINGS", + // shared/setup-schema.ts + "GET_ONLY_KEYS", + // shared/roles.ts + "ROLE_FOR_PERMISSION", + "STANDARD_PERMISSIONS", + "INVITATION_ROLES", + "DEFAULT_ROLE", + // branches/schema.ts + "ACTOR_LIST_EXAMPLE", + "PROTECTION_MAPPING_KEYS", + // deploy_keys/schema.ts + "PUBLIC_KEY_ALGORITHMS", + "BASE64_QUARTET", + "BASE64_BLOB", + "FIELD_SEPARATOR", + "PRIVATE_KEY_FRAMING", + // interaction_limits/schema.ts + "INTERACTION_GROUPS", + "INTERACTION_EXPIRIES", + // repository/schema.ts + "GET_ONLY_KEYS", + "SECTION_OWNED_KEYS", + "REVIEWER_TYPES", + "REVIEWER_MODES", + "COMMIT_MESSAGE_VOCABULARIES", + "SQUASH_COMMIT_PAIRS", + "TOPIC_GRAMMAR", + "CREATION_POLICIES", + // rulesets/schema.ts + "REF_NAME_TOKENS", + "REF_NAME_ILLEGAL", + "BYPASS_ACTOR_TYPES", + "IDENTIFIED_ACTOR_TYPES", + "PATTERN_OPERATORS", + // actions/schema.ts + "REPORTED_ONLY", + // code_quality_setup, code_scanning_default_setup + "CODE_QUALITY_LANGUAGES", + "CODE_SCANNING_LANGUAGES", + // compilable-form.ts + "GROUP_NAME", + "GROUP_REWRITES", + "CODE_POINT_ESCAPES", + // secret_scanning_custom_patterns/schema.ts + "REGEX_SYNTAX", +]); + +/** A list section's `noun:` at the top level of its declaration names the resource in its duplicate refusal ("names the same label as"). */ +const MODULE_NOUN_KEY = "noun"; + +/** The property keys whose value is a message: zod's `error`, an issue's `message`, the closed-surface `consequence`. */ +const MESSAGE_KEYS: ReadonlySet = new Set(["message", "error", "consequence", "legal"]); + +/** The issue builders of src/problem.ts, by name; their bodies are message positions. */ +const ISSUE_BUILDER = /Issue$/; + +const FUNCTION_TYPES: ReadonlySet = new Set([ + "FunctionDeclaration", + "FunctionExpression", + "ArrowFunctionExpression", +]); + +/** The test functions whose first argument is a title, not a string a user reads. */ +const TEST_FUNCTIONS: ReadonlySet = new Set(["test", "describe", "it"]); + +const SHARED_REFUSAL_HELPERS = [ + "schema-helpers", + "setup-schema", + "roles", + "renamed-key", + "raw-values", +]; + +function field(node: Node, name: string): T { + return (node as unknown as Record)[name] as T; +} + +/** The visitor's view of a step: the node, its parent, and the parent's field it sits in. */ +interface Step { + readonly node: Node; + readonly parent: Node | null; + readonly key: string | undefined; +} + +/** Depth-first over the tree; `enter` returns false to leave a subtree unread. */ +function walk( + root: Node, + enter: (step: Step) => boolean | undefined, + leave?: (node: Node) => void, +): void { + walkTree(root as never, { + enter(node, parent, key) { + const step: Step = { + node: node as unknown as Node, + parent: parent as unknown as Node | null, + key: typeof key === "string" ? key : undefined, + }; + if (enter(step) === false) { + this.skip(); + } + }, + leave(node) { + leave?.(node as unknown as Node); + }, + }); +} + +function isProse(text: string): boolean { + return text.length >= MIN_PROSE_LENGTH && text.includes(" "); +} + +function lineOf(text: string, offset: number): number { + return text.slice(0, offset).split("\n").length; +} + +function isStringLiteral(node: Node): boolean { + return node.type === "Literal" && typeof field(node, "value") === "string"; +} + +function isConcatenation(node: Node): boolean { + return node.type === "BinaryExpression" && field(node, "operator") === "+"; +} + +/** The name of a non-computed property key, or of a member expression's property. */ +function keyName(node: Node): string | undefined { + if (field(node, "computed") === true) { + return undefined; + } + const key = field(node, node.type === "Property" ? "key" : "property"); + if (key.type === "Identifier") { + return field(key, "name"); + } + return isStringLiteral(key) ? field(key, "value") : undefined; +} + +/** The method a call invokes (`enum` in `z.enum(...)`, `RegExp` in `new RegExp(...)`), or undefined. */ +function calledName(call: Node): string | undefined { + const callee = field(call, "callee"); + if (callee.type === "MemberExpression") { + return keyName(callee); + } + return callee.type === "Identifier" ? field(callee, "name") : undefined; +} + +/** The name a variable declarator binds, when it is one identifier. */ +function declaredName(declarator: Node): string | undefined { + const id = field(declarator, "id"); + return id.type === "Identifier" ? field(id, "name") : undefined; +} + +/** + * Why a literal at this step is not a message, or undefined when it is one. `ancestors` is the chain from the root + * down to the literal's parent. + */ +function exclusion(step: Step, ancestors: readonly Step[]): string | undefined { + const { parent, key } = step; + if (parent === null) { + return undefined; + } + let twin = false; + let constVocabulary = false; + if (parent.type === "Property" && key === "key") { + return "property key"; + } + if (parent.type.startsWith("Import") || parent.type.startsWith("Export")) { + return "module specifier"; + } + if (parent.type === "TaggedTemplateExpression" && isStringRaw(field(parent, "tag"))) { + return "regex source"; + } + if (parent.type === "SwitchCase") { + return "case label"; + } + // The receiver of a value method (`"(|^$".includes(x)`, `["a", "b"].join(", ")`) is data, whatever the argument. + if (parent.type === "MemberExpression" && key === "object") { + const method = field(parent, "property"); + const name = method.type === "Identifier" ? field(method, "name") : undefined; + if (name !== undefined && (VALUE_METHODS.has(name) || VOCABULARY_CALLS.has(name))) { + return `value method receiver ${name}`; + } + } + if ( + parent.type === "BinaryExpression" && + ["===", "!==", "==", "!=", "in"].includes(field(parent, "operator")) + ) { + return "comparison"; + } + if (parent.type === "Property" && key === "value" && DATA_FIELDS.has(keyName(parent) ?? "")) { + return `data field ${keyName(parent)}`; + } + ancestors.forEach(({ node }, index) => { + // The step below this ancestor: the literal's own step when the ancestor is the parent. + const below = ancestors[index + 1] ?? step; + if ( + (node.type === "CallExpression" || node.type === "NewExpression") && + SCHEMA_TWIN_CALLS.has(calledName(node) ?? "") && + below.key === "arguments" + ) { + twin = true; + } + }); + if (twin) { + return "schema twin"; + } + ancestors.forEach(({ node }, index) => { + if (node.type === "TSAsExpression" && isConstAssertion(node)) { + const between = ancestors.slice(index + 1).map(({ node: inner }) => inner.type); + if ( + between.every((type) => + ["ArrayExpression", "ObjectExpression", "Property"].includes(type), + ) && + !holdsProse(step.node) + ) { + constVocabulary = true; + } + } + }); + if (constVocabulary) { + return "as const vocabulary"; + } + for (const { node } of ancestors) { + if ( + node.type.startsWith("TS") && + ![ + "TSAsExpression", + "TSSatisfiesExpression", + "TSTypeAssertion", + "TSNonNullExpression", + ].includes(node.type) + ) { + return "type position"; + } + if ( + node.type === "VariableDeclarator" && + DATA_CONSTANTS.has(declaredName(node) ?? "") && + !holdsProse(step.node) + ) { + return `data constant ${declaredName(node)}`; + } + if (node.type === "FunctionDeclaration" && DATA_FUNCTIONS.has(functionName(node) ?? "")) { + return `data function ${functionName(node)}`; + } + } + // The literal's holder past any array nesting: a data field, or the call it is an argument of (directly, or as + // the receiver of a value method such as `["a", "b"].join(" ")`). + let depth = ancestors.length - 1; + while (depth >= 0 && ancestors[depth]?.node.type === "ArrayExpression") { + depth -= 1; + } + const holder = ancestors[depth]; + if (holder === undefined) { + return undefined; + } + if (holder.node.type === "Property" && DATA_FIELDS.has(keyName(holder.node) ?? "")) { + return `data field ${keyName(holder.node)}`; + } + const call = + holder.node.type === "MemberExpression" && ancestors[depth - 1]?.node.type === "CallExpression" + ? ancestors[depth - 1]?.node + : holder.node; + if (call === undefined || (call.type !== "CallExpression" && call.type !== "NewExpression")) { + return undefined; + } + const name = calledName(call) ?? ""; + const argument = ancestors[depth + 1]?.node ?? step.node; + const index = field(call, "arguments").indexOf(argument); + if (VOCABULARY_CALLS.has(name) && index === 0) { + return `vocabulary call ${name}`; + } + if (VALUE_METHODS.has(name)) { + return `value method ${name}`; + } + if (DATA_ARGUMENTS[name]?.includes(index)) { + return `data argument ${name}[${index}]`; + } + return undefined; +} + +/** Whether a literal (or any piece of a template) is prose: a vocabulary word never is, so prose in a table is a message. */ +function holdsProse(node: Node): boolean { + return textsOf(node).some(({ text }) => isProse(text)); +} + +/** `String.raw`: the one tag that marks a regex source; any other tag in a message position is a message. */ +function isStringRaw(tag: Node): boolean { + return ( + tag.type === "MemberExpression" && + field(tag, "object").type === "Identifier" && + field(field(tag, "object"), "name") === "String" && + keyName(tag) === "raw" + ); +} + +function functionName(declaration: Node): string | undefined { + const id = field(declaration, "id"); + return id === null ? undefined : field(id, "name"); +} + +/** `[...] as const`: a vocabulary the schema enumerates, never text. */ +function isConstAssertion(node: Node): boolean { + const annotation = field(node, "typeAnnotation"); + if (annotation.type !== "TSTypeReference") { + return false; + } + const name = field(annotation, "typeName"); + return name.type === "Identifier" && field(name, "name") === "const"; +} + +/** Whether the literal is an argument a registered helper composes into a longer message: pinned by containment. */ +function isPieceArgument(step: Step): boolean { + const { parent, node } = step; + if (parent?.type !== "CallExpression") { + return false; + } + const index = field(parent, "arguments").indexOf(node); + return PIECE_ARGUMENTS[calledName(parent) ?? ""]?.includes(index) ?? false; +} + +/** A literal's message texts: the literal, or the non-blank pieces of a template; a "BUG:" opening is an invariant. */ +function textsOf(node: Node): { start: number; text: string; whole: boolean }[] { + if (isStringLiteral(node)) { + const value = field(node, "value"); + if (value.trim() === "" || value.startsWith(INVARIANT_PREFIX)) { + return []; + } + return [{ start: node.start, text: value, whole: !isProse(value) }]; + } + const quasis = field(node, "quasis"); + const expressions = field(node, "expressions"); + const cooked = (quasi: TemplateElement) => quasi.value.cooked ?? quasi.value.raw; + if ((cooked(quasis[0] as TemplateElement) ?? "").startsWith(INVARIANT_PREFIX)) { + return []; + } + if (expressions.length === 0) { + const value = cooked(quasis[0] as TemplateElement); + return value.trim() === "" ? [] : [{ start: node.start, text: value, whole: !isProse(value) }]; + } + return quasis + .filter((quasi) => /[^\s()[\]{}.,;:]/.test(cooked(quasi))) + .map((quasi) => ({ start: quasi.start, text: cooked(quasi), whole: false })); +} + +/** Every message literal under `root`, the exclusions applied at each. */ +function literalsUnder( + root: Node, + path: string, + text: string, + into: Map, + asPiece = false, +): void { + const ancestors: Step[] = []; + walk( + root, + (step) => { + const { node, parent } = step; + if (isStringLiteral(node) || node.type === "TemplateLiteral") { + const why = exclusion(step, ancestors); + if (why === undefined) { + // A piece of a longer message, pinned by containment: an operand of a `+` chain, a literal inside a + // template hole, an arm of a conditional, a helper's return, a registered piece argument. + const piece = + asPiece || + ancestors.some( + ({ node: ancestor }) => + isConcatenation(ancestor) || ancestor.type === "TemplateLiteral", + ) || + parent?.type === "ConditionalExpression" || + parent?.type === "ReturnStatement" || + isPieceArgument(step); + for (const { start, text: value, whole } of textsOf(node)) { + if (piece && !/[^\s()[\]{}.,;:]/.test(value)) { + continue; + } + if (!into.has(start)) { + into.set(start, { + path, + line: lineOf(text, start), + text: value, + whole: whole && !piece, + }); + } + } + } + if (node.type === "TemplateLiteral") { + // The holes are read too: a literal inside one is a piece of this message. + ancestors.push(step); + return undefined; + } + return false; + } + ancestors.push(step); + return undefined; + }, + (node) => { + if (ancestors[ancestors.length - 1]?.node === node) { + ancestors.pop(); + } + }, + ); +} + +/** + * A module's message positions: an object literal carrying a `message` property (an issue), an `error` or + * `consequence` property's value, and the value of a top-level constant an `error`/`message` property names (a + * message function such as WILDCARD_KEY_ERROR). Everything else in a module is plan text and outcomes, out of this + * census by the owner's ruling (parse refusals only). + */ +function messagePositions(program: Node): { positions: Node[]; pieces: Node[] } { + const positions: Node[] = []; + const pieces: Node[] = []; + const named = new Set(); + const locals: Node[] = []; + let functionDepth = 0; + // A live shape (`LiveDeployKey`, the repository's naming for a GET body) reports a response outside the documented + // API shape, never a settings-file refusal; a transform anywhere else (the routed list shape) is a parse position. + const liveSpans: Node[] = []; + walk(program, ({ node }) => { + if (node.type === "VariableDeclarator" && /^Live/.test(declaredName(node) ?? "")) { + liveSpans.push(node); + } + return undefined; + }); + walk( + program, + ({ node }) => { + if ( + node.type === "CallExpression" && + calledName(node) === "transform" && + liveSpans.some((span) => span.start <= node.start && node.end <= span.end) + ) { + return false; + } + // A registered helper's piece argument (`identifiedBy(key, field, "workflow")`), and a top-level `noun:`. + if (node.type === "CallExpression") { + const indexes = PIECE_ARGUMENTS[calledName(node) ?? ""] ?? []; + const args = field(node, "arguments"); + for (const index of indexes) { + const argument = args[index]; + if (argument !== undefined) { + pieces.push(argument); + } + } + } + if (node.type === "Property" && keyName(node) === MODULE_NOUN_KEY && functionDepth === 0) { + pieces.push(field(node, "value")); + } + if (FUNCTION_TYPES.has(node.type)) { + functionDepth += 1; + } + if (node.type === "VariableDeclarator") { + locals.push(node); + } + if (node.type === "Property" && MESSAGE_KEYS.has(keyName(node) ?? "")) { + const value = field(node, "value"); + positions.push(value); + walk(value, ({ node: inner }) => { + if (inner.type === "Identifier") { + named.add(field(inner, "name")); + } + return undefined; + }); + } + return undefined; + }, + (node) => { + if (FUNCTION_TYPES.has(node.type)) { + functionDepth -= 1; + } + }, + ); + // The constants a message names, at the top level or local to the enclosing function (`beside`), one hop. + for (const declarator of locals) { + if (named.has(declaredName(declarator) ?? "")) { + positions.push(declarator); + } + } + for (const statement of field(program, "body")) { + const declaration = + statement.type === "ExportNamedDeclaration" + ? field(statement, "declaration") + : statement; + if (declaration?.type === "FunctionDeclaration") { + const name = field(field(declaration, "id"), "name"); + if (ISSUE_BUILDER.test(name) || named.has(name)) { + positions.push(declaration); + } + } + } + return { positions, pieces }; +} + +/** A message position reaches the top-level constants and functions it names, one hop and no further. */ +function builderConstants(program: Node, builders: readonly Node[]): Node[] { + const named = new Set(); + for (const builder of builders) { + walk(builder, ({ node }) => { + if (node.type === "Identifier") { + named.add(field(node, "name")); + } + return undefined; + }); + } + const constants: Node[] = []; + for (const statement of field(program, "body")) { + const declaration = + statement.type === "ExportNamedDeclaration" + ? field(statement, "declaration") + : statement; + if (declaration?.type === "VariableDeclaration") { + for (const declarator of field(declaration, "declarations")) { + if (named.has(declaredName(declarator) ?? "") && !builders.includes(declarator)) { + constants.push(declarator); + } + } + } + if ( + declaration?.type === "FunctionDeclaration" && + named.has(functionName(declaration) ?? "") && + !builders.includes(declaration) + ) { + constants.push(declaration); + } + } + return constants; +} + +function parse(path: string, text: string): Node { + const { program, errors } = parseSync(path, text); + if (errors.length > 0) { + throw new Error(`${path} does not parse, so its strings cannot be read: ${errors[0]?.message}`); + } + return program as unknown as Node; +} + +/** Every message literal of the sources, in file order. */ +export function messageFragments(sources: Iterable): Fragment[] { + const fragments: Fragment[] = []; + for (const { path, text, sites } of sources) { + const program = parse(path, text); + const found = new Map(); + if (sites === "every-literal") { + literalsUnder(program, path, text, found); + } else { + const { positions, pieces } = messagePositions(program); + for (const site of [...positions, ...builderConstants(program, positions)]) { + literalsUnder(site, path, text, found); + } + for (const piece of pieces) { + literalsUnder(piece, path, text, found, true); + } + } + fragments.push(...[...found.entries()].sort(([a], [b]) => a - b).map(([, f]) => f)); + } + return fragments; +} + +/** `test.each(rows)` is not a title call: its argument is the table, whose rows are assertions. */ +function isTestCall(call: Node): boolean { + let callee = field(call, "callee"); + if (callee.type === "MemberExpression" && keyName(callee) === "each") { + return false; + } + while (callee.type === "CallExpression" || callee.type === "MemberExpression") { + callee = field(callee, callee.type === "CallExpression" ? "callee" : "object"); + } + return callee.type === "Identifier" && TEST_FUNCTIONS.has(field(callee, "name")); +} + +/** A scenario asserts on printed text through the string lists under `expect`; its settings and outcomes are inputs and statuses. */ +function scenarioAssertions(scenario: unknown): string[] { + const expect = (scenario as { expect?: Record } | null)?.expect; + return Object.values(expect ?? {}).flatMap((value) => + Array.isArray(value) ? value.filter((item): item is string => typeof item === "string") : [], + ); +} + +/** A literal's value; a template's quasis, and a `+` chain's operands, joined around their holes. */ +function spelled(node: Node): string { + if (isStringLiteral(node)) { + return field(node, "value"); + } + if (node.type === "TemplateLiteral") { + return field(node, "quasis") + .map((quasi) => quasi.value.cooked ?? "") + .join(" "); + } + if (isConcatenation(node)) { + return `${spelled(field(node, "left"))}${spelled(field(node, "right"))}`; + } + return " "; +} + +/** + * The strings a test asserts on: a template's quasis, and the operands of a `+` chain, joined around their holes, so + * a pin with the same holes and a pin of the full sentence both contain the source fragment. A test's title describes + * the case and asserts nothing, so it is left out. A scenario's pins are its `expect` lists. + */ +export function testStrings(path: string, text: string): string[] { + if (!path.endsWith(".ts")) { + return scenarioAssertions(parseYaml(text)); + } + const strings: string[] = []; + const titles = new Set(); + walk(parse(path, text), ({ node }) => { + if (node.type === "CallExpression" && isTestCall(node)) { + const title = field(node, "arguments")[0]; + if (title !== undefined) { + titles.add(title); + } + } + if (titles.has(node)) { + return false; + } + if (isStringLiteral(node) || node.type === "TemplateLiteral" || isConcatenation(node)) { + strings.push(spelled(node)); + return false; + } + return undefined; + }); + return strings; +} + +/** + * Whether a test string pins a fragment. A piece of a message, and prose, are pinned by containment. A whole message + * that is not prose (terse, or one spaceless word) is a token another test may hold by accident (a role named + * "probe", the head of zod's own "Invalid input: ..."), so it counts only as the whole message of a rendered line: + * after the `: ` that follows the key path, and ending the string or the line. + */ +function pinsFragment(pinned: string, fragment: Fragment): boolean { + if (!fragment.whole) { + return isProse(fragment.text) + ? pinned.includes(fragment.text) + : containsWord(pinned, fragment.text); + } + const lead = `: ${fragment.text}`; + for (let at = pinned.indexOf(lead); at !== -1; at = pinned.indexOf(lead, at + 1)) { + const after = pinned[at + lead.length]; + if (after === undefined || after === "\n") { + return true; + } + } + return false; +} + +/** + * Whether `pinned` holds `piece` at word boundaries: "a list" inside "comma list" is not the piece "a list". An edge + * that is not a word character (a quote, a space, a colon) needs no boundary of its own. + */ +function containsWord(pinned: string, piece: string): boolean { + const isWord = (character: string | undefined) => character !== undefined && /\w/.test(character); + const headIsWord = isWord(piece[0]); + const tailIsWord = isWord(piece[piece.length - 1]); + for (let at = pinned.indexOf(piece); at !== -1; at = pinned.indexOf(piece, at + 1)) { + const clearBefore = !headIsWord || !isWord(pinned[at - 1]); + const clearAfter = !tailIsWord || !isWord(pinned[at + piece.length]); + if (clearBefore && clearAfter) { + return true; + } + } + return false; +} + +/** The fragments no test string pins, as `path:line: "fragment"` lines. */ +export function unpinnedMessages(sources: Iterable, tests: readonly string[]): string[] { + return messageFragments(sources) + .filter((fragment) => !tests.some((pinned) => pinsFragment(pinned, fragment))) + .map((fragment) => `${fragment.path}:${fragment.line}: ${JSON.stringify(fragment.text)}`); +} + +function read(root: string, path: string): string { + return readFileSync(join(root, path), "utf8"); +} + +/** The refusal sources of this repository, each with the way it is read; a listed file that is gone is a loud failure, never a silent drop. */ +export function refusalSources(root: string): Source[] { + const sources: Source[] = []; + const add = (path: string, sites: Sites) => { + if (!existsSync(join(root, path))) { + throw new Error( + `${path} is a refusal source of this census but does not exist; a moved or renamed source is listed again under its new path`, + ); + } + sources.push({ path, text: read(root, path), sites }); + }; + const sectionDirs = readdirSync(join(root, "src/sections"), { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort(); + for (const dir of sectionDirs) { + if (dir === "shared" || dir === "contract") { + continue; + } + add(`src/sections/${dir}/schema.ts`, "every-literal"); + add(`src/sections/${dir}/index.ts`, "message-positions"); + } + for (const helper of SHARED_REFUSAL_HELPERS) { + add(`src/sections/shared/${helper}.ts`, "every-literal"); + } + // The regex check's own reasons render inside the pattern refusal; the RegExp engine's messages pass through unread. + add("src/sections/secret_scanning_custom_patterns/compilable-form.ts", "every-literal"); + // The environments section's nested lists validate in their own files. + for (const nested of ["nested", "branch-policies", "protection-rules"]) { + add(`src/sections/environments/${nested}.ts`, "message-positions"); + } + add("src/sections/shared/repo-secrets.ts", "message-positions"); + add("src/sections/contract/module.ts", "message-positions"); + add("src/problem.ts", "message-positions"); + return sources; +} + +/** Every string spelled by a test under test/, this census excluded (it quotes nothing a user reads). */ +export function pinnedStrings(root: string, self: string): string[] { + return readdirSync(join(root, "test"), { recursive: true }) + .map(String) + .filter((name) => (name.endsWith(".ts") || name.endsWith(".yml")) && `test/${name}` !== self) + .sort() + .flatMap((name) => testStrings(`test/${name}`, read(root, `test/${name}`))); +} + +const SELF = "test/sections/refusal-messages.test.ts"; + +describe("every parse-refusal message a user can read is pinned by a test", () => { + test("each message literal of the refusal sources appears in a test's strings", () => { + expect(unpinnedMessages(refusalSources(ROOT), pinnedStrings(ROOT, SELF))).toEqual([]); + }); + + test("the census reads a real number of messages from both kinds of source (control)", () => { + const sources = refusalSources(ROOT); + const byKind = (sites: Sites) => + messageFragments(sources.filter((source) => source.sites === sites)).length; + expect(sources.map((source) => source.path)).toContain("src/sections/labels/schema.ts"); + expect(byKind("every-literal")).toBeGreaterThan(250); + expect(byKind("message-positions")).toBeGreaterThan(20); + }); + + test("every DATA_CONSTANTS name is a constant of some literal-first source, so the exclusion list cannot go stale (control)", () => { + const declared = new Set(); + for (const source of refusalSources(ROOT).filter((s) => s.sites === "every-literal")) { + walk(parse(source.path, source.text), ({ node }) => { + if (node.type === "VariableDeclarator") { + declared.add(declaredName(node) ?? ""); + } + return undefined; + }); + } + expect([...DATA_CONSTANTS].filter((name) => !declared.has(name))).toEqual([]); + }); + + test("every DATA_ARGUMENTS and PIECE_ARGUMENTS name is a function some census source declares or imports, so the registries cannot go stale (control)", () => { + const known = new Set(); + for (const source of refusalSources(ROOT)) { + walk(parse(source.path, source.text), ({ node }) => { + if (node.type === "FunctionDeclaration") { + known.add(functionName(node) ?? ""); + } + if (node.type === "VariableDeclarator") { + known.add(declaredName(node) ?? ""); + } + if (node.type === "ImportSpecifier") { + known.add(field(field(node, "local"), "name")); + } + return undefined; + }); + } + const registered = [...Object.keys(DATA_ARGUMENTS), ...Object.keys(PIECE_ARGUMENTS)]; + expect(registered.filter((name) => !known.has(name))).toEqual([]); + }); + + test("a listed source that is gone fails naming its path instead of dropping its messages (control)", () => + withTempDir("refusal-sources-", (root) => { + mkdirSync(join(root, "src/sections/planted"), { recursive: true }); + writeFileSync(join(root, "src/sections/planted/schema.ts"), "export const a = 1;\n"); + expect(() => refusalSources(root)).toThrow( + /^src\/sections\/planted\/index\.ts is a refusal source of this census but does not exist/, + ); + })); + + /** One literal per exclusion position, then a message in every spelling a source can give one. */ + const plantedSlice: Source = { + path: "src/sections/planted/schema.ts", + text: [ + 'import { z } from "zod";', + 'export { a as "renamed" } from "./b.js";', + 'const spec = { "quoted key": 1, code: "custom", path: ["title"], id: "PlantedConfig", route: "GET /repos" };', + 'type Kind = "branch" | "tag";', + 'declare const kind: "branch" | "tag";', + 'const enumerated = z.enum(["open", "closed"]).default("open");', + 'const lit = z.literal("web").includes("");', + 'const parts = list.split(",").join(", ");', + 'const framing = ["PRIVATE", "KEY-----"].join(" ");', + 'if (kind === "tag") { switch (kind) { case "tag": break; } }', + "const raw = String.raw`[~^: ]|\\.\\.`;", + 'const REF_NAME_TOKENS = ["~ALL", "~DEFAULT_BRANCH"] as const;', + 'const PLANTED_KINDS = { knobbed: { directives: "a planted directive sentence inside an as const table", renamed: true } } as const;', + 'const twin = z.object({}).meta({ anyOf: [{ required: ["contexts"] }] });', + 'const pattern = new RegExp("^[a-z]+$"); const names = new Set(["a", "b"]); const ok = !"(|^$".includes(previous);', + 'function render(): string { return "[^"; }', + 'throw new Error("BUG: a planted invariant names no user");', + // messages, every spelling + 'export const Word = z.string({ error: "probe" });', + "export const NoHoles = z.string({ error: `probe2` });", + `export const Picked = z.string({ error: (issue) => \`\${issue.input === undefined ? "Missing" : "Bad"}\` });`, + 'export const Sized = boundedString(9, "code points", () => "too long");', + 'const reasons = { bad: "probe3", long: "a planted table entry read by property" };', + "export const Tabled = z.string({ error: reasons.bad }).min(1, reasons.long);", + 'let msg: string; msg = "probe4";', + 'export const Computed = z.string({ ["error"]: "probe5" });', + "function titled(message: string) { return z.string().min(1, message); }", + 'export const Titled = titled("probe6");', + `export const Punct = z.string({ error: (issue) => issue.code + ": !!!" });`, + 'export const Cast = z.string({ error: "Color required" });', + 'const callback = { message: () => "probe7" };', + 'for (const m of ["probe8", "probe9"]) { ctx.addIssue({ message: m }); }', + 'const chained = () => helperA(); function helperA() { return helperB(); } function helperB() { return "probe10"; }', + `export const Holes = z.string().min(1, { error: (issue) => \`\${issue.input}: too short\` });`, + 'export const Plus = z.string({ error: "Please " + "enter text" });', + "export const Tagged = z.string({ error: plantedTag`probe tagged` });", + 'const KNOWN = [rule("creation", { error: "a planted rule message" })] as const;', + 'export const Picked2 = z.enum(["open", "closed"], "Please pick a planted state").or(z.literal("web", "probe lit"));', + `const RULE = \`a planted key holds only \${WHAT}; remove the other characters\`;`, + 'const PLANTED_ESCAPES = [[/^\\\\q/, "a planted reason for a q escape"]];', + 'const GET_ONLY_KEYS = ["node_id", "a planted sentence inside a data constant"];', + ].join("\n"), + sites: "every-literal", + }; + + const plantedModule: Source = { + path: "src/sections/planted/index.ts", + text: [ + `const WHY = (key: string) => \`the key \${key} would silently do nothing on this endpoint\`;`, + 'const ROUTE = "GET /repos/{owner}/{repo}/planted listing every entry";', + 'const note = "a planted plan note, outside the census by the owner ruling";', + 'const outcome = { describe: "planted setting applied" };', + 'const LivePlanted = z.object({}).transform((live, ctx) => { ctx.addIssue({ code: "custom", message: "a live planted body is not a document refusal" }); });', + 'function routedPlanted() { const beside = flag ? "an optional planted directive" : "a planted policy";', + ` return z.custom(() => true).transform((value, ctx) => { ctx.addIssue({ code: "custom", message: \`a planted routed shape refusal (and \${beside})\`, params: { legal: "a planted legal form" } }); }); }`, + 'function malformedListIssues(): DeclaredIssue[] { return [{ path: "[0].value", message: "an empty list is not a value" }]; }', + 'function plantedWhy(): string { return "probe why"; }', + `function plantedNested(entries: unknown[], envName: string) { return duplicateFieldIssues(entries, { field: "name" }, \`planted rule of the "\${envName}" environment\`); }`, + 'function planted(): void { const inner = { noun: "a nested planted noun, plan text" }; }', + "export const section = {", + ' ...identifiedBy("planted", "name", "planted thing"),', + ' noun: "planted noun",', + ' validate: (label: string) => [{ path: "[0].key", message: WHY(label) }, { path: "[1].key", message: "probe" }, { path: "[2].key", message: plantedWhy() }, ...malformedListIssues()],', + ' shape: loosen(z.object({ title: z.string({ error: "Please declare a title" }) })),', + ' closedSurface: { consequence: "the planted body carries only the name", other: { consequence: "drops keys" } },', + ' endpoints: { list: { statuses: { 200: "the planted list, which no refusal quotes" } } },', + "};", + ].join("\n"), + sites: "message-positions", + }; + + const plantedProblem: Source = { + path: "src/problem.ts", + text: [ + 'const ADVICE = "Remove the planted key, or keep the note as a YAML comment";', + 'function describePlantedShape(value: unknown): string { return Array.isArray(value) ? "a list" : "a mapping"; }', + "export function plantedIssue(unknown: string, actual: unknown): string {", + ` return \`unknown planted key: \${unknown}; got \${describePlantedShape(actual)}. \${ADVICE}\`;`, + "}", + 'function describeOther(): string { return "not a document refusal, not read here"; }', + ].join("\n"), + sites: "message-positions", + }; + + const planted = [plantedSlice, plantedModule, plantedProblem]; + + test("every planted message is a finding by construction, whatever its spelling; every planted exclusion is silent (control)", () => { + const prose = testStrings( + "test/prose.test.ts", + [ + 'const note = "the preflight probe found nothing"; const other = "no probes here";', + 'const longer = "planted.key: probe: expected string, received number";', + 'const split = "planted.key: probe" + ": expected string, received number";', + 'const inside = "prefix-probe6-suffix";', + 'test("does not remove the other characters", () => {});', + 'describe.each([1])("the planted body carries only the name %s", () => {});', + ].join("\n"), + ); + expect(unpinnedMessages(planted, prose)).toEqual([ + 'src/sections/planted/schema.ts:13: "a planted directive sentence inside an as const table"', + 'src/sections/planted/schema.ts:18: "probe"', + 'src/sections/planted/schema.ts:19: "probe2"', + 'src/sections/planted/schema.ts:20: "Missing"', + 'src/sections/planted/schema.ts:20: "Bad"', + 'src/sections/planted/schema.ts:21: "too long"', + 'src/sections/planted/schema.ts:22: "probe3"', + 'src/sections/planted/schema.ts:22: "a planted table entry read by property"', + 'src/sections/planted/schema.ts:24: "probe4"', + 'src/sections/planted/schema.ts:25: "probe5"', + 'src/sections/planted/schema.ts:27: "probe6"', + 'src/sections/planted/schema.ts:28: ": !!!"', + 'src/sections/planted/schema.ts:29: "Color required"', + 'src/sections/planted/schema.ts:30: "probe7"', + 'src/sections/planted/schema.ts:31: "probe8"', + 'src/sections/planted/schema.ts:31: "probe9"', + 'src/sections/planted/schema.ts:32: "probe10"', + 'src/sections/planted/schema.ts:33: ": too short"', + 'src/sections/planted/schema.ts:34: "Please "', + 'src/sections/planted/schema.ts:34: "enter text"', + 'src/sections/planted/schema.ts:35: "probe tagged"', + 'src/sections/planted/schema.ts:36: "a planted rule message"', + 'src/sections/planted/schema.ts:37: "Please pick a planted state"', + 'src/sections/planted/schema.ts:37: "probe lit"', + 'src/sections/planted/schema.ts:38: "a planted key holds only "', + 'src/sections/planted/schema.ts:38: "; remove the other characters"', + 'src/sections/planted/schema.ts:39: "a planted reason for a q escape"', + 'src/sections/planted/schema.ts:40: "a planted sentence inside a data constant"', + 'src/sections/planted/index.ts:1: "the key "', + 'src/sections/planted/index.ts:1: " would silently do nothing on this endpoint"', + 'src/sections/planted/index.ts:6: "an optional planted directive"', + 'src/sections/planted/index.ts:6: "a planted policy"', + 'src/sections/planted/index.ts:7: "a planted routed shape refusal (and "', + 'src/sections/planted/index.ts:7: "a planted legal form"', + 'src/sections/planted/index.ts:8: "an empty list is not a value"', + 'src/sections/planted/index.ts:9: "probe why"', + 'src/sections/planted/index.ts:10: "planted rule of the \\""', + 'src/sections/planted/index.ts:10: "\\" environment"', + 'src/sections/planted/index.ts:13: "planted thing"', + 'src/sections/planted/index.ts:14: "planted noun"', + 'src/sections/planted/index.ts:15: "probe"', + 'src/sections/planted/index.ts:16: "Please declare a title"', + 'src/sections/planted/index.ts:17: "the planted body carries only the name"', + 'src/sections/planted/index.ts:17: "drops keys"', + 'src/problem.ts:1: "Remove the planted key, or keep the note as a YAML comment"', + 'src/problem.ts:2: "a list"', + 'src/problem.ts:2: "a mapping"', + 'src/problem.ts:4: "unknown planted key: "', + 'src/problem.ts:4: "; got "', + ]); + }); + + test("the same messages pinned as a full sentence, a template with the same holes, or a + chain pass (control)", () => { + const pins = testStrings( + "test/planted.test.ts", + [ + "const full = 'planted.key: a planted key holds only letters; remove the other characters';", + 'const terse = ["planted.key: probe", "planted.key: probe2", "planted.key: Missing", "planted.key: Bad"];', + 'const more = "planted.key: code points\\nplanted.key: too long\\nplanted.key: probe3\\nplanted.key: probe4";', + 'const rest = "planted.key: probe5\\nplanted.key: probe6\\nplanted.key: custom: !!!\\nplanted.key: Color required";', + 'const fns = "planted.key: probe7\\nplanted.key: probe8\\nplanted.key: probe9\\nplanted.key: probe10";', + `const holes = \`planted.key: one raw word\\nplanted.key: \${input}: too short\`;`, + 'const pieces = "planted.key: " + "Please enter text";', + 'const table = "planted.key: a planted table entry read by property";', + 'const reason = "planted[0].pattern: cannot be compiled (a planted reason for a q escape)";', + 'const constant = "planted.key: a planted sentence inside a data constant";', + `const templated = (key: string) => \`the key \${key} would silently do nothing on this endpoint\`;`, + 'const clause = "the planted body carries only the name";', + 'const inline = "planted[0].title: Please declare a title";', + 'const shaped = "planted[0].value: an empty list is not a value";', + 'const routed = "planted: a planted routed shape refusal (and an optional planted directive); planted: a planted routed shape refusal (and a planted policy)";', + 'const legal = "planted has no empty state; write a planted legal form";', + 'const directive = "labels: Unrecognized key; a planted directive sentence inside an as const table";', + 'const why = "planted[2].key: probe why";', + `const nouns = \`"x" names the same planted thing as "y"; planted noun; planted rule of the "prod" environment\`;`, + 'const narrowed = "planted.key: probe tagged\\nplanted.key: a planted rule message\\nplanted.key: Please pick a planted state\\nplanted.key: probe lit";', + 'const shapes = "got a list; got a mapping";', + "const sealed = 'planted[0] (name \"x\"): drops keys';", + `const advice = \`unknown planted key: \${name}; got \${shape}. Remove the planted key, or keep the note as a YAML comment\`;`, + ].join("\n"), + ); + expect(unpinnedMessages(planted, pins)).toEqual([]); + }); + + test("a scenario pins a message through its expect lists; its settings and outcomes do not (control)", () => { + const inputsOnly = testStrings( + "test/planted/scenarios/inputs.yml", + "settings:\n planted:\n - key: probe\nexpect:\n outcomes:\n planted: probe2\n", + ); + expect(unpinnedMessages([plantedSlice], inputsOnly)).toContain( + 'src/sections/planted/schema.ts:18: "probe"', + ); + const scenario = [ + "expect:", + " stdout_contains:", + " - 'planted.key: probe'", + ' - "planted.key: probe2"', + "", + ].join("\n"); + const pins = testStrings("test/planted/scenarios/planted.yml", scenario); + const left = unpinnedMessages([plantedSlice], pins); + expect(left).not.toContain('src/sections/planted/schema.ts:18: "probe"'); + expect(left).not.toContain('src/sections/planted/schema.ts:19: "probe2"'); + }); + + test("a source that does not parse fails naming it instead of reading as pinned (control)", () => { + const broken: Source = { + path: "src/broken.ts", + text: "const x = ;", + sites: "every-literal", + }; + expect(() => unpinnedMessages([broken], [])).toThrow( + /^src\/broken\.ts does not parse, so its strings cannot be read: /, + ); + }); +}); diff --git a/test/sections/repository/schema.test.ts b/test/sections/repository/schema.test.ts new file mode 100644 index 00000000..571746da --- /dev/null +++ b/test/sections/repository/schema.test.ts @@ -0,0 +1,186 @@ +/** + * The repository section's parse refusals, each pinned as the problem line a user reads: a quoted boolean or a bare + * number where the PATCH takes a toggle or a string, the vocabularies GitHub 422s (feature status, creation + * policy, the commit-message pairs), the topic grammar and cap, the closed security_and_analysis shape, and the + * GET-only fields that would drift forever. Parsed through the loosened document shape, so a rule that survives here + * reaches the run. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(repository: Record): readonly string[] | null { + return validateSectionShapes({ repository }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +const SQUASH_PAIRS = + ". Legal pairs: PR_TITLE with PR_BODY or BLANK or COMMIT_MESSAGES; COMMIT_OR_PR_TITLE with COMMIT_MESSAGES"; + +describe("a repository setting GitHub would 422 or never converge on is refused at parse, naming the key and the fix", () => { + test.each<[what: string, repository: Record, expected: string[]]>([ + [ + "a YAML-quoted toggle", + { has_issues: "yes" }, + [ + 'repository.has_issues: "yes" is not a boolean, so the toggle direction is ambiguous. Use unquoted true or false (YAML parses "no"/"off"/"yes" as strings, not booleans)', + ], + ], + [ + "a list where a toggle goes", + { has_issues: ["yes"] }, + [ + 'repository.has_issues: a list is not a boolean, so the toggle direction is ambiguous. Use unquoted true or false (YAML parses "no"/"off"/"yes" as strings, not booleans)', + ], + ], + [ + "a mapping where a toggle goes", + { has_issues: { on: true } }, + [ + 'repository.has_issues: a mapping is not a boolean, so the toggle direction is ambiguous. Use unquoted true or false (YAML parses "no"/"off"/"yes" as strings, not booleans)', + ], + ], + [ + "a bare number where a clearable string goes", + { description: 5 }, + [ + "repository.description: 5 is not a string; quote the value, or write null to clear the field", + ], + ], + [ + "a bare number where a string goes", + { default_branch: 2 }, + ["repository.default_branch: 2 is not a string; quote the value"], + ], + [ + "a key a feature toggle does not take", + { security_and_analysis: { secret_scanning: { enabled: true } } }, + [ + 'repository.security_and_analysis.secret_scanning: "enabled" is not a key a security_and_analysis feature accepts (GitHub rejects it with a 422); remove it. Known keys: "status"', + ], + ], + [ + "a bypass reviewer with a key GitHub rejects", + { + security_and_analysis: { + secret_scanning_delegated_bypass_options: { + reviewers: [{ reviewer_id: 1, reviewer_type: "TEAM", role: "admin" }], + }, + }, + }, + [ + 'repository.security_and_analysis.secret_scanning_delegated_bypass_options.reviewers[0]: "role" is not a key a bypass reviewer accepts (GitHub rejects it with a 422); remove it. Known keys: "reviewer_id", "reviewer_type", "mode"', + ], + ], + [ + "a bypass options key GitHub rejects", + { security_and_analysis: { secret_scanning_delegated_bypass_options: { reviewer: [] } } }, + [ + 'repository.security_and_analysis.secret_scanning_delegated_bypass_options: "reviewer" is not a key secret_scanning_delegated_bypass_options accepts (GitHub rejects it with a 422); remove it. Known keys: "reviewers"', + ], + ], + [ + "a squash title in the wrong case", + { squash_merge_commit_title: "pr_title" }, + [ + `repository.squash_merge_commit_title: "pr_title" is not a squash_merge_commit_title value; use "PR_TITLE", "COMMIT_OR_PR_TITLE"${SQUASH_PAIRS}`, + ], + ], + [ + "a squash message GitHub has no value for", + { squash_merge_commit_title: "PR_TITLE", squash_merge_commit_message: "PR_DESCRIPTION" }, + [ + `repository.squash_merge_commit_message: "PR_DESCRIPTION" is not a squash_merge_commit_message value; use "PR_BODY", "BLANK", "COMMIT_MESSAGES"${SQUASH_PAIRS}`, + ], + ], + [ + "a merge message without its title, where GitHub documents no pair matrix", + { merge_commit_message: "PR_BODY" }, + [ + "repository.merge_commit_message: merge_commit_message needs merge_commit_title declared beside it (GitHub requires the pair)", + ], + ], + [ + "a merge title GitHub has no value for", + { merge_commit_title: "PR_BODY" }, + [ + 'repository.merge_commit_title: "PR_BODY" is not a merge_commit_title value; use "PR_TITLE", "MERGE_MESSAGE"', + ], + ], + [ + "a creation policy spelled in prose", + { pull_request_creation_policy: "everyone" }, + [ + 'repository.pull_request_creation_policy: "everyone" is not a recognized policy. Use "all" (everyone) or "collaborators_only"', + ], + ], + [ + "an empty topic in the list form", + { topics: ["ci", ""] }, + [ + "repository.topics[1]: an empty topic is not one GitHub accepts; drop the entry, or declare topics: [] to remove every topic", + ], + ], + [ + "a topic starting with a hyphen, in the list form", + { topics: ["-ci"] }, + [ + 'repository.topics[0]: "-ci" is not a topic GitHub accepts: a topic is 1 to 50 characters, each a letter, digit, or hyphen, starting with a letter or digit (uppercase is lowercased on the wire)', + ], + ], + [ + "a topic with a space, in the comma form, named by its position", + { topics: "ci, bad topic" }, + [ + 'repository.topics: "bad topic" (entry 2 of the comma list) is not a topic GitHub accepts: a topic is 1 to 50 characters, each a letter, digit, or hyphen, starting with a letter or digit (uppercase is lowercased on the wire)', + ], + ], + [ + "an empty segment in the comma form", + { topics: "ci,,docs" }, + [ + "repository.topics: an empty topic (entry 2 of the comma list) is not one GitHub accepts; drop the entry, or declare topics: [] to remove every topic", + ], + ], + [ + "more distinct topics than GitHub stores", + { topics: Array.from({ length: 21 }, (_, i) => `topic-${i}`) }, + ["repository.topics: 21 topics declared; GitHub allows at most 20"], + ], + [ + "a GET-only field no write accepts", + { full_name: "octocat/hello-world" }, + [ + "repository.full_name: full_name is reported by GitHub but cannot be set through the API; remove it", + ], + ], + [ + "a GET-only field another section owns", + { has_pages: true }, + [ + "repository.has_pages: has_pages is reported by GitHub but cannot be set through the repository PATCH; declare it in the pages section instead", + ], + ], + ])("%s", (_what, repository, expected) => { + expect(issues(repository)).toEqual(expected); + }); + + test("the forms GitHub accepts parse: unquoted toggles, null to clear, both topic forms, the documented squash pairs", () => { + expect( + issues({ + has_issues: true, + description: null, + topics: ["CI", "ci", "docs"], + squash_merge_commit_title: "COMMIT_OR_PR_TITLE", + squash_merge_commit_message: "COMMIT_MESSAGES", + merge_commit_title: "MERGE_MESSAGE", + merge_commit_message: "PR_TITLE", + pull_request_creation_policy: "collaborators_only", + security_and_analysis: { secret_scanning: { status: "enabled" } }, + }), + ).toBeNull(); + expect(issues({ topics: "ci, docs" })).toBeNull(); + }); +}); diff --git a/test/sections/roles.test.ts b/test/sections/roles.test.ts index 94ed866a..c614d9da 100644 --- a/test/sections/roles.test.ts +++ b/test/sections/roles.test.ts @@ -52,11 +52,13 @@ describe("a permission the grant PUT would 422 never reaches it", () => { }); }); - test.each<[what: string, permission: unknown, message: RegExp]>([ + const OPTIONS = '"pull", "triage", "push", "maintain", "admin", or a custom org role name'; + + test.each<[what: string, permission: unknown, message: string | RegExp]>([ [ "the read vocabulary of the write role", "write", - /"write" is the vocabulary GitHub reports.*declare "push"/, + `"write" is the vocabulary GitHub reports a role in (role_name), not one a grant accepts; declare "push" (${OPTIONS})`, ], [ "the read vocabulary of the read role", @@ -71,7 +73,7 @@ describe("a permission the grant PUT would 422 never reaches it", () => { [ "a mis-cased standard permission", "Push", - /"Push" is not a permission GitHub accepts.*declare "push"$/, + '"Push" is not a permission GitHub accepts; the standard permissions are lowercase: declare "push"', ], [ "an upper-cased standard permission", @@ -81,12 +83,12 @@ describe("a permission the grant PUT would 422 never reaches it", () => { [ "an empty permission", "", - /an empty permission grants nothing.*omit the key for the default "push"$/, + `an empty permission grants nothing; declare ${OPTIONS}, or omit the key for the default "push"`, ], [ "a block scalar: the newline is named and the fix is the trimmed standard form", "Push\n", - /"Push\\n" carries whitespace at an end \(a YAML block scalar ends in a newline\); declare "push"$/, + '"Push\\n" carries whitespace at an end (a YAML block scalar ends in a newline); declare "push"', ], [ "a quoted scalar with a leading space, which GitHub would not match", @@ -106,9 +108,13 @@ describe("a permission the grant PUT would 422 never reaches it", () => { [ "a custom role spanning two lines: no fix is suggested, since none would parse", "Security\nTeam\n", - /"Security\\nTeam\\n" spans several lines; a permission is one line: "pull".*custom org role name$/, + `"Security\\nTeam\\n" spans several lines; a permission is one line: ${OPTIONS}`, + ], + [ + "a whitespace-only permission", + " ", + `" " (whitespace only) grants nothing; declare ${OPTIONS}, or omit the key for the default "push"`, ], - ["a whitespace-only permission", " ", /" " \(whitespace only\) grants nothing.*omit the key/], [ "a mapping: the pattern check must not run on a non-string, so the type error is the only issue", { length: 4 }, @@ -118,7 +124,9 @@ describe("a permission the grant PUT would 422 never reaches it", () => { "fails at parse naming the entry and the form to declare: %s", (_what, permission, message) => { const issue = (key: string) => [ - expect.stringMatching(new RegExp(`^${key}\\[0\\]\\.permission: .*${message.source}`)), + typeof message === "string" + ? `${key}[0].permission: ${message}` + : expect.stringMatching(new RegExp(`^${key}\\[0\\]\\.permission: .*${message.source}`)), ]; expect({ collaborators: verdict("collaborators", permission), diff --git a/test/sections/rulesets/schema.test.ts b/test/sections/rulesets/schema.test.ts index 99b5fdad..6b95370e 100644 --- a/test/sections/rulesets/schema.test.ts +++ b/test/sections/rulesets/schema.test.ts @@ -129,7 +129,7 @@ describe("a ruleset the API would reject never reaches it", () => { expect([...KNOWN_RULE_TYPES].sort()).toEqual([...RULESET_RULE_TYPES].sort()); }); - test.each<[what: string, ruleset: Record, issues: RegExp[]]>([ + test.each<[what: string, ruleset: Record, issues: (string | RegExp)[]]>([ [ "an enforcement level spelled the way branch protection spells it", { name: "main", enforcement: "enabled" }, @@ -147,8 +147,8 @@ describe("a ruleset the API would reject never reaches it", () => { bypass_actors: [{ actor_type: "Team" }, { actor_type: "User", actor_id: null }], }, [ - /^rulesets\[0\]\.bypass_actors\[0\]\.actor_id: a Team bypass actor needs its numeric actor_id/, - /^rulesets\[0\]\.bypass_actors\[1\]\.actor_id: a User bypass actor needs its numeric actor_id/, + "rulesets[0].bypass_actors[0].actor_id: a Team bypass actor needs its numeric actor_id (the id GitHub assigns the app, role, team, or user); GitHub rejects the ruleset without it", + "rulesets[0].bypass_actors[1].actor_id: a User bypass actor needs its numeric actor_id (the id GitHub assigns the app, role, team, or user); GitHub rejects the ruleset without it", ], ], [ @@ -158,8 +158,8 @@ describe("a ruleset the API would reject never reaches it", () => { bypass_actors: [{ actor_type: "DeployKey", actor_id: 7, bypass_mode: "pull_request" }], }, [ - /^rulesets\[0\]\.bypass_actors\[0\]\.actor_id: a DeployKey bypass actor takes no actor_id/, - /^rulesets\[0\]\.bypass_actors\[0\]\.bypass_mode: bypass_mode "pull_request" does not apply to a DeployKey/, + "rulesets[0].bypass_actors[0].actor_id: a DeployKey bypass actor takes no actor_id (GitHub documents it as null); remove the key or write null", + 'rulesets[0].bypass_actors[0].bypass_mode: bypass_mode "pull_request" does not apply to a DeployKey actor; use "always" or "exempt"', ], ], [ @@ -178,15 +178,15 @@ describe("a ruleset the API would reject never reaches it", () => { bypass_actors: [{ actor_type: "Team", actor_id: 1, bypass_mode: "pull_request" }], }, [ - /^rulesets\[0\]\.bypass_actors\[0\]\.bypass_mode: .*branch rulesets only, and this ruleset targets tag/, + 'rulesets[0].bypass_actors[0].bypass_mode: bypass_mode "pull_request" applies to branch rulesets only, and this ruleset targets tag; use "always" or "exempt"', ], ], [ "a mis-cased token in include and a made-up one in exclude", { name: "main", conditions: { ref_name: { include: ["~all"], exclude: ["main", "~MAIN"] } } }, [ - /^rulesets\[0\]\.conditions\.ref_name\.include\[0\]: "~all" is not a ref-name token: the tokens are ~ALL and ~DEFAULT_BRANCH/, - /^rulesets\[0\]\.conditions\.ref_name\.exclude\[1\]: "~MAIN" is not a ref-name token/, + 'rulesets[0].conditions.ref_name.include[0]: "~all" is not a ref-name token: the tokens are ~ALL and ~DEFAULT_BRANCH (case-sensitive), and no ref name contains "~"', + 'rulesets[0].conditions.ref_name.exclude[1]: "~MAIN" is not a ref-name token: the tokens are ~ALL and ~DEFAULT_BRANCH (case-sensitive), and no ref name contains "~"', ], ], [ @@ -209,7 +209,7 @@ describe("a ruleset the API would reject never reaches it", () => { }, }, [ - /^rulesets\[0\]\.conditions\.ref_name\.include\[0\]: "release\^2" contains "\^": git refuses "~", "\^", ":", "\\", space, "\.\.", "@\{", and control characters in a ref name, and a ruleset pattern has no use for them$/, + 'rulesets[0].conditions.ref_name.include[0]: "release^2" contains "^": git refuses "~", "^", ":", "\\", space, "..", "@{", and control characters in a ref name, and a ruleset pattern has no use for them', /^rulesets\[0\]\.conditions\.ref_name\.include\[1\]: "refs\/heads\/a:b" contains ":"/, /^rulesets\[0\]\.conditions\.ref_name\.include\[2\]: "back\\\\slash" contains "\\\\"/, /^rulesets\[0\]\.conditions\.ref_name\.include\[3\]: "hot fix" contains " "/, @@ -282,7 +282,7 @@ describe("a ruleset the API would reject never reaches it", () => { ], }, [ - /^rulesets\[0\]\.rules\[0\]: parameters\.allowed_merge_methods: allowed_merge_methods needs at least one of "merge", "squash", "rebase"; omit the key to allow all three$/, + 'rulesets[0].rules[0]: parameters.allowed_merge_methods: allowed_merge_methods needs at least one of "merge", "squash", "rebase"; omit the key to allow all three', ], ], [ @@ -376,7 +376,9 @@ describe("a ruleset the API would reject never reaches it", () => { "what would 422 at apply fails at parse naming the key and the fix: %s", (_what, ruleset, issues) => { expect(verdict(ruleset)).toEqual({ - issues: issues.map((issue) => expect.stringMatching(issue)), + issues: issues.map((issue) => + typeof issue === "string" ? issue : expect.stringMatching(issue), + ), }); }, ); diff --git a/test/sections/secret-variable-schema.test.ts b/test/sections/secret-variable-schema.test.ts index d5a04548..936f7958 100644 --- a/test/sections/secret-variable-schema.test.ts +++ b/test/sections/secret-variable-schema.test.ts @@ -119,3 +119,13 @@ describe("variable values", () => { }, ); }); + +describe("secret entry keys", () => { + test("a key outside name and value is refused naming the entry, since the sealed PUT body carries nothing else", () => { + expect( + issuesOf({ actions_secrets: [{ name: "TOKEN", value: "$TOKEN", values: "x" }] }), + ).toEqual([ + 'actions_secrets[0] (name "TOKEN"): declares "values", which this section does not recognize (known keys: name, value) - the API body carries only the sealed value, so the key would silently do nothing. Fix the key name, or remove it', + ]); + }); +}); diff --git a/test/sections/secret_scanning_custom_patterns/compilable-form.test.ts b/test/sections/secret_scanning_custom_patterns/compilable-form.test.ts index 91d602bb..8960e551 100644 --- a/test/sections/secret_scanning_custom_patterns/compilable-form.test.ts +++ b/test/sections/secret_scanning_custom_patterns/compilable-form.test.ts @@ -240,3 +240,19 @@ describe("compilableForm", () => { expect(compileFailure(source)).toBeDefined(); }); }); + +describe("the check's own reasons, as the pattern refusal renders them", () => { + // PCRE's wording for what it refuses; the RegExp engine's own message passes through for the rest and is not pinned. + test.each<[source: string, reason: string]>([ + ["*a", "quantifier does not follow a repeatable item"], + ["\\x{ZZ}", "non-hex character or missing } in \\x{}"], + ["\\o{8}", "non-octal character or missing braces in \\o{}"], + ["\\c", "\\c needs a printable ASCII character after it"], + ["\\x{110000}", "\\x{110000} is above U+10FFFF"], + ["(?#x", "missing ) after (?# comment"], + ["(?$a)", "unrecognized character after (?"], + ["(?x)(?y)", "two named groups have the same name (a)"], + ])("%s is refused as %s", (source, reason) => { + expect(compileFailure(source)).toBe(reason); + }); +}); diff --git a/test/sections/secret_scanning_custom_patterns/schema.test.ts b/test/sections/secret_scanning_custom_patterns/schema.test.ts index 3760d0a6..e54d1ee2 100644 --- a/test/sections/secret_scanning_custom_patterns/schema.test.ts +++ b/test/sections/secret_scanning_custom_patterns/schema.test.ts @@ -6,9 +6,9 @@ import { describe, expect, test } from "bun:test"; import { validateSectionShapes } from "../../../src/engine/validate.js"; +import { compileFailure } from "../../../src/sections/secret_scanning_custom_patterns/compilable-form.js"; const KEY = "secret_scanning_custom_patterns"; -const REFUSAL = "cannot be compiled as a regular expression"; function issues(entry: Record): readonly string[] { return validateSectionShapes({ [KEY]: [entry] }, "settings.yml").match( @@ -20,20 +20,31 @@ function issues(entry: Record): readonly string[] { const VALID = { name: "internal-token", pattern: "int_[a-z0-9]{8}" }; describe("secret_scanning_custom_patterns regex fields", () => { - test.each([ - ["pattern", { ...VALID, pattern: "([a-z" }, "pattern"], - ["start_delimiter", { ...VALID, start_delimiter: "[" }, "start_delimiter"], - ["end_delimiter", { ...VALID, end_delimiter: "*)" }, "end_delimiter"], - ["must_match", { ...VALID, must_match: ["[A-Z]", "(?, path: string, bad: string]>([ + ["pattern", { ...VALID, pattern: "([a-z" }, "pattern", "([a-z"], + ["start_delimiter", { ...VALID, start_delimiter: "[" }, "start_delimiter", "["], + ["end_delimiter", { ...VALID, end_delimiter: "*)" }, "end_delimiter", "*)"], + ["must_match", { ...VALID, must_match: ["[A-Z]", "(? { - const found = issues(entry); - expect(found).toHaveLength(1); - expect(found[0]).toStartWith(`${KEY}[0].${path}: ${REFUSAL} (`); - expect(found[0]).toContain("Hyperscan"); + "an uncompilable %s is refused at parse with the field path and the engine's reason, not at the bulk-create 422", + (_label, entry, path, bad) => { + expect(issues(entry)).toEqual([ + String(KEY) + + "[0]." + + String(path) + + ": cannot be compiled as a regular expression (" + + String(compileFailure(bad)) + + "); fix the expression, or report a documentation issue if Hyperscan accepts it as written " + + "- the check translates the PCRE-only forms the field docs list before compiling, and " + + "GitHub can still refuse at apply what Hyperscan alone refuses", + ]); }, ); @@ -61,4 +72,28 @@ describe("secret_scanning_custom_patterns regex fields", () => { ])("%s parse clean: a user can declare what GitHub already holds", (_label, fields) => { expect(issues({ ...VALID, ...fields })).toEqual([]); }); + + test.each<[what: string, entry: Record, expected: string[]]>([ + [ + "an empty delimiter, which cannot clear the stored one", + { ...VALID, start_delimiter: "" }, + [ + `${KEY}[0].start_delimiter: a delimiter cannot be cleared with an empty string; remove the pattern and redeclare it without the field instead`, + ], + ], + [ + "a read-only field copied from the GET response", + { ...VALID, state: "enabled" }, + [ + String(KEY) + + '[0] (name "internal-token"): declares "state", which this section does not recognize ' + + "(known keys: name, pattern, start_delimiter, end_delimiter, must_match, must_not_match) - " + + 'the pattern endpoints accept no other field - in particular "state" and ' + + '"push_protection_enabled" are read-only through this API surface - so the key would be ' + + "dropped silently and never converge. Fix the key name, or remove it", + ], + ], + ])("every refusal, as the user reads it: %s", (_what, entry, expected) => { + expect(issues(entry)).toEqual(expected); + }); }); diff --git a/test/sections/teams/schema.test.ts b/test/sections/teams/schema.test.ts index abc2fab2..3793968a 100644 --- a/test/sections/teams/schema.test.ts +++ b/test/sections/teams/schema.test.ts @@ -23,17 +23,17 @@ describe("a team name that is not a slug never reaches the API path", () => { expect(verdict(name)).toEqual({ ok: true }); }); - test.each<[what: string, name: unknown, message: RegExp]>([ + test.each<[what: string, name: unknown, message: string | RegExp]>([ [ "a display name with a space: the slug it usually has is named", "Core Team", - /letters, digits.*; a team named "Core Team" usually has the slug "core-team"$/, + 'teams[0].name: a team is declared by its slug (the name in its URL, /orgs//teams/): letters, digits, ".", "_", and "-" only, at least one letter or digit; a team named "Core Team" usually has the slug "core-team"', ], ["a leading space folded away by the guess", " core", /usually has the slug "core"$/], [ "a path separator, for which no slug can be guessed", "core/team", - /letters, digits.*, and "core\/team" is not one$/, + 'teams[0].name: a team is declared by its slug (the name in its URL, /orgs//teams/): letters, digits, ".", "_", and "-" only, at least one letter or digit, and "core/team" is not one', ], ["an @-prefixed mention", "@core", /and "@core" is not one$/], ["an empty name", "", /and "" is not one$/], @@ -55,7 +55,32 @@ describe("a team name that is not a slug never reaches the API path", () => { ], ])("fails at parse naming the entry and the slug rule: %s", (_what, name, message) => { expect(verdict(name)).toEqual({ - issues: [expect.stringMatching(new RegExp(`^teams\\[0\\]\\.name: .*${message.source}`))], + issues: [ + typeof message === "string" + ? message + : expect.stringMatching(new RegExp(`^teams\\[0\\]\\.name: .*${message.source}`)), + ], }); }); }); + +describe("every teams refusal, as the user reads it", () => { + test.each<[what: string, entry: Record, expected: string[]]>([ + [ + "a misspelled permission key, which would grant the default role instead", + { name: "core", permissions: "admin" }, + [ + 'teams[0] (name "core"): declares "permissions", which this section does not recognize ' + + '(known keys: name, permission) - a misspelled "permission" key would silently grant the ' + + 'default "push" role instead of the intended one. Fix the key name, or remove it', + ], + ], + ])("%s", (_what, entry, expected) => { + expect( + validateSectionShapes({ teams: [entry] }, "settings.yml").match( + () => null, + (p) => p.issues, + ), + ).toEqual(expected); + }); +}); diff --git a/test/sections/webhooks/schema.test.ts b/test/sections/webhooks/schema.test.ts index 482407ed..bdb5dab4 100644 --- a/test/sections/webhooks/schema.test.ts +++ b/test/sections/webhooks/schema.test.ts @@ -63,6 +63,13 @@ describe("webhooks values GitHub would refuse with 422 at apply time are refused 'webhooks[0].config.url: "hooks.example.com/ci" is not an absolute URL (the shape is https://hooks.example.com/ci); GitHub refuses the hook otherwise', ], ], + [ + "a secret at the entry level, which would create an unauthenticated hook", + { config: { url: HOOK_URL }, secret: "$HOOK_SECRET" }, + [ + "webhooks[0].secret: a webhook secret belongs under config.secret, not at the entry level; here it would pass through verbatim and the hook would be created without a working secret", + ], + ], [ "every refused field of one entry, reported together so the fix takes one run", { diff --git a/test/sections/workflows/scenarios/workflows-invalid-state-rejected.yml b/test/sections/workflows/scenarios/workflows-invalid-state-rejected.yml new file mode 100644 index 00000000..56408e5a --- /dev/null +++ b/test/sections/workflows/scenarios/workflows-invalid-state-rejected.yml @@ -0,0 +1,18 @@ +# A workflow state outside the two this section sets (active, disabled) is +# refused when the settings file is parsed, naming the entry and the accepted +# values, before any API contact: no enable or disable call exists for it, so +# apply could only fail late with every other workflow untouched. +name: workflows-invalid-state-rejected +settings: + workflows: + - path: .github/workflows/ci.yml + state: paused +inputs: + mode: apply +expect: + exit_code: 1 + result: failed + zero_requests: true + stdout_contains: + - "workflows[0].state" + - '"active"|"disabled"' diff --git a/test/sections/workflows/schema.test.ts b/test/sections/workflows/schema.test.ts new file mode 100644 index 00000000..2f588635 --- /dev/null +++ b/test/sections/workflows/schema.test.ts @@ -0,0 +1,37 @@ +/** + * The workflows section's own parse refusal, pinned as the problem line a user reads: a key outside the two the + * enable and disable calls read, which send no payload at all. + */ + +import { describe, expect, test } from "bun:test"; +import { validateSectionShapes } from "../../../src/engine/validate.js"; + +function issues(entries: unknown[]): readonly string[] | null { + return validateSectionShapes({ workflows: entries }, "settings.yml").match( + () => null, + (problem) => problem.issues, + ); +} + +describe("a workflow entry key the calls cannot carry is refused at parse", () => { + test.each<[what: string, entry: Record, expected: string[]]>([ + [ + "an enabled flag beside the state, which would silently do nothing", + { path: "ci.yml", state: "active", enabled: true }, + [ + 'workflows[0] (path "ci.yml"): declares "enabled", which this section does not recognize (known keys: path, state) - the enable/disable calls send no payload, so the key would silently do nothing. Fix the key name, or remove it', + ], + ], + ])("%s", (_what, entry, expected) => { + expect(issues([entry])).toEqual(expected); + }); + + test("the two keys the calls read parse, in both states", () => { + expect( + issues([ + { path: "ci.yml", state: "active" }, + { path: "nightly.yml", state: "disabled" }, + ]), + ).toBeNull(); + }); +});