diff --git a/test/e2e/mock/graphql-answer.test.ts b/test/e2e/mock/graphql-answer.test.ts new file mode 100644 index 00000000..3b1ce95c --- /dev/null +++ b/test/e2e/mock/graphql-answer.test.ts @@ -0,0 +1,311 @@ +/** + * The pipeline serves a handler's data as graphql-js completes it under the request's own document, and fails the run + * on data GitHub's schema could never have produced. Without these tests the guard could stop firing and no scenario + * would turn red: the corpus holds only answers on the schema. + */ + +import { describe, expect, spyOn, test } from "bun:test"; +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { stringify as stringifyYaml } from "yaml"; +import { parseRepoSlug } from "../../../src/discovery/targets.js"; +import { SectionSelection } from "../../../src/engine/section-selection.js"; +import { runSingle } from "../../../src/flows/single.js"; +import { GitHubApi } from "../../../src/github/api.js"; +import { silentIo } from "../../../src/io.js"; +import { allGraphqlOps, type SectionGraphqlKey } from "../../../src/sections/registry.js"; +import { withTempDir } from "../../temp-dir.js"; +import { + ADMIN_SLUG, + E2E_TOKEN, + ADMIN_OWNER as OWNER, + ADMIN_REPO as REPO, + VIOLATION_PREFIX, +} from "../constants.js"; +import { type LoggedRequest, newPipelineRunState, type PipelineResult } from "./contract.js"; +import { isWriteRequest } from "./dispatch.js"; +import { completedGraphqlAnswer } from "./graphql-answer.js"; +import { type AcceptedGraphqlRequest, acceptedGraphqlRequest } from "./graphql-request.js"; +import { GRAPHQL_HANDLERS } from "./handlers.js"; +import { mintNodeId } from "./node-id.js"; +import { handleGraphqlRequest } from "./routes.js"; +import { mockServerLifecycle, scenario } from "./server-test-support.js"; +import { buildState, completeRule, ruleWireNode } from "./state.js"; +import type { Json } from "./support.js"; + +const start = mockServerLifecycle(); +const OPS = allGraphqlOps(); +const READ = { owner: OWNER, repo: REPO }; +const DELETE_RULE = { + input: { branchProtectionRuleId: mintNodeId("rule", ADMIN_SLUG, "release/*") }, +}; +const NUMBER_FOR_A_PAYLOAD = + 'data.deleteBranchProtectionRule: answered 7, the schema serves {"clientMutationId":null}'; +const NULL_ADMIN_ENFORCED = + "data.repository.branchProtectionRules.nodes.0.isAdminEnforced: Cannot return null for non-nullable field BranchProtectionRule.isAdminEnforced."; +const NULL_ADMIN_ENFORCED_VIOLATION = `GraphQL BranchProtectionRules was answered off GitHub's schema by the mock handler "branches.rulesQuery": ${NULL_ADMIN_ENFORCED}`; + +/** A wildcard rule node as the mock's own state builds one, `overrides` on top. */ +function ruleNode(overrides: Json = {}): Json { + return ruleWireNode(completeRule({ id: "BPR_1", pattern: "release/*", ...overrides })); +} + +/** The rules read's data holding `nodes`. */ +function rulesData(...nodes: Json[]): Json { + return { + repository: { + branchProtectionRules: { nodes, pageInfo: { hasNextPage: false, endCursor: null } }, + }, + }; +} + +/** The features read's data holding `repository`. */ +function featuresData(repository: Json): Json { + return { repository: { id: "R_1", ...repository } }; +} + +/** The declared operation `key` under `variables`, as GitHub accepts it. */ +function accepted(key: SectionGraphqlKey, variables: Json): AcceptedGraphqlRequest { + const request = acceptedGraphqlRequest(OPS[key].query, OPS[key].name, variables); + if ("errors" in request) { + throw new Error(`${key} is refused by GitHub's schema: ${JSON.stringify(request.errors)}`); + } + return request; +} + +/** The pipeline's result for the declared operation `key` when its handler answers `data`. */ +function served(key: SectionGraphqlKey, variables: Json, data: Json): PipelineResult { + const op = OPS[key]; + const body = { query: op.query, operationName: op.name, variables }; + const run = scenario(); + const log: LoggedRequest = { method: "POST", pathname: "/graphql", query: "", status: 0, body }; + return handleGraphqlRequest( + { method: "POST", body }, + { + scenario: run, + working: { mode: "single", state: buildState(run.live_state, run.owner_kind) }, + checkMode: false, + ...newPipelineRunState(), + }, + log, + OPS, + { ...GRAPHQL_HANDLERS, [key]: () => ({ data }) }, + ); +} + +/** + * One check run of the product's own code, in-process and over the wire, on a settings file that matches the mock's one + * wildcard rule. Returns the run's result and the violations the mock recorded, the list the runner fails a scenario on. + */ +async function checkRun(dir: string): Promise<{ result: string; violations: string[] }> { + const mock = await start( + scenario({ + live_state: { branch_protection_rules: [{ pattern: "release/*", isAdminEnforced: true }] }, + }), + ); + const settingsFile = join(dir, "settings.yml"); + const settings = { branches: [{ name: "release/*", protection: { enforce_admins: true } }] }; + writeFileSync(settingsFile, stringifyYaml(settings)); + const io = silentIo(); + const client = new GitHubApi({ token: E2E_TOKEN, io, baseUrl: mock.url, retryBaseMs: 1 }); + + const outcome = await runSingle( + client, + { + mode: "check", + repo: parseRepoSlug(ADMIN_SLUG)._unsafeUnwrap(), + settingsFile, + selfSlug: ADMIN_SLUG, + privateRepos: "redact", + onMissingPermission: "fail", + sections: SectionSelection.ALL, + privateReport: "none", + reportPublicKey: "", + runUrl: "", + }, + io, + ); + return { result: outcome._unsafeUnwrap().result, violations: mock.violations }; +} + +describe("data GitHub's schema could never serialize does not complete", () => { + test.each([ + { + name: "a null in a non-null field", + key: "branches.rulesQuery", + data: rulesData(ruleNode({ isAdminEnforced: null })), + line: NULL_ADMIN_ENFORCED, + }, + { + name: "a selected non-null field the data lacks", + key: "repository.featuresQuery", + data: featuresData({ issueCreationPolicy: "ALL" }), + line: "data.repository.hasSponsorshipsEnabled: Cannot return null for non-nullable field Repository.hasSponsorshipsEnabled.", + }, + { + name: "a value off its enum", + key: "repository.featuresQuery", + data: featuresData({ hasSponsorshipsEnabled: false, issueCreationPolicy: "all" }), + line: 'data.repository.issueCreationPolicy: Enum "IssueCreationPolicy" cannot represent value: "all"', + }, + { + name: "1.5 for an Int", + key: "branches.rulesQuery", + data: rulesData(ruleNode({ requiredApprovingReviewCount: 1.5 })), + line: "data.repository.branchProtectionRules.nodes.0.requiredApprovingReviewCount: Int cannot represent non-integer value: 1.5", + }, + { + // The line goes on with graphql-js's advice to its own caller. + name: "an abstract value that names no type", + key: "branches.rulesQuery", + data: rulesData({ + ...ruleNode(), + bypassForcePushAllowances: { + nodes: [{ actor: { login: "octocat" } }], + pageInfo: { hasNextPage: false }, + }, + }), + line: 'data.repository.branchProtectionRules.nodes.0.bypassForcePushAllowances.nodes.0.actor: Abstract type "BranchActorAllowanceActor" must resolve to an Object type at runtime', + }, + ] as const)("$name", ({ key, data, line }) => { + expect(completedGraphqlAnswer(accepted(key, READ), data)).toEqual({ + errors: [expect.stringContaining(line)], + }); + }); + + test("control: data on the schema completes to itself, a union member named by its __typename included", () => { + const data = rulesData(ruleNode({ bypassForcePushActors: ["octocat"] })); + expect(completedGraphqlAnswer(accepted("branches.rulesQuery", READ), data)).toEqual({ data }); + }); +}); + +// graphql-js completes each of these without an error: an object of nulls for a non-object, the coerced value for a leaf. +describe("data that completes to anything but itself is no answer of GitHub's schema", () => { + test.each([ + { + name: "a number at an object position", + key: "branches.deleteRule", + variables: DELETE_RULE, + data: { deleteBranchProtectionRule: 7 }, + line: NUMBER_FOR_A_PAYLOAD, + }, + { + name: "an array at an object position", + key: "branches.deleteRule", + variables: DELETE_RULE, + data: { deleteBranchProtectionRule: [] }, + line: 'data.deleteBranchProtectionRule: answered [], the schema serves {"clientMutationId":null}', + }, + { + name: "a string as the whole data", + key: "environments.pins", + variables: READ, + data: "not an object", + line: 'data: answered "not an object", the schema serves {"repository":null}', + }, + { + name: "a string inside a list of objects", + key: "branches.rulesQuery", + variables: READ, + data: rulesData({ + ...ruleNode(), + bypassForcePushAllowances: { nodes: ["octocat"], pageInfo: { hasNextPage: false } }, + }), + line: 'data.repository.branchProtectionRules.nodes.0.bypassForcePushAllowances.nodes.0: answered "octocat", the schema serves {"actor":null}', + }, + { + name: "true for an Int", + key: "branches.rulesQuery", + variables: READ, + data: rulesData(ruleNode({ requiredApprovingReviewCount: true })), + line: "data.repository.branchProtectionRules.nodes.0.requiredApprovingReviewCount: answered true, the schema serves 1", + }, + { + name: "5 for an ID", + key: "repository.featuresQuery", + variables: READ, + data: { repository: { id: 5, hasSponsorshipsEnabled: false, issueCreationPolicy: "ALL" } }, + line: 'data.repository.id: answered 5, the schema serves "5"', + }, + ] as const)("$name", ({ key, variables, data, line }) => { + expect(completedGraphqlAnswer(accepted(key, variables), data)).toEqual({ errors: [line] }); + }); +}); + +describe("the product receives the completed data, and its two differences from the handler's data are no violation", () => { + test.each([ + { + name: "a key the document does not select is dropped", + key: "repository.featuresQuery", + data: featuresData({ + hasSponsorshipsEnabled: true, + issueCreationPolicy: "ALL", + nameWithOwner: `${OWNER}/${REPO}`, + }), + received: featuresData({ hasSponsorshipsEnabled: true, issueCreationPolicy: "ALL" }), + }, + { + name: "a selected nullable key the handler leaves out is null", + key: "environments.pins", + data: { repository: { pinnedEnvironments: { nodes: [], pageInfo: { hasNextPage: false } } } }, + received: { + repository: { + pinnedEnvironments: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } }, + }, + }, + }, + { + name: "control: data that is the selection already is served as it is", + key: "branches.rulesQuery", + data: rulesData(ruleNode()), + received: rulesData(ruleNode()), + }, + ] as const)("$name", ({ key, data, received }) => { + const result = served(key, READ, data); + expect(result.violation).toBeUndefined(); + expect(result.response).toEqual({ status: 200, body: { data: received } }); + }); +}); + +describe("data off the schema fails the run", () => { + test("the violation names the operation, the handler, the path, and graphql-js's reason, and the read stays a read in the log", () => { + const result = served( + "branches.rulesQuery", + READ, + rulesData(ruleNode({ isAdminEnforced: null })), + ); + expect(result.violation).toBe(NULL_ADMIN_ENFORCED_VIOLATION); + // The product is answered the violation, never the data. + expect(result.response).toEqual({ + status: 400, + body: { message: `${VIOLATION_PREFIX} ${NULL_ADMIN_ENFORCED_VIOLATION}` }, + }); + // Logged without its GraphQL kind, the POST would count as a write in the runner's check-mode and idempotence rules. + expect(result.log.graphql).toEqual({ operationName: "BranchProtectionRules", kind: "read" }); + expect(isWriteRequest(result.log)).toBe(false); + }); + + test("data that completes to another value is a violation naming the path and both values", () => { + const result = served("branches.deleteRule", DELETE_RULE, { deleteBranchProtectionRule: 7 }); + const violation = `GraphQL DeleteBranchProtectionRule was answered off GitHub's schema by the mock handler "branches.deleteRule": ${NUMBER_FOR_A_PAYLOAD}`; + expect(result.violation).toBe(violation); + expect(result.response).toEqual({ + status: 400, + body: { message: `${VIOLATION_PREFIX} ${violation}` }, + }); + }); + + test("inside a run of the product: a handler answering a null in a Boolean! field fails the run and records the violation, and the handlers as they are run clean (the control)", () => + withTempDir("graphql-answer-run-", async (dir) => { + expect(await checkRun(dir)).toEqual({ result: "clean", violations: [] }); + + using mutant = spyOn(GRAPHQL_HANDLERS, "branches.rulesQuery").mockImplementation(() => ({ + data: rulesData(ruleNode({ isAdminEnforced: null })), + })); + const refused = await checkRun(dir); + expect(mutant).toHaveBeenCalled(); + expect(refused.result).toBe("failed"); + // Every rules read of the run earns the line; how many the product makes is not this test's subject. + expect(new Set(refused.violations)).toEqual(new Set([NULL_ADMIN_ENFORCED_VIOLATION])); + })); +}); diff --git a/test/e2e/mock/graphql-answer.ts b/test/e2e/mock/graphql-answer.ts new file mode 100644 index 00000000..7d541ad7 --- /dev/null +++ b/test/e2e/mock/graphql-answer.ts @@ -0,0 +1,82 @@ +/** + * A handler's data as GitHub's schema serves it: graphql-js completes it under the request's own document, and the + * completed data must be the handler's own at every selected position, except null under a nullable key the handler + * left out. Without it a key the document never selected reaches the product, and an answer off the schema is judged + * only where the product parses it, as the product's error. + */ + +import { executeRootSelectionSet, type GraphQLFieldResolver } from "graphql"; +import { isPlainObject } from "../../../src/plain-data.js"; +import type { AcceptedGraphqlRequest } from "./graphql-request.js"; +import type { Json } from "./support.js"; + +/** A handler's data is keyed as the response is, so a field is read under its response key, an alias included. */ +const answeredField: GraphQLFieldResolver = (source, _args, _context, info) => + isPlainObject(source) ? source[info.path.key] : undefined; + +/** + * Where `completed` is not what the handler `answered`, one line per position: the path and both values. Completion + * alone lets these through: a string where an object belongs completes to an object of nulls, `true` in an Int field + * to 1. The one difference allowed is null under a nullable key the handler left out. + */ +function differences(answered: unknown, completed: unknown, path: string): string[] { + if (completed === null && answered === undefined) { + return []; + } + + if (Array.isArray(completed) && Array.isArray(answered) && answered.length === completed.length) { + return completed.flatMap((item, index) => + differences(answered[index], item, `${path}.${index}`), + ); + } + + if (isPlainObject(completed) && isPlainObject(answered)) { + return Object.entries(completed).flatMap(([key, value]) => + differences(answered[key], value, `${path}.${key}`), + ); + } + + if (answered === completed) { + return []; + } + return [ + `${path}: answered ${JSON.stringify(answered)}, the schema serves ${JSON.stringify(completed)}`, + ]; +} + +/** + * `data` as GitHub would serialize it for `request`: the selection and nothing beside it, a selected nullable field the + * data lacks as null. Data GitHub's schema could never have produced returns one line per fault instead: graphql-js's + * error at its response path, or a position where the completed value is not the handler's own. + */ +export function completedGraphqlAnswer( + request: AcceptedGraphqlRequest, + data: unknown, +): { data: Json } | { errors: readonly string[] } { + const completed = executeRootSelectionSet({ + ...request, + rootValue: data, + fieldResolver: answeredField, + }); + if (completed instanceof Promise) { + throw new Error("BUG: graphql-js completed plain data asynchronously"); + } + + const errors = completed.errors ?? []; + if (errors.length > 0) { + return { + errors: errors.map((error) => { + const path = ["data", ...(error.path ?? [])].join("."); + return `${path}: ${error.message}`; + }), + }; + } + + const differing = differences(data, completed.data, "data"); + if (differing.length > 0) { + return { errors: differing }; + } + + // graphql-js leaves `data` unset only beside errors. + return { data: completed.data as Json }; +} diff --git a/test/e2e/mock/graphql-request.ts b/test/e2e/mock/graphql-request.ts index ae2a8ef0..13c27317 100644 --- a/test/e2e/mock/graphql-request.ts +++ b/test/e2e/mock/graphql-request.ts @@ -9,11 +9,15 @@ import { GraphQLError, type GraphQLFormattedError, parse, + type ValidatedExecutionArgs, validate, validateExecutionArgs, } from "graphql"; import { sharedGraphqlSchema } from "./graphql-schema.js"; +/** A request GitHub's schema accepts, as graphql-js prepared it to run: the parsed document, its operation, the coerced variables. */ +export type AcceptedGraphqlRequest = ValidatedExecutionArgs; + /** The document, or the syntax error graphql-js reports by throwing it. */ function parsedDocument(query: string): DocumentNode | GraphQLError { try { @@ -27,24 +31,24 @@ function parsedDocument(query: string): DocumentNode | GraphQLError { } /** - * The errors GitHub answers `operationName` of `query` with under `variables`, or null when it accepts the request. - * graphql-js words them; api.github.com words the same refusals its own way. Coercion only judges: a caller keeps - * handing on the variables as sent. + * `operationName` of `query` under `variables` as GitHub accepts it, or the errors GitHub answers it with. graphql-js + * words them; api.github.com words the same refusals its own way. Coercion only judges: a caller keeps handing on the + * variables as sent. */ -export function graphqlRequestErrors( +export function acceptedGraphqlRequest( query: string, operationName: string, variables: Readonly>, -): readonly GraphQLFormattedError[] | null { +): AcceptedGraphqlRequest | { errors: readonly GraphQLFormattedError[] } { const document = parsedDocument(query); if (document instanceof GraphQLError) { - return [document.toJSON()]; + return { errors: [document.toJSON()] }; } const schema = sharedGraphqlSchema(); const documentErrors = validate(schema, document); if (documentErrors.length > 0) { - return documentErrors.map((error) => error.toJSON()); + return { errors: documentErrors.map((error) => error.toJSON()) }; } const prepared = validateExecutionArgs({ @@ -53,5 +57,15 @@ export function graphqlRequestErrors( operationName, variableValues: variables, }); - return "operation" in prepared ? null : prepared.map((error) => error.toJSON()); + return "operation" in prepared ? prepared : { errors: prepared.map((error) => error.toJSON()) }; +} + +/** The errors of acceptedGraphqlRequest(), or null when GitHub accepts the request. */ +export function graphqlRequestErrors( + query: string, + operationName: string, + variables: Readonly>, +): readonly GraphQLFormattedError[] | null { + const request = acceptedGraphqlRequest(query, operationName, variables); + return "errors" in request ? request.errors : null; } diff --git a/test/e2e/mock/routes.ts b/test/e2e/mock/routes.ts index 73ca925b..fbb3a34a 100644 --- a/test/e2e/mock/routes.ts +++ b/test/e2e/mock/routes.ts @@ -9,6 +9,7 @@ * GraphQL has no body allowlist: its schema check follows the route match, as GitHub validates before it runs anything. */ +import type { GraphQLFormattedError } from "graphql"; import { isPlainObject } from "../../../src/plain-data.js"; import type { SectionKey } from "../../../src/schema.js"; import { endpointPath, toleratedStatuses } from "../../../src/sections/contract/endpoints.js"; @@ -58,7 +59,8 @@ import { type Requirement, SECTION_BY_KEY, } from "./grading.js"; -import { graphqlRequestErrors } from "./graphql-request.js"; +import { completedGraphqlAnswer } from "./graphql-answer.js"; +import { type AcceptedGraphqlRequest, acceptedGraphqlRequest } from "./graphql-request.js"; import { GRAPHQL_HANDLERS, HANDLERS } from "./handlers.js"; import { decodeNodeId, type NodeFamily } from "./node-id.js"; import { acceptedBody } from "./request-body.js"; @@ -67,7 +69,6 @@ import { asObject, type GraphqlErrorReply, type GraphqlHandler, - type GraphqlHandlerResult, type Json, type MockResponse, } from "./support.js"; @@ -274,20 +275,15 @@ function parseGraphqlRequest( } /** - * The schema check. GitHub answers a document off its schema, or variables off the operation's declared types, with - * HTTP 200 carrying errors and no data. The action shapes every document and variable itself, so the refusal is a - * violation too: the scenario fails whatever outcome it expected. + * The schema check's refusal. GitHub answers a document off its schema, or variables off the operation's declared + * types, with HTTP 200 carrying errors and no data. The action shapes every document and variable itself, so the + * refusal is a violation too: the scenario fails whatever outcome it expected. */ function graphqlSchemaRefusal( op: TaggedGraphqlOp, - query: string, - variables: Json, + errors: readonly GraphQLFormattedError[], log: LoggedRequest, -): PipelineResult | null { - const errors = graphqlRequestErrors(query, op.name, variables); - if (errors === null) { - return null; - } +): PipelineResult { return { response: { status: 200, body: { errors } }, log: { ...log, status: 200 }, @@ -331,23 +327,43 @@ function graphqlDenial( return { response, log: denied, violation: barrierViolation }; } -/** Response guard, the status-subset analog: an undeclared error type is a mock design bug, not a scenario outcome. */ -function graphqlResponse( +/** Response guard of an `errors` answer, the status-subset analog: an undeclared error type is a mock design bug, not a scenario outcome. */ +function graphqlErrorsResponse( key: string, op: TaggedGraphqlOp, - result: GraphqlHandlerResult, -): MockResponse | { violation: string } { - if (result.errors === undefined) { - return { status: 200, body: { data: result.data } }; - } + errors: readonly GraphqlErrorReply[], + violation: (message: string) => PipelineResult, +): MockResponse | PipelineResult { const declared = toleratedGraphqlErrors(op); - const undeclared = result.errors.filter((entry) => !declared.includes(entry.type)); + const undeclared = errors.filter((entry) => !declared.includes(entry.type)); if (undeclared.length > 0) { - return { - violation: `GraphQL handler "${key}" answered undeclared error type(s) [${undeclared.map((e) => `${e.type}: "${e.message}"`).join(", ")}]; the operation declares only [${declared.join(", ")}] as tolerated outcomes`, - }; + return violation( + `GraphQL handler "${key}" answered undeclared error type(s) [${undeclared.map((e) => `${e.type}: "${e.message}"`).join(", ")}]; the operation declares only [${declared.join(", ")}] as tolerated outcomes`, + ); } - return { status: 200, body: { data: null, errors: result.errors } }; + return { status: 200, body: { data: null, errors } }; +} + +/** + * Response guard of a `data` answer. The product receives what graphql-js completes from the handler's data under the + * request's own document, as from GitHub: no key the document did not select, and null under a selected nullable key + * the handler left out. Data that does not complete, or differs from its completion in any other way, is the mock's + * own fault: a violation, whatever outcome the scenario expected. + */ +function graphqlDataResponse( + key: string, + op: TaggedGraphqlOp, + request: AcceptedGraphqlRequest, + data: Json, + violation: (message: string) => PipelineResult, +): MockResponse | PipelineResult { + const completed = completedGraphqlAnswer(request, data); + if ("errors" in completed) { + return violation( + `GraphQL ${op.name} was answered off GitHub's schema by the mock handler "${key}": ${completed.errors.join("; ")}`, + ); + } + return { status: 200, body: { data: completed.data } }; } /** @@ -373,9 +389,9 @@ export function handleGraphqlRequest( }; // Ahead of every barrier: no fault, denial, or handler answers a request GitHub refuses before running it. - const refused = graphqlSchemaRefusal(op, query, variables, graphqlLog); - if (refused) { - return refused; + const accepted = acceptedGraphqlRequest(query, op.name, variables); + if ("errors" in accepted) { + return graphqlSchemaRefusal(op, accepted.errors, graphqlLog); } // Check-mode barrier before the fault barrier: a synthetic fault must not mask the one bug this barrier exists to catch. @@ -413,9 +429,14 @@ export function handleGraphqlRequest( // Unreachable after assertGraphqlHandlerCompleteness at construction; loud rather than a silent undefined call. return violation(`no GraphQL handler registered for dispatched operation "${key}"`); } - const response = graphqlResponse(key, op, handler({ state: target.state, op, variables })); - if ("violation" in response) { - return violation(response.violation); + const result = handler({ state: target.state, op, variables }); + // Data off the schema is refused under the operation's own log entry, so the runner still reads a read as a read. + const response = + result.errors === undefined + ? graphqlDataResponse(key, op, accepted, result.data, violationFor(graphqlLog)) + : graphqlErrorsResponse(key, op, result.errors, violation); + if ("log" in response) { + return response; } const corrupted = takeCorruption(key, options, response, graphqlLog);