Commit8de7b28
test: close the removable-guard hole across the security suite
test: close the removable-guard hole across the security suite
Follow-up to 6ae664d, which fixed this only in the six files written
during the audit. Swept all 38 source-asserting test files: 21 used a
bare `toContain("identifier")`, and six of those were guarding
security-critical call sites, where `void guard;` satisfies the assertion
while the guard does nothing.
Tightened to the call form:
resolveRepoAccess — api-v2-repo-privacy, private-repo-subpage-gate,
fork-privacy (the core authorization primitive)
buildRunnerEnv — secret-egress
resolvesToPrivate — webhook-ssrf-redirect
evaluateLockout — auth-logout-2fa
Left bare where bare is correct, same as before: `not.toContain(...)`
absence checks, constants (RUNNER_ENV_ALLOWLIST, MAX_REDIRECTS,
LOGIN_FAIL_WINDOW_MS), column names and the `catch` keyword.
Proven on the two that matter most, not assumed:
- Replacing preview-builder's `buildRunnerEnv({ CI: "1" })` with
`process.env` — which is verbatim the command-injection exposure that
audit found, handing a user-supplied build command every secret in
the environment — now fails secret-egress. Under the old assertion it
passed, because the reverted expression still mentions the function.
- Replacing fork.tsx's access check with a hardcoded "read" — one POST
turning someone else's private repo into a public fork — now fails
fork-privacy. It also passed before.
Both reverted; the diff is tests only.
Noted while running: this group timed out once on a cold first run
(11.7s) then completed in 209ms with 0 fail. Transient, unrelated to the
change, but recorded rather than ignored.
Suite: 3524 pass, 0 fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>6 files changed+7−78de7b2876fc36be7e90d7fc32f9b4c5b1139b98d
6 changed files+7−7
Modifiedsrc/__tests__/api-v2-repo-privacy.test.ts+2−2View fileUnifiedSplit
@@ -44,7 +44,7 @@ describe("api-v2 repo privacy gate", () => {
4444 );
4545 // An ownerId equality check would wrongly deny collaborators and org
4646 // members — that was the flaw in the pre-existing inline checks.
47 expect(gate).toContain("resolveRepoAccess");
47 expect(gate).toContain("resolveRepoAccess(");
4848 expect(gate).not.toMatch(/user\.id\s*!==\s*\w*[Oo]wner/);
4949 });
5050
@@ -87,7 +87,7 @@ describe("GET /api/users/:username/repos does not leak private repos", () => {
8787 );
8888
8989 it("filters each row through resolveRepoAccess", () => {
90 expect(handler).toContain("resolveRepoAccess");
90 expect(handler).toContain("resolveRepoAccess(");
9191 expect(handler).toContain('access === "none"');
9292 });
9393
Modifiedsrc/__tests__/auth-logout-2fa.test.ts+1−1View fileUnifiedSplit
@@ -120,7 +120,7 @@ describe("2FA failures are counted per account, not just per IP", () => {
120120 it("reuses the password step's window and threshold", () => {
121121 // One threshold across both factors rather than a second, divergent one.
122122 expect(handler).toContain("LOGIN_FAIL_WINDOW_MS");
123 expect(handler).toContain("evaluateLockout");
123 expect(handler).toContain("evaluateLockout(");
124124 });
125125
126126 it("fails OPEN if the lockout lookup errors", () => {
Modifiedsrc/__tests__/fork-privacy.test.ts+1−1View fileUnifiedSplit
@@ -17,7 +17,7 @@ const handler = SRC.slice(SRC.indexOf('fork.post("/:owner/:repo/fork"'));
1717
1818describe("fork access gate", () => {
1919 it("checks read access via resolveRepoAccess before cloning", () => {
20 expect(handler).toContain("resolveRepoAccess");
20 expect(handler).toContain("resolveRepoAccess(");
2121 expect(handler).toContain('access === "none"');
2222 });
2323
Modifiedsrc/__tests__/private-repo-subpage-gate.test.ts+1−1View fileUnifiedSplit
@@ -56,7 +56,7 @@ describe("deny behaviour", () => {
5656 });
5757
5858 it("uses resolveRepoAccess so collaborators and org members still pass", () => {
59 expect(gate).toContain("resolveRepoAccess");
59 expect(gate).toContain("resolveRepoAccess(");
6060 // An ownerId equality check would lock out collaborators.
6161 expect(gate).not.toMatch(/user\.id\s*!==/);
6262 });
Modifiedsrc/__tests__/secret-egress.test.ts+1−1View fileUnifiedSplit
@@ -84,7 +84,7 @@ describe("no secret is named in a user-code path's env construction", () => {
8484describe("the shared allowlist is the only way user code gets an env", () => {
8585 it("preview-builder uses buildRunnerEnv rather than its own env object", () => {
8686 const src = readFileSync("src/lib/preview-builder.ts", "utf8");
87 expect(src).toContain("buildRunnerEnv");
87 expect(src).toContain("buildRunnerEnv(");
8888 });
8989
9090 it("workflow-runner still filters through allowlist AND denylist", () => {
Modifiedsrc/__tests__/webhook-ssrf-redirect.test.ts+1−1View fileUnifiedSplit
@@ -67,7 +67,7 @@ describe("redirects cannot escape the guard", () => {
6767describe("the DNS layer is actually wired in", () => {
6868 it("webhook delivery calls resolvesToPrivate", () => {
6969 // It had zero production callers before this.
70 expect(SRC).toContain("resolvesToPrivate");
70 expect(SRC).toContain("resolvesToPrivate(");
7171 expect(fn).toContain("await resolvesToPrivate(guard.url.hostname)");
7272 });
7373
7474