From ad205fd7a402e5b2c0fabc6d987302b2a3ec13dc Mon Sep 17 00:00:00 2001 From: rikohomeless <163776849+rikohomeless@users.noreply.github.com> Date: Thu, 14 May 2026 16:50:47 +0000 Subject: [PATCH 1/3] fix(auth): remove github delete repo scope --- .../src/services/auth-github-login-stream.ts | 66 ++++-- .../tests/auth-github-login-stream.test.ts | 10 + packages/app/src/lib/usecases/auth-github.ts | 63 ++++-- .../src/lib/usecases/github-scope-policy.ts | 58 +++++ .../lib/usecases/github-token-validation.ts | 17 +- packages/lib/src/usecases/auth-github.ts | 63 ++++-- .../lib/src/usecases/github-scope-policy.ts | 56 +++++ .../src/usecases/github-token-validation.ts | 17 +- .../usecases/auth-container-paths.test.ts | 214 ++++++++++++++++-- .../usecases/github-scope-policy.test.ts | 34 +++ 10 files changed, 502 insertions(+), 96 deletions(-) create mode 100644 packages/app/src/lib/usecases/github-scope-policy.ts create mode 100644 packages/lib/src/usecases/github-scope-policy.ts create mode 100644 packages/lib/tests/usecases/github-scope-policy.test.ts diff --git a/packages/api/src/services/auth-github-login-stream.ts b/packages/api/src/services/auth-github-login-stream.ts index 08958ab4..004530e8 100644 --- a/packages/api/src/services/auth-github-login-stream.ts +++ b/packages/api/src/services/auth-github-login-stream.ts @@ -4,10 +4,16 @@ import * as FileSystem from "@effect/platform/FileSystem" import * as Path from "@effect/platform/Path" import { defaultTemplateConfig } from "@effect-template/lib/core/template-defaults" import { buildDockerAuthArgs, resolveDockerVolumeHostPath, runDockerAuthCapture } from "@effect-template/lib/shell/docker-auth" -import { CommandFailedError } from "@effect-template/lib/shell/errors" +import { AuthError, CommandFailedError } from "@effect-template/lib/shell/errors" import { buildDockerAuthSpec, normalizeAccountLabel } from "@effect-template/lib/usecases/auth-helpers" import { ensureEnvFile, readEnvText, upsertEnvKey } from "@effect-template/lib/usecases/env-file" import { ensureGhAuthImage, ghAuthDir, ghAuthRoot, ghImageName } from "@effect-template/lib/usecases/github-auth-image" +import { + githubForbiddenDeleteRepoScopeMessage, + hasGithubRepositoryDeleteScope, + normalizeGithubScopes +} from "@effect-template/lib/usecases/github-scope-policy" +import { validateGithubToken } from "@effect-template/lib/usecases/github-token-validation" import { resolvePathFromCwd } from "@effect-template/lib/usecases/path-helpers" import { autoSyncState } from "@effect-template/lib/usecases/state-repo" import { ensureStateDotDockerGitRepo } from "@effect-template/lib/usecases/state-repo-github" @@ -20,7 +26,7 @@ import type { GithubAuthLoginRequest } from "../api/contracts.js" import { ApiBadRequestError, ApiInternalError } from "../api/errors.js" type GithubRuntime = FileSystem.FileSystem | Path.Path | CommandExecutor.CommandExecutor -type GithubSetupError = CommandFailedError | PlatformError +type GithubSetupError = AuthError | CommandFailedError | PlatformError type PreparedGithubLogin = { readonly cwd: string @@ -36,7 +42,6 @@ const githubLoginStreamSuccessMarker = "__DOCKER_GIT_GITHUB_LOGIN_STATUS__:ok" const githubLoginStreamErrorMarkerPrefix = "__DOCKER_GIT_GITHUB_LOGIN_STATUS__:error:" const githubTokenKey = "GITHUB_TOKEN" const githubTokenPrefix = "GITHUB_TOKEN__" -const defaultGithubScopes = "repo,workflow,read:org" const ensureGithubOrchLayout = ( cwd: string, @@ -71,25 +76,19 @@ const buildGithubTokenKey = (label: string | null): string => { const labelFromKey = (key: string): string => key.startsWith(githubTokenPrefix) ? key.slice(githubTokenPrefix.length) : "default" -const normalizeGithubScopes = (value: string | null | undefined): ReadonlyArray => { - const raw = value?.trim() ?? "" - const input = raw.length === 0 ? defaultGithubScopes : raw - const scopes = input - .split(/[,\s]+/g) - .map((scope) => scope.trim()) - .filter((scope) => scope.length > 0 && scope !== "delete_repo") - return scopes.length === 0 ? defaultGithubScopes.split(",") : scopes -} - const toApiError = (error: GithubSetupError): ApiBadRequestError | ApiInternalError => - error._tag === "CommandFailedError" + error._tag === "AuthError" ? new ApiBadRequestError({ - message: `${error.command} failed (exit ${error.exitCode}).` - }) - : new ApiInternalError({ - message: String(error), - cause: error + message: error.message }) + : error._tag === "CommandFailedError" + ? new ApiBadRequestError({ + message: `${error.command} failed (exit ${error.exitCode}).` + }) + : new ApiInternalError({ + message: String(error), + cause: error + }) const prepareGithubLogin = ( request: GithubAuthLoginRequest @@ -171,6 +170,32 @@ const resolveGithubToken = ( ) ) +const runGithubRemoveDeleteRepoScope = ( + cwd: string, + accountPath: string +): Effect.Effect => + runDockerAuthCapture( + buildDockerAuthSpec({ + cwd, + image: ghImageName, + hostPath: accountPath, + containerPath: ghAuthDir, + env: ["BROWSER=echo", `GH_CONFIG_DIR=${ghAuthDir}`], + args: ["auth", "refresh", "-h", "github.com", "--remove-scopes", "delete_repo"], + interactive: false + }), + [0], + (exitCode) => new CommandFailedError({ command: "gh auth refresh --remove-scopes delete_repo", exitCode }) + ).pipe(Effect.asVoid) + +const rejectGithubTokenWithRepositoryDeleteScope = (token: string): Effect.Effect => + validateGithubToken(token).pipe( + Effect.flatMap((validation) => + hasGithubRepositoryDeleteScope(validation.oauthScopes) + ? Effect.fail(new AuthError({ message: githubForbiddenDeleteRepoScopeMessage })) + : Effect.void) + ) + const persistGithubToken = ( fs: FileSystem.FileSystem, envPath: string, @@ -216,7 +241,10 @@ const finalizeGithubLogin = ( Effect.gen(function*(_) { const fs = yield* _(FileSystem.FileSystem) const path = yield* _(Path.Path) + yield* _(Effect.log("Removing repository delete scope from GH auth token...")) + yield* _(runGithubRemoveDeleteRepoScope(prepared.cwd, prepared.accountPath).pipe(Effect.mapError(toApiError))) const token = yield* _(resolveGithubToken(prepared.cwd, prepared.accountPath).pipe(Effect.mapError(toApiError))) + yield* _(rejectGithubTokenWithRepositoryDeleteScope(token).pipe(Effect.mapError(toApiError))) yield* _(ensureEnvFile(fs, path, prepared.envPath).pipe(Effect.mapError(toApiError))) yield* _(persistGithubToken(fs, prepared.envPath, prepared.key, token).pipe(Effect.mapError(toApiError))) yield* _(ensureStateDotDockerGitRepo(token)) diff --git a/packages/api/tests/auth-github-login-stream.test.ts b/packages/api/tests/auth-github-login-stream.test.ts index 015556a6..d4b275b1 100644 --- a/packages/api/tests/auth-github-login-stream.test.ts +++ b/packages/api/tests/auth-github-login-stream.test.ts @@ -16,6 +16,16 @@ describe("GitHub auth login stream", () => { expect(output.indexOf("State dir ready")).toBeLessThan(output.indexOf("GitHub login completed.")) }) + it("renders repository delete scope refresh output before the success marker", () => { + const output = renderGithubPostLoginOutput([ + "Removing repository delete scope from GH auth token..." + ], "ok") + + expect(output).toContain("Removing repository delete scope") + expect(output).toContain("__DOCKER_GIT_GITHUB_LOGIN_STATUS__:ok") + expect(output.indexOf("Removing repository delete scope")).toBeLessThan(output.indexOf("GitHub login completed.")) + }) + it("renders post-login failure details before the failure marker", () => { const output = renderGithubPostLoginOutput([ "GitHub login finished in browser, but post-login sync failed: git fetch failed" diff --git a/packages/app/src/lib/usecases/auth-github.ts b/packages/app/src/lib/usecases/auth-github.ts index 41c55a15..3d11c43d 100644 --- a/packages/app/src/lib/usecases/auth-github.ts +++ b/packages/app/src/lib/usecases/auth-github.ts @@ -10,12 +10,16 @@ import type { AuthGithubLoginCommand, AuthGithubLogoutCommand, AuthGithubStatusC import { defaultTemplateConfig } from "../core/domain.js" import { trimLeftChar, trimRightChar } from "../core/strings.js" import { runDockerAuth, runDockerAuthCapture } from "../shell/docker-auth.js" -import type { AuthError } from "../shell/errors.js" -import { CommandFailedError } from "../shell/errors.js" +import { AuthError, CommandFailedError } from "../shell/errors.js" import { buildDockerAuthSpec, normalizeAccountLabel } from "./auth-helpers.js" import { migrateLegacyOrchLayout } from "./auth-sync.js" import { ensureEnvFile, parseEnvEntries, readEnvText, removeEnvKey, upsertEnvKey } from "./env-file.js" import { ensureGhAuthImage, ghAuthDir, ghAuthRoot, ghImageName } from "./github-auth-image.js" +import { + githubForbiddenDeleteRepoScopeMessage, + hasGithubRepositoryDeleteScope, + normalizeGithubScopes +} from "./github-scope-policy.js" import type { GithubTokenValidationResult } from "./github-token-validation.js" import { validateGithubToken } from "./github-token-validation.js" import { resolvePathFromCwd } from "./path-helpers.js" @@ -86,28 +90,6 @@ const listGithubTokens = (envText: string): ReadonlyArray => })) .filter((entry) => entry.token.trim().length > 0) -const defaultGithubScopes = "repo,workflow,read:org" - -// CHANGE: normalize GitHub scopes for gh auth login -// WHY: ensure required scopes are requested without delete_repo -// QUOTE(ТЗ): "Передай все нужные скопы" -// REF: user-request-2026-02-05-gh-scopes -// SOURCE: n/a -// FORMAT THEOREM: ∀s: normalize(s) -> scopes(s) ⊆ required -// PURITY: CORE -// EFFECT: n/a -// INVARIANT: empty input yields default scopes -// COMPLEXITY: O(n) where n = |scopes| -const normalizeGithubScopes = (value: string | null | undefined): ReadonlyArray => { - const raw = value?.trim() ?? "" - const input = raw.length === 0 ? defaultGithubScopes : raw - const scopes = input - .split(/[,\s]+/g) - .map((scope) => scope.trim()) - .filter((scope) => scope.length > 0 && scope !== "delete_repo") - return scopes.length === 0 ? defaultGithubScopes.split(",") : scopes -} - const withEnvContext = ( envGlobalPath: string, run: (context: EnvContext) => Effect.Effect @@ -144,7 +126,8 @@ const validateGithubTokenEntry = ( label: entry.label, token: entry.token, status: validation.status, - login: validation.login + login: validation.login, + oauthScopes: validation.oauthScopes })) ) @@ -200,6 +183,32 @@ const runGithubLogin = ( (exitCode) => new CommandFailedError({ command: "gh auth login --web", exitCode }) ) +const runGithubRemoveDeleteRepoScope = ( + cwd: string, + accountPath: string +): Effect.Effect => + runDockerAuth( + buildDockerAuthSpec({ + cwd, + image: ghImageName, + hostPath: accountPath, + containerPath: ghAuthDir, + env: ["BROWSER=echo", `GH_CONFIG_DIR=${ghAuthDir}`], + args: ["auth", "refresh", "-h", "github.com", "--remove-scopes", "delete_repo"], + interactive: false + }), + [0], + (exitCode) => new CommandFailedError({ command: "gh auth refresh --remove-scopes delete_repo", exitCode }) + ) + +const rejectGithubTokenWithRepositoryDeleteScope = (token: string): Effect.Effect => + validateGithubToken(token).pipe( + Effect.flatMap((validation) => + hasGithubRepositoryDeleteScope(validation.oauthScopes) + ? Effect.fail(new AuthError({ message: githubForbiddenDeleteRepoScopeMessage })) + : Effect.void) + ) + const retryGithubLogin = ( effect: Effect.Effect ): Effect.Effect => @@ -243,7 +252,10 @@ const runGithubInteractiveLogin = ( yield* _(ensureGhAuthImage(fs, path, cwd, "gh auth")) yield* _(Effect.log(`Starting GH auth login in container (scopes: ${scopes.join(", ")})...`)) yield* _(retryGithubLogin(runGithubLogin(cwd, accountPath, scopes))) + yield* _(Effect.log("Removing repository delete scope from GH auth token...")) + yield* _(runGithubRemoveDeleteRepoScope(cwd, accountPath)) const resolved = yield* _(resolveGithubTokenFromGh(cwd, accountPath)) + yield* _(rejectGithubTokenWithRepositoryDeleteScope(resolved)) yield* _(ensureEnvFile(fs, path, envPath)) const key = buildGithubTokenKey(command.label) yield* _(persistGithubToken(fs, envPath, key, resolved)) @@ -271,6 +283,7 @@ export const authGithubLogin = ( const key = buildGithubTokenKey(command.label) const label = labelFromKey(key) if (token.length > 0) { + yield* _(rejectGithubTokenWithRepositoryDeleteScope(token)) yield* _(ensureEnvFile(fs, path, envPath)) yield* _(persistGithubToken(fs, envPath, key, token)) yield* _(ensureStateDotDockerGitRepo(token)) diff --git a/packages/app/src/lib/usecases/github-scope-policy.ts b/packages/app/src/lib/usecases/github-scope-policy.ts new file mode 100644 index 00000000..70e860e3 --- /dev/null +++ b/packages/app/src/lib/usecases/github-scope-policy.ts @@ -0,0 +1,58 @@ +/* jscpd:ignore-start */ +export const defaultGithubScopes: ReadonlyArray = ["repo", "workflow", "read:org"] +export const githubRepositoryDeleteScope = "delete_repo" +export const githubForbiddenDeleteRepoScopeMessage = [ + "GitHub auth token includes forbidden OAuth scope: delete_repo.", + "Repository deletion is not allowed for docker-git tokens. The token was not stored." +].join("\n") + +const scopeSeparator = /[,\s]+/g + +const normalizeScopeForComparison = (scope: string): string => scope.trim().toLowerCase() + +const isGithubRepositoryDeleteScope = (scope: string): boolean => + normalizeScopeForComparison(scope) === githubRepositoryDeleteScope + +// CHANGE: centralize GitHub OAuth scope normalization +// WHY: every auth surface must request useful scopes while excluding repository deletion +// QUOTE(user): "Generated GitHub tokens must not be able to delete repositories." +// REF: issue-288 +// SOURCE: n/a +// FORMAT THEOREM: forall input: delete_repo notin normalizeGithubScopes(input) +// PURITY: CORE +// EFFECT: n/a +// INVARIANT: empty or all-forbidden input falls back to default safe scopes +// COMPLEXITY: O(n) where n = |input scopes| +export const normalizeGithubScopes = (value: string | null | undefined): ReadonlyArray => { + const raw = value?.trim() ?? "" + const input = raw.length === 0 ? defaultGithubScopes.join(",") : raw + const scopes = input + .split(scopeSeparator) + .map((scope) => scope.trim()) + .filter((scope) => scope.length > 0 && !isGithubRepositoryDeleteScope(scope)) + return scopes.length === 0 ? defaultGithubScopes : scopes +} + +// CHANGE: parse GitHub's X-OAuth-Scopes response header +// WHY: persisted tokens must be checked against the effective scopes granted by GitHub +// QUOTE(user): "Generated GitHub tokens must not be able to delete repositories." +// REF: issue-288 +// SOURCE: n/a +// FORMAT THEOREM: forall header: parse(header) = granted OAuth scopes or empty +// PURITY: CORE +// EFFECT: n/a +// INVARIANT: absent header remains unknown; empty header is a known empty scope set +// COMPLEXITY: O(n) where n = |header| +export const parseGithubOauthScopesHeader = (value: string | null | undefined): ReadonlyArray | null => { + if (value === null || value === undefined) { + return null + } + return value + .split(scopeSeparator) + .map((scope) => scope.trim()) + .filter((scope) => scope.length > 0) +} + +export const hasGithubRepositoryDeleteScope = (scopes: ReadonlyArray | null): boolean => + scopes?.some(isGithubRepositoryDeleteScope) ?? false +/* jscpd:ignore-end */ diff --git a/packages/app/src/lib/usecases/github-token-validation.ts b/packages/app/src/lib/usecases/github-token-validation.ts index fd71cc6b..4b330841 100644 --- a/packages/app/src/lib/usecases/github-token-validation.ts +++ b/packages/app/src/lib/usecases/github-token-validation.ts @@ -4,6 +4,8 @@ import * as ParseResult from "@effect/schema/ParseResult" import * as Schema from "@effect/schema/Schema" import { Effect, Either } from "effect" +import { parseGithubOauthScopesHeader } from "./github-scope-policy.js" + const githubTokenValidationUrl = "https://api.github.com/user" export const githubTokenValidationWarning = "Unable to validate GitHub token before start; continuing." @@ -21,6 +23,7 @@ export type GithubTokenValidationStatus = "valid" | "invalid" | "unknown" export type GithubTokenValidationResult = { readonly status: GithubTokenValidationStatus readonly login: string | null + readonly oauthScopes: ReadonlyArray | null } const GithubUserSchema: Schema.Schema = Schema.Struct({ @@ -30,7 +33,8 @@ const GithubUserJsonSchema = Schema.parseJson(GithubUserSchema) const unknownGithubTokenValidationResult = (): GithubTokenValidationResult => ({ status: "unknown", - login: null + login: null, + oauthScopes: null }) const decodeGithubUserLogin = (input: string): string | null => @@ -46,8 +50,8 @@ const mapGithubTokenValidationStatus = (status: number): GithubTokenValidationSt return status >= 200 && status < 300 ? "valid" : "unknown" } -// CHANGE: validate GitHub token and decode the authenticated account login on success -// WHY: auth status and create preflight must share one live GitHub validation boundary +// CHANGE: validate GitHub token and decode the authenticated account login/scopes on success +// WHY: auth status, create preflight, and auth persistence must share one live GitHub validation boundary // QUOTE(ТЗ): "status проверял валидность токена и если он валидный то писал бы кто овнер" // REF: user-request-2026-03-19-github-token-status-owner // SOURCE: n/a @@ -69,17 +73,20 @@ export const validateGithubToken = (token: string): Effect.Effect => })) .filter((entry) => entry.token.trim().length > 0) -const defaultGithubScopes = "repo,workflow,read:org" - -// CHANGE: normalize GitHub scopes for gh auth login -// WHY: ensure required scopes are requested without delete_repo -// QUOTE(ТЗ): "Передай все нужные скопы" -// REF: user-request-2026-02-05-gh-scopes -// SOURCE: n/a -// FORMAT THEOREM: ∀s: normalize(s) -> scopes(s) ⊆ required -// PURITY: CORE -// EFFECT: n/a -// INVARIANT: empty input yields default scopes -// COMPLEXITY: O(n) where n = |scopes| -const normalizeGithubScopes = (value: string | null | undefined): ReadonlyArray => { - const raw = value?.trim() ?? "" - const input = raw.length === 0 ? defaultGithubScopes : raw - const scopes = input - .split(/[,\s]+/g) - .map((scope) => scope.trim()) - .filter((scope) => scope.length > 0 && scope !== "delete_repo") - return scopes.length === 0 ? defaultGithubScopes.split(",") : scopes -} - const withEnvContext = ( envGlobalPath: string, run: (context: EnvContext) => Effect.Effect @@ -143,7 +125,8 @@ const validateGithubTokenEntry = ( label: entry.label, token: entry.token, status: validation.status, - login: validation.login + login: validation.login, + oauthScopes: validation.oauthScopes })) ) @@ -199,6 +182,32 @@ const runGithubLogin = ( (exitCode) => new CommandFailedError({ command: "gh auth login --web", exitCode }) ) +const runGithubRemoveDeleteRepoScope = ( + cwd: string, + accountPath: string +): Effect.Effect => + runDockerAuth( + buildDockerAuthSpec({ + cwd, + image: ghImageName, + hostPath: accountPath, + containerPath: ghAuthDir, + env: ["BROWSER=echo", `GH_CONFIG_DIR=${ghAuthDir}`], + args: ["auth", "refresh", "-h", "github.com", "--remove-scopes", "delete_repo"], + interactive: false + }), + [0], + (exitCode) => new CommandFailedError({ command: "gh auth refresh --remove-scopes delete_repo", exitCode }) + ) + +const rejectGithubTokenWithRepositoryDeleteScope = (token: string): Effect.Effect => + validateGithubToken(token).pipe( + Effect.flatMap((validation) => + hasGithubRepositoryDeleteScope(validation.oauthScopes) + ? Effect.fail(new AuthError({ message: githubForbiddenDeleteRepoScopeMessage })) + : Effect.void) + ) + const retryGithubLogin = ( effect: Effect.Effect ): Effect.Effect => @@ -242,7 +251,10 @@ const runGithubInteractiveLogin = ( yield* _(ensureGhAuthImage(fs, path, cwd, "gh auth")) yield* _(Effect.log(`Starting GH auth login in container (scopes: ${scopes.join(", ")})...`)) yield* _(retryGithubLogin(runGithubLogin(cwd, accountPath, scopes))) + yield* _(Effect.log("Removing repository delete scope from GH auth token...")) + yield* _(runGithubRemoveDeleteRepoScope(cwd, accountPath)) const resolved = yield* _(resolveGithubTokenFromGh(cwd, accountPath)) + yield* _(rejectGithubTokenWithRepositoryDeleteScope(resolved)) yield* _(ensureEnvFile(fs, path, envPath)) const key = buildGithubTokenKey(command.label) yield* _(persistGithubToken(fs, envPath, key, resolved)) @@ -270,6 +282,7 @@ export const authGithubLogin = ( const key = buildGithubTokenKey(command.label) const label = labelFromKey(key) if (token.length > 0) { + yield* _(rejectGithubTokenWithRepositoryDeleteScope(token)) yield* _(ensureEnvFile(fs, path, envPath)) yield* _(persistGithubToken(fs, envPath, key, token)) yield* _(ensureStateDotDockerGitRepo(token)) diff --git a/packages/lib/src/usecases/github-scope-policy.ts b/packages/lib/src/usecases/github-scope-policy.ts new file mode 100644 index 00000000..9ca78ba9 --- /dev/null +++ b/packages/lib/src/usecases/github-scope-policy.ts @@ -0,0 +1,56 @@ +export const defaultGithubScopes: ReadonlyArray = ["repo", "workflow", "read:org"] +export const githubRepositoryDeleteScope = "delete_repo" +export const githubForbiddenDeleteRepoScopeMessage = [ + "GitHub auth token includes forbidden OAuth scope: delete_repo.", + "Repository deletion is not allowed for docker-git tokens. The token was not stored." +].join("\n") + +const scopeSeparator = /[,\s]+/g + +const normalizeScopeForComparison = (scope: string): string => scope.trim().toLowerCase() + +const isGithubRepositoryDeleteScope = (scope: string): boolean => + normalizeScopeForComparison(scope) === githubRepositoryDeleteScope + +// CHANGE: centralize GitHub OAuth scope normalization +// WHY: every auth surface must request useful scopes while excluding repository deletion +// QUOTE(user): "Generated GitHub tokens must not be able to delete repositories." +// REF: issue-288 +// SOURCE: n/a +// FORMAT THEOREM: forall input: delete_repo notin normalizeGithubScopes(input) +// PURITY: CORE +// EFFECT: n/a +// INVARIANT: empty or all-forbidden input falls back to default safe scopes +// COMPLEXITY: O(n) where n = |input scopes| +export const normalizeGithubScopes = (value: string | null | undefined): ReadonlyArray => { + const raw = value?.trim() ?? "" + const input = raw.length === 0 ? defaultGithubScopes.join(",") : raw + const scopes = input + .split(scopeSeparator) + .map((scope) => scope.trim()) + .filter((scope) => scope.length > 0 && !isGithubRepositoryDeleteScope(scope)) + return scopes.length === 0 ? defaultGithubScopes : scopes +} + +// CHANGE: parse GitHub's X-OAuth-Scopes response header +// WHY: persisted tokens must be checked against the effective scopes granted by GitHub +// QUOTE(user): "Generated GitHub tokens must not be able to delete repositories." +// REF: issue-288 +// SOURCE: n/a +// FORMAT THEOREM: forall header: parse(header) = granted OAuth scopes or empty +// PURITY: CORE +// EFFECT: n/a +// INVARIANT: absent header remains unknown; empty header is a known empty scope set +// COMPLEXITY: O(n) where n = |header| +export const parseGithubOauthScopesHeader = (value: string | null | undefined): ReadonlyArray | null => { + if (value === null || value === undefined) { + return null + } + return value + .split(scopeSeparator) + .map((scope) => scope.trim()) + .filter((scope) => scope.length > 0) +} + +export const hasGithubRepositoryDeleteScope = (scopes: ReadonlyArray | null): boolean => + scopes?.some(isGithubRepositoryDeleteScope) ?? false diff --git a/packages/lib/src/usecases/github-token-validation.ts b/packages/lib/src/usecases/github-token-validation.ts index b24a47cf..401ee0dc 100644 --- a/packages/lib/src/usecases/github-token-validation.ts +++ b/packages/lib/src/usecases/github-token-validation.ts @@ -3,6 +3,8 @@ import * as ParseResult from "@effect/schema/ParseResult" import * as Schema from "@effect/schema/Schema" import { Effect, Either } from "effect" +import { parseGithubOauthScopesHeader } from "./github-scope-policy.js" + const githubTokenValidationUrl = "https://api.github.com/user" export const githubTokenValidationWarning = "Unable to validate GitHub token before start; continuing." @@ -20,6 +22,7 @@ export type GithubTokenValidationStatus = "valid" | "invalid" | "unknown" export type GithubTokenValidationResult = { readonly status: GithubTokenValidationStatus readonly login: string | null + readonly oauthScopes: ReadonlyArray | null } const GithubUserSchema: Schema.Schema = Schema.Struct({ @@ -29,7 +32,8 @@ const GithubUserJsonSchema = Schema.parseJson(GithubUserSchema) const unknownGithubTokenValidationResult = (): GithubTokenValidationResult => ({ status: "unknown", - login: null + login: null, + oauthScopes: null }) const decodeGithubUserLogin = (input: string): string | null => @@ -45,8 +49,8 @@ const mapGithubTokenValidationStatus = (status: number): GithubTokenValidationSt return status >= 200 && status < 300 ? "valid" : "unknown" } -// CHANGE: validate GitHub token and decode the authenticated account login on success -// WHY: auth status and create preflight must share one live GitHub validation boundary +// CHANGE: validate GitHub token and decode the authenticated account login/scopes on success +// WHY: auth status, create preflight, and auth persistence must share one live GitHub validation boundary // QUOTE(ТЗ): "status проверял валидность токена и если он валидный то писал бы кто овнер" // REF: user-request-2026-03-19-github-token-status-owner // SOURCE: n/a @@ -68,17 +72,20 @@ export const validateGithubToken = (token: string): Effect.Effect( }) ) +const withPatchedFetch = ( + fetchImpl: typeof globalThis.fetch, + effect: Effect.Effect +): Effect.Effect => + Effect.acquireUseRelease( + Effect.sync(() => { + const previous = globalThis.fetch + globalThis.fetch = fetchImpl + return previous + }), + () => effect, + (previous) => + Effect.sync(() => { + globalThis.fetch = previous + }) + ) + const withWorkingDirectory = ( cwd: string, effect: Effect.Effect @@ -145,6 +164,15 @@ const makeFakeExecutor = ( return CommandExecutor.makeExecutor(start) } +const githubUserResponse = (scopes: string): Response => + new Response(JSON.stringify({ login: "octocat" }), { + status: 200, + headers: { + "content-type": "application/json", + "x-oauth-scopes": scopes + } + }) + describe("auth container paths", () => { it.effect("pins gh auth login and token reads to the same writable config dir", () => withTempDir((root) => @@ -154,35 +182,50 @@ describe("auth container paths", () => { const accountPath = `${root}/.docker-git/.orch/auth/gh/default` const recorded: Array = [] const executor = makeFakeExecutor(recorded) + const fetchMock = vi.fn(() => + Effect.runPromise(Effect.succeed(githubUserResponse("repo, workflow, read:org"))) + ) yield* _( - withPatchedEnv( - { - HOME: root, - DOCKER_GIT_STATE_AUTO_SYNC: "0" - }, - withWorkingDirectory( - root, - authGithubLogin({ - _tag: "AuthGithubLogin", - label: null, - token: null, - scopes: null, - envGlobalPath: ".docker-git/.orch/env/global.env" - }).pipe(Effect.provideService(CommandExecutor.CommandExecutor, executor)) + withPatchedFetch( + fetchMock, + withPatchedEnv( + { + HOME: root, + DOCKER_GIT_STATE_AUTO_SYNC: "0" + }, + withWorkingDirectory( + root, + authGithubLogin({ + _tag: "AuthGithubLogin", + label: null, + token: null, + scopes: null, + envGlobalPath: ".docker-git/.orch/env/global.env" + }).pipe(Effect.provideService(CommandExecutor.CommandExecutor, executor)) + ) ) ) ) - const loginCommand = recorded.find((entry) => + const loginIndex = recorded.findIndex((entry) => isDockerRunFor(entry, "docker-git-auth-gh:latest", ["auth", "login"]) ) - const tokenCommand = recorded.find((entry) => + const refreshIndex = recorded.findIndex((entry) => + isDockerRunFor(entry, "docker-git-auth-gh:latest", ["auth", "refresh"]) + ) + const tokenIndex = recorded.findIndex((entry) => isDockerRunFor(entry, "docker-git-auth-gh:latest", ["auth", "token"]) ) + const loginCommand = recorded[loginIndex] + const refreshCommand = recorded[refreshIndex] + const tokenCommand = recorded[tokenIndex] expect(loginCommand).toBeDefined() + expect(refreshCommand).toBeDefined() expect(tokenCommand).toBeDefined() + expect(refreshIndex).toBeGreaterThan(loginIndex) + expect(tokenIndex).toBeGreaterThan(refreshIndex) expect( includesArgsInOrder(loginCommand?.args ?? [], [ "-v", @@ -193,7 +236,24 @@ describe("auth container paths", () => { "GH_CONFIG_DIR=/gh-auth", "docker-git-auth-gh:latest", "auth", - "login" + "login", + "--scopes", + "repo,workflow,read:org" + ]) + ).toBe(true) + expect( + includesArgsInOrder(refreshCommand?.args ?? [], [ + "-v", + `${accountPath}:/gh-auth`, + "-e", + "BROWSER=echo", + "-e", + "GH_CONFIG_DIR=/gh-auth", + "docker-git-auth-gh:latest", + "auth", + "refresh", + "--remove-scopes", + "delete_repo" ]) ).toBe(true) expect( @@ -210,6 +270,126 @@ describe("auth container paths", () => { const envText = yield* _(fs.readFileString(envPath)) expect(envText).toContain("GITHUB_TOKEN=test-gh-token") + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + ).pipe(Effect.provide(NodeContext.layer))) + + it.effect("filters delete_repo from requested scopes before GitHub web login", () => + withTempDir((root) => + Effect.gen(function*(_) { + const recorded: Array = [] + const executor = makeFakeExecutor(recorded) + const fetchMock = vi.fn(() => + Effect.runPromise(Effect.succeed(githubUserResponse("repo, workflow"))) + ) + + yield* _( + withPatchedFetch( + fetchMock, + withPatchedEnv( + { + HOME: root, + DOCKER_GIT_STATE_AUTO_SYNC: "0" + }, + withWorkingDirectory( + root, + authGithubLogin({ + _tag: "AuthGithubLogin", + label: null, + token: null, + scopes: "repo,DELETE_REPO workflow", + envGlobalPath: ".docker-git/.orch/env/global.env" + }).pipe(Effect.provideService(CommandExecutor.CommandExecutor, executor)) + ) + ) + ) + ) + + const loginCommand = recorded.find((entry) => + isDockerRunFor(entry, "docker-git-auth-gh:latest", ["auth", "login"]) + ) + + expect(loginCommand).toBeDefined() + expect(includesArgsInOrder(loginCommand?.args ?? [], ["--scopes", "repo,workflow"])).toBe(true) + }) + ).pipe(Effect.provide(NodeContext.layer))) + + it.effect("does not persist a generated token when GitHub reports delete_repo", () => + withTempDir((root) => + Effect.gen(function*(_) { + const fs = yield* _(FileSystem.FileSystem) + const envPath = `${root}/.docker-git/.orch/env/global.env` + const recorded: Array = [] + const executor = makeFakeExecutor(recorded) + const fetchMock = vi.fn(() => + Effect.runPromise(Effect.succeed(githubUserResponse("repo, delete_repo"))) + ) + + const failure = yield* _( + withPatchedFetch( + fetchMock, + withPatchedEnv( + { + HOME: root, + DOCKER_GIT_STATE_AUTO_SYNC: "0" + }, + withWorkingDirectory( + root, + authGithubLogin({ + _tag: "AuthGithubLogin", + label: null, + token: null, + scopes: null, + envGlobalPath: ".docker-git/.orch/env/global.env" + }).pipe( + Effect.provideService(CommandExecutor.CommandExecutor, executor), + Effect.flip + ) + ) + ) + ) + ) + + expect(failure._tag).toBe("AuthError") + expect(failure.message).toBe(githubForbiddenDeleteRepoScopeMessage) + expect(yield* _(fs.exists(envPath))).toBe(false) + }) + ).pipe(Effect.provide(NodeContext.layer))) + + it.effect("rejects a manual token when GitHub reports delete_repo", () => + withTempDir((root) => + Effect.gen(function*(_) { + const fs = yield* _(FileSystem.FileSystem) + const envPath = `${root}/.docker-git/.orch/env/global.env` + const fetchMock = vi.fn(() => + Effect.runPromise(Effect.succeed(githubUserResponse("repo, delete_repo"))) + ) + + const failure = yield* _( + withPatchedFetch( + fetchMock, + withPatchedEnv( + { + HOME: root, + DOCKER_GIT_STATE_AUTO_SYNC: "0" + }, + withWorkingDirectory( + root, + authGithubLogin({ + _tag: "AuthGithubLogin", + label: null, + token: "manual-token", + scopes: null, + envGlobalPath: ".docker-git/.orch/env/global.env" + }).pipe(Effect.flip) + ) + ) + ) + ) + + expect(failure._tag).toBe("AuthError") + expect(failure.message).toBe(githubForbiddenDeleteRepoScopeMessage) + expect(yield* _(fs.exists(envPath))).toBe(false) }) ).pipe(Effect.provide(NodeContext.layer))) diff --git a/packages/lib/tests/usecases/github-scope-policy.test.ts b/packages/lib/tests/usecases/github-scope-policy.test.ts new file mode 100644 index 00000000..5d78a772 --- /dev/null +++ b/packages/lib/tests/usecases/github-scope-policy.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest" + +import { + defaultGithubScopes, + hasGithubRepositoryDeleteScope, + normalizeGithubScopes, + parseGithubOauthScopesHeader +} from "../../src/usecases/github-scope-policy.js" + +describe("github scope policy", () => { + it("preserves default safe scopes", () => { + expect(normalizeGithubScopes(null)).toEqual(defaultGithubScopes) + expect(normalizeGithubScopes("")).toEqual(defaultGithubScopes) + }) + + it("accepts comma and space separated scopes", () => { + expect(normalizeGithubScopes("repo,workflow read:org")).toEqual(["repo", "workflow", "read:org"]) + }) + + it("removes delete_repo case-insensitively", () => { + expect(normalizeGithubScopes("repo,DELETE_REPO workflow delete_repo")).toEqual(["repo", "workflow"]) + }) + + it("falls back to defaults when every requested scope is forbidden", () => { + expect(normalizeGithubScopes("delete_repo DELETE_REPO")).toEqual(defaultGithubScopes) + }) + + it("parses GitHub OAuth scope headers and detects repository deletion", () => { + expect(parseGithubOauthScopesHeader("repo, workflow, delete_repo")).toEqual(["repo", "workflow", "delete_repo"]) + expect(hasGithubRepositoryDeleteScope(["repo", "DELETE_REPO"])).toBe(true) + expect(hasGithubRepositoryDeleteScope(["repo", "workflow"])).toBe(false) + expect(hasGithubRepositoryDeleteScope(null)).toBe(false) + }) +}) From fa0862749caa5d6237af64b0e332a7976f53b8c9 Mon Sep 17 00:00:00 2001 From: rikohomeless <163776849+rikohomeless@users.noreply.github.com> Date: Thu, 14 May 2026 17:24:30 +0000 Subject: [PATCH 2/3] fix(auth): reject unverified github token scopes --- .../src/services/auth-github-login-stream.ts | 69 ++++------- packages/app/src/lib/usecases/auth-github.ts | 11 +- .../src/lib/usecases/github-scope-policy.ts | 8 +- .../lib/usecases/github-token-validation.ts | 5 +- packages/lib/src/usecases/auth-github.ts | 15 ++- .../lib/src/usecases/github-scope-policy.ts | 6 +- .../src/usecases/github-token-validation.ts | 5 +- .../usecases/auth-container-paths.test.ts | 108 ++++++++++++++++-- .../usecases/github-scope-policy.test.ts | 2 + 9 files changed, 157 insertions(+), 72 deletions(-) diff --git a/packages/api/src/services/auth-github-login-stream.ts b/packages/api/src/services/auth-github-login-stream.ts index 004530e8..eb06ee91 100644 --- a/packages/api/src/services/auth-github-login-stream.ts +++ b/packages/api/src/services/auth-github-login-stream.ts @@ -4,21 +4,21 @@ import * as FileSystem from "@effect/platform/FileSystem" import * as Path from "@effect/platform/Path" import { defaultTemplateConfig } from "@effect-template/lib/core/template-defaults" import { buildDockerAuthArgs, resolveDockerVolumeHostPath, runDockerAuthCapture } from "@effect-template/lib/shell/docker-auth" -import { AuthError, CommandFailedError } from "@effect-template/lib/shell/errors" +import type { AuthError } from "@effect-template/lib/shell/errors" +import { CommandFailedError } from "@effect-template/lib/shell/errors" +import { + rejectGithubTokenWithRepositoryDeleteScope, + runGithubRemoveDeleteRepoScope +} from "@effect-template/lib/usecases/auth-github" import { buildDockerAuthSpec, normalizeAccountLabel } from "@effect-template/lib/usecases/auth-helpers" import { ensureEnvFile, readEnvText, upsertEnvKey } from "@effect-template/lib/usecases/env-file" import { ensureGhAuthImage, ghAuthDir, ghAuthRoot, ghImageName } from "@effect-template/lib/usecases/github-auth-image" -import { - githubForbiddenDeleteRepoScopeMessage, - hasGithubRepositoryDeleteScope, - normalizeGithubScopes -} from "@effect-template/lib/usecases/github-scope-policy" -import { validateGithubToken } from "@effect-template/lib/usecases/github-token-validation" +import { normalizeGithubScopes } from "@effect-template/lib/usecases/github-scope-policy" import { resolvePathFromCwd } from "@effect-template/lib/usecases/path-helpers" import { autoSyncState } from "@effect-template/lib/usecases/state-repo" import { ensureStateDotDockerGitRepo } from "@effect-template/lib/usecases/state-repo-github" import { migrateLegacyOrchLayout } from "@effect-template/lib/usecases/auth-sync" -import { Effect, Logger, Runtime } from "effect" +import { Effect, Logger, Match, Runtime } from "effect" import * as Stream from "effect/Stream" import { spawn, type ChildProcess } from "node:child_process" @@ -77,18 +77,21 @@ const buildGithubTokenKey = (label: string | null): string => { const labelFromKey = (key: string): string => key.startsWith(githubTokenPrefix) ? key.slice(githubTokenPrefix.length) : "default" const toApiError = (error: GithubSetupError): ApiBadRequestError | ApiInternalError => - error._tag === "AuthError" - ? new ApiBadRequestError({ - message: error.message - }) - : error._tag === "CommandFailedError" - ? new ApiBadRequestError({ - message: `${error.command} failed (exit ${error.exitCode}).` - }) - : new ApiInternalError({ - message: String(error), - cause: error - }) + Match.value(error).pipe( + Match.tag("AuthError", (authError) => + new ApiBadRequestError({ + message: authError.message + })), + Match.tag("CommandFailedError", (commandError) => + new ApiBadRequestError({ + message: `${commandError.command} failed (exit ${commandError.exitCode}).` + })), + Match.orElse((platformError) => + new ApiInternalError({ + message: String(platformError), + cause: platformError + })) + ) const prepareGithubLogin = ( request: GithubAuthLoginRequest @@ -170,32 +173,6 @@ const resolveGithubToken = ( ) ) -const runGithubRemoveDeleteRepoScope = ( - cwd: string, - accountPath: string -): Effect.Effect => - runDockerAuthCapture( - buildDockerAuthSpec({ - cwd, - image: ghImageName, - hostPath: accountPath, - containerPath: ghAuthDir, - env: ["BROWSER=echo", `GH_CONFIG_DIR=${ghAuthDir}`], - args: ["auth", "refresh", "-h", "github.com", "--remove-scopes", "delete_repo"], - interactive: false - }), - [0], - (exitCode) => new CommandFailedError({ command: "gh auth refresh --remove-scopes delete_repo", exitCode }) - ).pipe(Effect.asVoid) - -const rejectGithubTokenWithRepositoryDeleteScope = (token: string): Effect.Effect => - validateGithubToken(token).pipe( - Effect.flatMap((validation) => - hasGithubRepositoryDeleteScope(validation.oauthScopes) - ? Effect.fail(new AuthError({ message: githubForbiddenDeleteRepoScopeMessage })) - : Effect.void) - ) - const persistGithubToken = ( fs: FileSystem.FileSystem, envPath: string, diff --git a/packages/app/src/lib/usecases/auth-github.ts b/packages/app/src/lib/usecases/auth-github.ts index 3d11c43d..5d674f5d 100644 --- a/packages/app/src/lib/usecases/auth-github.ts +++ b/packages/app/src/lib/usecases/auth-github.ts @@ -17,6 +17,7 @@ import { ensureEnvFile, parseEnvEntries, readEnvText, removeEnvKey, upsertEnvKey import { ensureGhAuthImage, ghAuthDir, ghAuthRoot, ghImageName } from "./github-auth-image.js" import { githubForbiddenDeleteRepoScopeMessage, + githubUnverifiedTokenScopesMessage, hasGithubRepositoryDeleteScope, normalizeGithubScopes } from "./github-scope-policy.js" @@ -203,10 +204,14 @@ const runGithubRemoveDeleteRepoScope = ( const rejectGithubTokenWithRepositoryDeleteScope = (token: string): Effect.Effect => validateGithubToken(token).pipe( - Effect.flatMap((validation) => - hasGithubRepositoryDeleteScope(validation.oauthScopes) + Effect.flatMap((validation) => { + if (validation.status !== "valid" || validation.oauthScopes === null) { + return Effect.fail(new AuthError({ message: githubUnverifiedTokenScopesMessage })) + } + return hasGithubRepositoryDeleteScope(validation.oauthScopes) ? Effect.fail(new AuthError({ message: githubForbiddenDeleteRepoScopeMessage })) - : Effect.void) + : Effect.void + }) ) const retryGithubLogin = ( diff --git a/packages/app/src/lib/usecases/github-scope-policy.ts b/packages/app/src/lib/usecases/github-scope-policy.ts index 70e860e3..cbb5cfaa 100644 --- a/packages/app/src/lib/usecases/github-scope-policy.ts +++ b/packages/app/src/lib/usecases/github-scope-policy.ts @@ -5,6 +5,10 @@ export const githubForbiddenDeleteRepoScopeMessage = [ "GitHub auth token includes forbidden OAuth scope: delete_repo.", "Repository deletion is not allowed for docker-git tokens. The token was not stored." ].join("\n") +export const githubUnverifiedTokenScopesMessage = [ + "Unable to verify GitHub token OAuth scopes.", + "The token was not stored because docker-git could not confirm repository deletion is disabled." +].join("\n") const scopeSeparator = /[,\s]+/g @@ -14,7 +18,7 @@ const isGithubRepositoryDeleteScope = (scope: string): boolean => normalizeScopeForComparison(scope) === githubRepositoryDeleteScope // CHANGE: centralize GitHub OAuth scope normalization -// WHY: every auth surface must request useful scopes while excluding repository deletion +// WHY: every app auth surface must request useful scopes while excluding repository deletion // QUOTE(user): "Generated GitHub tokens must not be able to delete repositories." // REF: issue-288 // SOURCE: n/a @@ -54,5 +58,5 @@ export const parseGithubOauthScopesHeader = (value: string | null | undefined): } export const hasGithubRepositoryDeleteScope = (scopes: ReadonlyArray | null): boolean => - scopes?.some(isGithubRepositoryDeleteScope) ?? false + scopes?.some((scope) => isGithubRepositoryDeleteScope(scope)) ?? false /* jscpd:ignore-end */ diff --git a/packages/app/src/lib/usecases/github-token-validation.ts b/packages/app/src/lib/usecases/github-token-validation.ts index 4b330841..f544242e 100644 --- a/packages/app/src/lib/usecases/github-token-validation.ts +++ b/packages/app/src/lib/usecases/github-token-validation.ts @@ -1,8 +1,9 @@ /* jscpd:ignore-start */ import { FetchHttpClient, HttpClient } from "@effect/platform" +import * as Headers from "@effect/platform/Headers" import * as ParseResult from "@effect/schema/ParseResult" import * as Schema from "@effect/schema/Schema" -import { Effect, Either } from "effect" +import { Effect, Either, Option } from "effect" import { parseGithubOauthScopesHeader } from "./github-scope-policy.js" @@ -73,7 +74,7 @@ export const validateGithubToken = (token: string): Effect.Effect new CommandFailedError({ command: "gh auth login --web", exitCode }) ) -const runGithubRemoveDeleteRepoScope = ( +export const runGithubRemoveDeleteRepoScope = ( cwd: string, accountPath: string ): Effect.Effect => @@ -200,12 +201,16 @@ const runGithubRemoveDeleteRepoScope = ( (exitCode) => new CommandFailedError({ command: "gh auth refresh --remove-scopes delete_repo", exitCode }) ) -const rejectGithubTokenWithRepositoryDeleteScope = (token: string): Effect.Effect => +export const rejectGithubTokenWithRepositoryDeleteScope = (token: string): Effect.Effect => validateGithubToken(token).pipe( - Effect.flatMap((validation) => - hasGithubRepositoryDeleteScope(validation.oauthScopes) + Effect.flatMap((validation) => { + if (validation.status !== "valid" || validation.oauthScopes === null) { + return Effect.fail(new AuthError({ message: githubUnverifiedTokenScopesMessage })) + } + return hasGithubRepositoryDeleteScope(validation.oauthScopes) ? Effect.fail(new AuthError({ message: githubForbiddenDeleteRepoScopeMessage })) - : Effect.void) + : Effect.void + }) ) const retryGithubLogin = ( diff --git a/packages/lib/src/usecases/github-scope-policy.ts b/packages/lib/src/usecases/github-scope-policy.ts index 9ca78ba9..d9250d7a 100644 --- a/packages/lib/src/usecases/github-scope-policy.ts +++ b/packages/lib/src/usecases/github-scope-policy.ts @@ -4,6 +4,10 @@ export const githubForbiddenDeleteRepoScopeMessage = [ "GitHub auth token includes forbidden OAuth scope: delete_repo.", "Repository deletion is not allowed for docker-git tokens. The token was not stored." ].join("\n") +export const githubUnverifiedTokenScopesMessage = [ + "Unable to verify GitHub token OAuth scopes.", + "The token was not stored because docker-git could not confirm repository deletion is disabled." +].join("\n") const scopeSeparator = /[,\s]+/g @@ -53,4 +57,4 @@ export const parseGithubOauthScopesHeader = (value: string | null | undefined): } export const hasGithubRepositoryDeleteScope = (scopes: ReadonlyArray | null): boolean => - scopes?.some(isGithubRepositoryDeleteScope) ?? false + scopes?.some((scope) => isGithubRepositoryDeleteScope(scope)) ?? false diff --git a/packages/lib/src/usecases/github-token-validation.ts b/packages/lib/src/usecases/github-token-validation.ts index 401ee0dc..f65d9419 100644 --- a/packages/lib/src/usecases/github-token-validation.ts +++ b/packages/lib/src/usecases/github-token-validation.ts @@ -1,7 +1,8 @@ import { FetchHttpClient, HttpClient } from "@effect/platform" +import * as Headers from "@effect/platform/Headers" import * as ParseResult from "@effect/schema/ParseResult" import * as Schema from "@effect/schema/Schema" -import { Effect, Either } from "effect" +import { Effect, Either, Option } from "effect" import { parseGithubOauthScopesHeader } from "./github-scope-policy.js" @@ -72,7 +73,7 @@ export const validateGithubToken = (token: string): Effect.Effect - new Response(JSON.stringify({ login: "octocat" }), { +const githubUserResponse = (scopes: string | null): Response => { + const headers: Record = { + "content-type": "application/json" + } + if (scopes !== null) { + headers["x-oauth-scopes"] = scopes + } + return new Response(JSON.stringify({ login: "octocat" }), { status: 200, - headers: { - "content-type": "application/json", - "x-oauth-scopes": scopes - } + headers }) +} describe("auth container paths", () => { it.effect("pins gh auth login and token reads to the same writable config dir", () => @@ -183,7 +190,7 @@ describe("auth container paths", () => { const recorded: Array = [] const executor = makeFakeExecutor(recorded) const fetchMock = vi.fn(() => - Effect.runPromise(Effect.succeed(githubUserResponse("repo, workflow, read:org"))) + Promise.resolve(githubUserResponse("repo, workflow, read:org")) ) yield* _( @@ -280,7 +287,7 @@ describe("auth container paths", () => { const recorded: Array = [] const executor = makeFakeExecutor(recorded) const fetchMock = vi.fn(() => - Effect.runPromise(Effect.succeed(githubUserResponse("repo, workflow"))) + Promise.resolve(githubUserResponse("repo, workflow")) ) yield* _( @@ -322,7 +329,7 @@ describe("auth container paths", () => { const recorded: Array = [] const executor = makeFakeExecutor(recorded) const fetchMock = vi.fn(() => - Effect.runPromise(Effect.succeed(githubUserResponse("repo, delete_repo"))) + Promise.resolve(githubUserResponse("repo, delete_repo")) ) const failure = yield* _( @@ -356,13 +363,55 @@ describe("auth container paths", () => { }) ).pipe(Effect.provide(NodeContext.layer))) + it.effect("does not persist a generated token when GitHub omits OAuth scopes", () => + withTempDir((root) => + Effect.gen(function*(_) { + const fs = yield* _(FileSystem.FileSystem) + const envPath = `${root}/.docker-git/.orch/env/global.env` + const recorded: Array = [] + const executor = makeFakeExecutor(recorded) + const fetchMock = vi.fn(() => + Promise.resolve(githubUserResponse(null)) + ) + + const failure = yield* _( + withPatchedFetch( + fetchMock, + withPatchedEnv( + { + HOME: root, + DOCKER_GIT_STATE_AUTO_SYNC: "0" + }, + withWorkingDirectory( + root, + authGithubLogin({ + _tag: "AuthGithubLogin", + label: null, + token: null, + scopes: null, + envGlobalPath: ".docker-git/.orch/env/global.env" + }).pipe( + Effect.provideService(CommandExecutor.CommandExecutor, executor), + Effect.flip + ) + ) + ) + ) + ) + + expect(failure._tag).toBe("AuthError") + expect(failure.message).toBe(githubUnverifiedTokenScopesMessage) + expect(yield* _(fs.exists(envPath))).toBe(false) + }) + ).pipe(Effect.provide(NodeContext.layer))) + it.effect("rejects a manual token when GitHub reports delete_repo", () => withTempDir((root) => Effect.gen(function*(_) { const fs = yield* _(FileSystem.FileSystem) const envPath = `${root}/.docker-git/.orch/env/global.env` const fetchMock = vi.fn(() => - Effect.runPromise(Effect.succeed(githubUserResponse("repo, delete_repo"))) + Promise.resolve(githubUserResponse("repo, delete_repo")) ) const failure = yield* _( @@ -393,6 +442,43 @@ describe("auth container paths", () => { }) ).pipe(Effect.provide(NodeContext.layer))) + it.effect("rejects a manual token when GitHub omits OAuth scopes", () => + withTempDir((root) => + Effect.gen(function*(_) { + const fs = yield* _(FileSystem.FileSystem) + const envPath = `${root}/.docker-git/.orch/env/global.env` + const fetchMock = vi.fn(() => + Promise.resolve(githubUserResponse(null)) + ) + + const failure = yield* _( + withPatchedFetch( + fetchMock, + withPatchedEnv( + { + HOME: root, + DOCKER_GIT_STATE_AUTO_SYNC: "0" + }, + withWorkingDirectory( + root, + authGithubLogin({ + _tag: "AuthGithubLogin", + label: null, + token: "manual-token", + scopes: null, + envGlobalPath: ".docker-git/.orch/env/global.env" + }).pipe(Effect.flip) + ) + ) + ) + ) + + expect(failure._tag).toBe("AuthError") + expect(failure.message).toBe(githubUnverifiedTokenScopesMessage) + expect(yield* _(fs.exists(envPath))).toBe(false) + }) + ).pipe(Effect.provide(NodeContext.layer))) + it.effect("runs codex auth against a non-root CODEX_HOME mount", () => withTempDir((root) => Effect.gen(function*(_) { diff --git a/packages/lib/tests/usecases/github-scope-policy.test.ts b/packages/lib/tests/usecases/github-scope-policy.test.ts index 5d78a772..d6c92470 100644 --- a/packages/lib/tests/usecases/github-scope-policy.test.ts +++ b/packages/lib/tests/usecases/github-scope-policy.test.ts @@ -26,6 +26,8 @@ describe("github scope policy", () => { }) it("parses GitHub OAuth scope headers and detects repository deletion", () => { + expect(parseGithubOauthScopesHeader(null)).toBe(null) + expect(parseGithubOauthScopesHeader(undefined)).toBe(null) expect(parseGithubOauthScopesHeader("repo, workflow, delete_repo")).toEqual(["repo", "workflow", "delete_repo"]) expect(hasGithubRepositoryDeleteScope(["repo", "DELETE_REPO"])).toBe(true) expect(hasGithubRepositoryDeleteScope(["repo", "workflow"])).toBe(false) From b652da799158d2932b6f1ae70a803517defcc43a Mon Sep 17 00:00:00 2001 From: rikohomeless <163776849+rikohomeless@users.noreply.github.com> Date: Thu, 14 May 2026 18:38:21 +0000 Subject: [PATCH 3/3] fix: address github scope review feedback --- packages/lib/src/usecases/auth-github.ts | 7 +++-- .../lib/src/usecases/github-scope-policy.ts | 2 +- .../usecases/auth-container-paths.test.ts | 12 +++---- .../usecases/github-scope-policy.test.ts | 31 +++++++++++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/packages/lib/src/usecases/auth-github.ts b/packages/lib/src/usecases/auth-github.ts index 22672f53..33482f77 100644 --- a/packages/lib/src/usecases/auth-github.ts +++ b/packages/lib/src/usecases/auth-github.ts @@ -21,7 +21,7 @@ import { normalizeGithubScopes } from "./github-scope-policy.js" import type { GithubTokenValidationResult } from "./github-token-validation.js" -import { validateGithubToken } from "./github-token-validation.js" +import { githubInvalidTokenMessage, validateGithubToken } from "./github-token-validation.js" import { resolvePathFromCwd } from "./path-helpers.js" import { withFsPathContext } from "./runtime.js" import { ensureStateDotDockerGitRepo } from "./state-repo-github.js" @@ -204,7 +204,10 @@ export const runGithubRemoveDeleteRepoScope = ( export const rejectGithubTokenWithRepositoryDeleteScope = (token: string): Effect.Effect => validateGithubToken(token).pipe( Effect.flatMap((validation) => { - if (validation.status !== "valid" || validation.oauthScopes === null) { + if (validation.status === "invalid") { + return Effect.fail(new AuthError({ message: githubInvalidTokenMessage })) + } + if (validation.status === "unknown" || validation.oauthScopes === null) { return Effect.fail(new AuthError({ message: githubUnverifiedTokenScopesMessage })) } return hasGithubRepositoryDeleteScope(validation.oauthScopes) diff --git a/packages/lib/src/usecases/github-scope-policy.ts b/packages/lib/src/usecases/github-scope-policy.ts index d9250d7a..1f536722 100644 --- a/packages/lib/src/usecases/github-scope-policy.ts +++ b/packages/lib/src/usecases/github-scope-policy.ts @@ -1,4 +1,4 @@ -export const defaultGithubScopes: ReadonlyArray = ["repo", "workflow", "read:org"] +export const defaultGithubScopes: ReadonlyArray = Object.freeze(["repo", "workflow", "read:org"]) export const githubRepositoryDeleteScope = "delete_repo" export const githubForbiddenDeleteRepoScopeMessage = [ "GitHub auth token includes forbidden OAuth scope: delete_repo.", diff --git a/packages/lib/tests/usecases/auth-container-paths.test.ts b/packages/lib/tests/usecases/auth-container-paths.test.ts index 07da3965..2d91214b 100644 --- a/packages/lib/tests/usecases/auth-container-paths.test.ts +++ b/packages/lib/tests/usecases/auth-container-paths.test.ts @@ -190,7 +190,7 @@ describe("auth container paths", () => { const recorded: Array = [] const executor = makeFakeExecutor(recorded) const fetchMock = vi.fn(() => - Promise.resolve(githubUserResponse("repo, workflow, read:org")) + Effect.runPromise(Effect.succeed(githubUserResponse("repo, workflow, read:org"))) ) yield* _( @@ -287,7 +287,7 @@ describe("auth container paths", () => { const recorded: Array = [] const executor = makeFakeExecutor(recorded) const fetchMock = vi.fn(() => - Promise.resolve(githubUserResponse("repo, workflow")) + Effect.runPromise(Effect.succeed(githubUserResponse("repo, workflow"))) ) yield* _( @@ -329,7 +329,7 @@ describe("auth container paths", () => { const recorded: Array = [] const executor = makeFakeExecutor(recorded) const fetchMock = vi.fn(() => - Promise.resolve(githubUserResponse("repo, delete_repo")) + Effect.runPromise(Effect.succeed(githubUserResponse("repo, delete_repo"))) ) const failure = yield* _( @@ -371,7 +371,7 @@ describe("auth container paths", () => { const recorded: Array = [] const executor = makeFakeExecutor(recorded) const fetchMock = vi.fn(() => - Promise.resolve(githubUserResponse(null)) + Effect.runPromise(Effect.succeed(githubUserResponse(null))) ) const failure = yield* _( @@ -411,7 +411,7 @@ describe("auth container paths", () => { const fs = yield* _(FileSystem.FileSystem) const envPath = `${root}/.docker-git/.orch/env/global.env` const fetchMock = vi.fn(() => - Promise.resolve(githubUserResponse("repo, delete_repo")) + Effect.runPromise(Effect.succeed(githubUserResponse("repo, delete_repo"))) ) const failure = yield* _( @@ -448,7 +448,7 @@ describe("auth container paths", () => { const fs = yield* _(FileSystem.FileSystem) const envPath = `${root}/.docker-git/.orch/env/global.env` const fetchMock = vi.fn(() => - Promise.resolve(githubUserResponse(null)) + Effect.runPromise(Effect.succeed(githubUserResponse(null))) ) const failure = yield* _( diff --git a/packages/lib/tests/usecases/github-scope-policy.test.ts b/packages/lib/tests/usecases/github-scope-policy.test.ts index d6c92470..8fb0356f 100644 --- a/packages/lib/tests/usecases/github-scope-policy.test.ts +++ b/packages/lib/tests/usecases/github-scope-policy.test.ts @@ -1,3 +1,4 @@ +import fc from "fast-check" import { describe, expect, it } from "vitest" import { @@ -8,6 +9,21 @@ import { } from "../../src/usecases/github-scope-policy.js" describe("github scope policy", () => { + const deleteRepoScopeArbitrary = fc.constantFrom("delete_repo", "DELETE_REPO", "Delete_Repo", " delete_repo ") + const separatorArbitrary = fc.constantFrom(",", " ", "\n", "\t", ", ", " , ") + const scopeTokenArbitrary = fc.oneof(fc.string(), deleteRepoScopeArbitrary) + const scopeListInputArbitrary = fc.array(scopeTokenArbitrary, { maxLength: 20 }).chain((scopes) => + fc.array(separatorArbitrary, { minLength: scopes.length, maxLength: scopes.length }).map((separators) => + scopes.map((scope, index) => `${scope}${separators[index] ?? ""}`).join("") + ) + ) + const nullableScopeInputArbitrary = fc.oneof(fc.constant(null), fc.constant(undefined), fc.string(), scopeListInputArbitrary) + const forbiddenOnlyInputArbitrary = fc.array(deleteRepoScopeArbitrary, { minLength: 1, maxLength: 20 }).chain((scopes) => + fc.array(separatorArbitrary, { minLength: scopes.length, maxLength: scopes.length }).map((separators) => + scopes.map((scope, index) => `${scope}${separators[index] ?? ""}`).join("") + ) + ) + it("preserves default safe scopes", () => { expect(normalizeGithubScopes(null)).toEqual(defaultGithubScopes) expect(normalizeGithubScopes("")).toEqual(defaultGithubScopes) @@ -25,6 +41,21 @@ describe("github scope policy", () => { expect(normalizeGithubScopes("delete_repo DELETE_REPO")).toEqual(defaultGithubScopes) }) + it("preserves the no-delete-repo invariant for arbitrary scope inputs", () => { + fc.assert( + fc.property(nullableScopeInputArbitrary, (input) => { + expect( + normalizeGithubScopes(input).some((scope) => scope.trim().toLowerCase() === "delete_repo") + ).toBe(false) + }) + ) + fc.assert( + fc.property(forbiddenOnlyInputArbitrary, (input) => { + expect(normalizeGithubScopes(input)).toEqual(defaultGithubScopes) + }) + ) + }) + it("parses GitHub OAuth scope headers and detects repository deletion", () => { expect(parseGithubOauthScopesHeader(null)).toBe(null) expect(parseGithubOauthScopesHeader(undefined)).toBe(null)