[codex] Remove GitHub repository delete scope#299
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughPR добавляет централизованную политику GitHub OAuth scope и интегрирует её в валидацию токенов и flow авторизации (lib, app, api). Токены с запрещённым scope ChangesGitHub Scope Policy Implementation and Integration
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Flow as Auth Flow
participant GH as GitHub Auth
participant Remove as runGithubRemoveDeleteRepoScope
participant Validate as validateGithubToken
participant Reject as rejectGithubTokenWithRepositoryDeleteScope
User->>Flow: инициирует web login
Flow->>GH: gh auth login --web
GH->>Flow: выдан token + x-oauth-scopes (возможно delete_repo)
Flow->>Remove: gh auth refresh --remove-scopes delete_repo
Remove->>Flow: refresh завершён
Flow->>Validate: получить token из контейнера и вызвать validateGithubToken
Validate->>GH: запрос user/token info
GH->>Validate: тело + x-oauth-scopes
Validate->>Flow: возврат oauthScopes
Flow->>Reject: rejectGithubTokenWithRepositoryDeleteScope(token)
alt delete_repo присутствует или scopes неизвестны
Reject->>Flow: AuthError (reject)
Flow->>User: отказ с сообщением
else
Reject->>Flow: success
Flow->>User: login completed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api/src/services/auth-github-login-stream.ts`:
- Around line 173-197: The two functions runGithubRemoveDeleteRepoScope and
rejectGithubTokenWithRepositoryDeleteScope are duplicated from packages/lib;
remove the local implementations and instead import and re-export or directly
use the implementations from packages/lib (the file that contains the originals,
e.g., lib's usecases/auth-github.ts). Replace the local definitions in
auth-github-login-stream.ts with imports of runGithubRemoveDeleteRepoScope and
rejectGithubTokenWithRepositoryDeleteScope from the lib package, update any type
or error references to align with the lib exports, and ensure the package's
tsconfig/exports allow importing these symbols so the duplicated code is
eliminated.
- Around line 79-91: The nested ternary in toApiError should be replaced with
Effect's exhaustive pattern matching: use Match.exhaustive on the
GithubSetupError discriminant to handle "_tag" === "AuthError" (return new
ApiBadRequestError({message: error.message})), "_tag" === "CommandFailedError"
(return new ApiBadRequestError({message: `${error.command} failed (exit
${error.exitCode}).`})), and the fallback case should return new
ApiInternalError({message: String(error), cause: error}); import Match from
Effect if not already imported and ensure the match is exhaustive for
GithubSetupError variants.
In `@packages/app/src/lib/usecases/auth-github.ts`:
- Around line 204-210: rejectGithubTokenWithRepositoryDeleteScope currently
allows tokens when validateGithubToken degrades to unknown/oauthScopes=null;
change it to "fail-closed": after calling validateGithubToken (in
rejectGithubTokenWithRepositoryDeleteScope) check validation.status and
validation.oauthScopes and treat any non-OK/verified result (e.g., status ===
"unknown" or oauthScopes == null) as a failure by returning Effect.fail(new
AuthError(...)), and also fail when
hasGithubRepositoryDeleteScope(validation.oauthScopes) is true; update error
message/context to indicate unverified or forbidden scopes. Use the existing
validateGithubToken, hasGithubRepositoryDeleteScope and AuthError symbols to
locate and implement this logic.
In `@packages/app/src/lib/usecases/github-scope-policy.ts`:
- Around line 1-58: Remove the duplicated local implementations
(defaultGithubScopes, githubRepositoryDeleteScope,
githubForbiddenDeleteRepoScopeMessage, normalizeGithubScopes,
parseGithubOauthScopesHeader, hasGithubRepositoryDeleteScope) and replace them
by importing and re-exporting those symbols from the shared package
("@effect-template/lib/usecases/github-scope-policy"); ensure the file no longer
defines the functions/consts itself but instead imports the named exports
defaultGithubScopes, githubRepositoryDeleteScope,
githubForbiddenDeleteRepoScopeMessage, normalizeGithubScopes,
parseGithubOauthScopesHeader, hasGithubRepositoryDeleteScope and re-exports them
so existing consumers keep the same exported API.
In `@packages/lib/src/usecases/github-token-validation.ts`:
- Line 75: Replace the bracket-access to response headers with the typed .get()
accessor: when computing oauthScopes (near parseGithubOauthScopesHeader usage),
call response.headers.get("x-oauth-scopes") instead of
response.headers["x-oauth-scopes"]; ensure the value passed into
parseGithubOauthScopesHeader is the string (or null/undefined handled) returned
by response.headers.get and adjust any null checks or types in the surrounding
code (the response variable and parseGithubOauthScopesHeader call) accordingly.
In `@packages/lib/tests/usecases/auth-container-paths.test.ts`:
- Around line 185-187: The mock fetch function fetchMock currently returns
Effect.runPromise(Effect.succeed(...)) which is unnecessary; update the
fetchMock implementation (the vi.fn<typeof globalThis.fetch> that returns the
githubUserResponse) to return Promise.resolve(githubUserResponse("repo,
workflow, read:org")) directly so the mock returns a real Promise instead of
wrapping an Effect.
In `@packages/lib/tests/usecases/github-scope-policy.test.ts`:
- Around line 28-33: Add an explicit test asserting
parseGithubOauthScopesHeader(null) (and optionally
parseGithubOauthScopesHeader(undefined)) returns null to cover the edge-case
currently only implied by hasGithubRepositoryDeleteScope(null); update the test
in the github-scope-policy.test.ts file to call parseGithubOauthScopesHeader
with null/undefined and expect null so the behavior of
parseGithubOauthScopesHeader is verified directly alongside existing assertions
for hasGithubRepositoryDeleteScope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e6fcc287-bda5-4488-b29d-2b071f96bc66
📒 Files selected for processing (10)
packages/api/src/services/auth-github-login-stream.tspackages/api/tests/auth-github-login-stream.test.tspackages/app/src/lib/usecases/auth-github.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/auth-github.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/src/usecases/github-token-validation.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/tests/usecases/github-scope-policy.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never useany,unknown,eslint-disable,ts-ignore, orastype assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through.exhaustive()pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects:Effect<Success, Error, Requirements>. Compose effects throughpipe()andEffect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example:Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including:@pure(true/false),@effect(required services),@invariant(mathematical invariants),@precondition,@postcondition,@complexity(time and space),@throwsNever (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...
Files:
packages/api/tests/auth-github-login-stream.test.tspackages/lib/src/usecases/github-token-validation.tspackages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-scope-policy.tspackages/app/src/lib/usecases/auth-github.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/api/src/services/auth-github-login-stream.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example:fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example:Effect.provide(MockService), Effect.runPromise.
Files:
packages/api/tests/auth-github-login-stream.test.tspackages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code:any,eslint-disable,ts-ignore,async/await, raw Promise chains (then/catch),Promise.all,try/catchfor logic control,console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects:Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.
Files:
packages/api/tests/auth-github-login-stream.test.tspackages/lib/src/usecases/github-token-validation.tspackages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-scope-policy.tspackages/app/src/lib/usecases/auth-github.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/api/src/services/auth-github-login-stream.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.
Files:
packages/api/tests/auth-github-login-stream.test.tspackages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Files:
packages/api/tests/auth-github-login-stream.test.tspackages/lib/src/usecases/github-token-validation.tspackages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-scope-policy.tspackages/app/src/lib/usecases/auth-github.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/api/src/services/auth-github-login-stream.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce path traversal or writes outside intended project/container state directories
Files:
packages/api/tests/auth-github-login-stream.test.tspackages/lib/src/usecases/github-token-validation.tspackages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-scope-policy.tspackages/app/src/lib/usecases/auth-github.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/api/src/services/auth-github-login-stream.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Files:
packages/api/tests/auth-github-login-stream.test.tspackages/lib/src/usecases/github-token-validation.tspackages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-scope-policy.tspackages/app/src/lib/usecases/auth-github.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/api/src/services/auth-github-login-stream.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/api/tests/auth-github-login-stream.test.tspackages/lib/src/usecases/github-token-validation.tspackages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-scope-policy.tspackages/app/src/lib/usecases/auth-github.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/api/src/services/auth-github-login-stream.ts
🔇 Additional comments (5)
packages/lib/src/usecases/github-scope-policy.ts (1)
1-57: LGTM!packages/api/tests/auth-github-login-stream.test.ts (1)
19-27: LGTM!packages/lib/src/usecases/auth-github.ts (2)
185-262: LGTM!
274-296: LGTM!packages/lib/tests/usecases/auth-container-paths.test.ts (1)
277-315: LGTM!Also applies to: 317-357, 359-394
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/lib/src/usecases/auth-github.ts`:
- Around line 204-213: rejectGithubTokenWithRepositoryDeleteScope currently
collapses non-valid statuses into one error about unverified scopes; update the
validation branch to distinguish validation.status values instead of treating
all non-"valid" cases the same: call validateGithubToken and if
validation.status === "invalid" return Effect.fail(new AuthError({ message:
githubInvalidTokenMessage })), if validation.status === "unknown" or
validation.oauthScopes === null return Effect.fail(new AuthError({ message:
githubUnverifiedTokenScopesMessage })); keep the subsequent
hasGithubRepositoryDeleteScope(validation.oauthScopes) check as-is to reject
tokens with delete scope using githubForbiddenDeleteRepoScopeMessage.
In `@packages/lib/src/usecases/github-scope-policy.ts`:
- Line 1: The exported defaultGithubScopes is only type-readonly and can be
mutated at runtime; change its initialization to produce a truly immutable value
by wrapping the literal in Object.freeze (e.g., export const defaultGithubScopes
= Object.freeze(["repo","workflow","read:org"]) as ReadonlyArray<string>),
keeping the exported name defaultGithubScopes and its ReadonlyArray<string> type
so consumers cannot alter the array at runtime.
In `@packages/lib/tests/usecases/github-scope-policy.test.ts`:
- Around line 10-35: Add a fast-check property test that asserts the invariant
of the policy module: for any arbitrary input (including null, undefined, empty
string, random strings, arrays of strings, and strings with mixed
separators/spaces/casing) the function normalizeGithubScopes(...) never returns
a scope equal to "delete_repo" case-insensitively; also assert that when every
requested scope would be forbidden the function falls back to
defaultGithubScopes. Locate normalizeGithubScopes and defaultGithubScopes in the
existing github-scope-policy.test.ts and create a new it(...) using fast-check's
property/assert (no async/await) that generates strings and string arrays, feeds
them through normalizeGithubScopes (and parseGithubOauthScopesHeader if needed),
and checks both that the returned list does not contain any entry that
lowercased equals "delete_repo" and that the fallback condition returns
defaultGithubScopes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 56c0186b-7e91-4008-8890-bf11a04de6e1
📒 Files selected for processing (9)
packages/api/src/services/auth-github-login-stream.tspackages/app/src/lib/usecases/auth-github.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/auth-github.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/src/usecases/github-token-validation.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/tests/usecases/github-scope-policy.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never useany,unknown,eslint-disable,ts-ignore, orastype assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through.exhaustive()pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects:Effect<Success, Error, Requirements>. Compose effects throughpipe()andEffect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example:Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including:@pure(true/false),@effect(required services),@invariant(mathematical invariants),@precondition,@postcondition,@complexity(time and space),@throwsNever (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-token-validation.tspackages/app/src/lib/usecases/auth-github.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/api/src/services/auth-github-login-stream.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/src/usecases/github-scope-policy.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example:fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example:Effect.provide(MockService), Effect.runPromise.
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code:any,eslint-disable,ts-ignore,async/await, raw Promise chains (then/catch),Promise.all,try/catchfor logic control,console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects:Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-token-validation.tspackages/app/src/lib/usecases/auth-github.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/api/src/services/auth-github-login-stream.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/src/usecases/github-scope-policy.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-token-validation.tspackages/app/src/lib/usecases/auth-github.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/api/src/services/auth-github-login-stream.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/src/usecases/github-scope-policy.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce path traversal or writes outside intended project/container state directories
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-token-validation.tspackages/app/src/lib/usecases/auth-github.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/api/src/services/auth-github-login-stream.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/src/usecases/github-scope-policy.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-token-validation.tspackages/app/src/lib/usecases/auth-github.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/api/src/services/auth-github-login-stream.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/src/usecases/github-scope-policy.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/app/src/lib/usecases/github-token-validation.tspackages/lib/src/usecases/github-token-validation.tspackages/app/src/lib/usecases/auth-github.tspackages/app/src/lib/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/api/src/services/auth-github-login-stream.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/src/usecases/github-scope-policy.ts
🔇 Additional comments (7)
packages/app/src/lib/usecases/github-scope-policy.ts (1)
1-62: Этот файл всё ещё дублирует policy-модуль изpackages/lib.Замечание уже поднималось: для security-policy лучше иметь один источник истины и реэкспортировать его, а не поддерживать две копии с
jscpd:ignore, которые могут разъехаться при следующем изменении.packages/app/src/lib/usecases/auth-github.ts (6)
13-23: LGTM!
130-131: LGTM!
205-215: LGTM!
260-263: LGTM!
291-291: LGTM!
187-203: 💤 Low valueПоведение
gh auth refresh --remove-scopesбезопасно при отсутствии scope.Команда
gh auth refresh --remove-scopes delete_repoработает идемпотентно: если scopedelete_repoне был запрошен при логине, команда успешно завершится без ошибки, просто не имея эффекта для этого scope. Никаких неожиданных поведений или ошибок при отсутствии scope, плюс существующая валидация на Line 263 обеспечивает дополнительную защиту.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/lib/tests/usecases/auth-container-paths.test.ts (1)
192-194: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueУпростите реализацию
fetchMock.Использование
Effect.runPromise(Effect.succeed(...))избыточно. Можно напрямую возвращатьPromise.resolve(...).♻️ Предлагаемое упрощение
- const fetchMock = vi.fn<typeof globalThis.fetch>(() => - Effect.runPromise(Effect.succeed(githubUserResponse("repo, workflow, read:org"))) - ) + const fetchMock = vi.fn<typeof globalThis.fetch>(() => + Promise.resolve(githubUserResponse("repo, workflow, read:org")) + )То же самое относится к другим
fetchMockопределениям в строках 289-291, 331-333, 373-375, 413-415 и 450-452.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/tests/usecases/auth-container-paths.test.ts` around lines 192 - 194, The fetch mock implementations in usecases/auth-container-paths.test.ts are overly complex—replace calls that return Effect.runPromise(Effect.succeed(...)) with direct Promise.resolve(...) returns; update each vi.fn<typeof globalThis.fetch>(() => Effect.runPromise(Effect.succeed(githubUserResponse(...)))) (and the other similar fetchMock definitions around the indicated lines) to simply return Promise.resolve(githubUserResponse(...)) so the mock returns a plain Promise as expected by globalThis.fetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/lib/tests/usecases/auth-container-paths.test.ts`:
- Around line 192-194: The fetch mock implementations in
usecases/auth-container-paths.test.ts are overly complex—replace calls that
return Effect.runPromise(Effect.succeed(...)) with direct Promise.resolve(...)
returns; update each vi.fn<typeof globalThis.fetch>(() =>
Effect.runPromise(Effect.succeed(githubUserResponse(...)))) (and the other
similar fetchMock definitions around the indicated lines) to simply return
Promise.resolve(githubUserResponse(...)) so the mock returns a plain Promise as
expected by globalThis.fetch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f2017093-b63f-4d65-8eb6-69122d81d194
📒 Files selected for processing (4)
packages/lib/src/usecases/auth-github.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/tests/usecases/auth-container-paths.test.tspackages/lib/tests/usecases/github-scope-policy.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Login context)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (OpenCode)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: E2E (Browser command)
- GitHub Check: Final build (windows-latest)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never useany,unknown,eslint-disable,ts-ignore, orastype assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through.exhaustive()pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects:Effect<Success, Error, Requirements>. Compose effects throughpipe()andEffect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example:Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including:@pure(true/false),@effect(required services),@invariant(mathematical invariants),@precondition,@postcondition,@complexity(time and space),@throwsNever (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example:fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example:Effect.provide(MockService), Effect.runPromise.
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code:any,eslint-disable,ts-ignore,async/await, raw Promise chains (then/catch),Promise.all,try/catchfor logic control,console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects:Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce path traversal or writes outside intended project/container state directories
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/lib/tests/usecases/github-scope-policy.test.tspackages/lib/src/usecases/github-scope-policy.tspackages/lib/src/usecases/auth-github.tspackages/lib/tests/usecases/auth-container-paths.test.ts
🔇 Additional comments (25)
packages/lib/src/usecases/github-scope-policy.ts (5)
1-10: LGTM!
12-17: LGTM!
19-37: LGTM!
39-57: LGTM!
59-60: LGTM!packages/lib/tests/usecases/github-scope-policy.test.ts (5)
1-10: LGTM!
11-26: LGTM!
27-42: LGTM!
44-57: LGTM!
59-67: LGTM!packages/lib/src/usecases/auth-github.ts (6)
12-12: LGTM!Also applies to: 17-24
129-131: LGTM!
186-202: LGTM!
204-217: LGTM!
262-265: LGTM!
293-293: LGTM!packages/lib/tests/usecases/auth-container-paths.test.ts (9)
10-10: LGTM!Also applies to: 14-17
71-86: LGTM!
170-181: LGTM!
196-282: LGTM!
284-322: LGTM!
324-364: LGTM!
366-406: LGTM!
408-443: LGTM!
445-480: LGTM!
Summary
Why
GitHub repository deletion requires the delete_repo OAuth scope. docker-git should not generate or store tokens that can delete repositories.
Closes #288
Validation