Skip to content

[codex] Remove GitHub repository delete scope#299

Draft
rikohomeless wants to merge 3 commits into
ProverCoderAI:mainfrom
rikohomeless:issue-288
Draft

[codex] Remove GitHub repository delete scope#299
rikohomeless wants to merge 3 commits into
ProverCoderAI:mainfrom
rikohomeless:issue-288

Conversation

@rikohomeless
Copy link
Copy Markdown
Contributor

Summary

  • centralize GitHub OAuth scope normalization and strip delete_repo case-insensitively
  • run gh auth refresh --remove-scopes delete_repo after web login before reading the token
  • validate reported X-OAuth-Scopes before persisting generated or manual GitHub tokens
  • add focused tests for safe scopes, refresh ordering, stream output, and rejection before persistence

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

  • bun install --frozen-lockfile
  • bun run --cwd packages/lib test tests/usecases/github-scope-policy.test.ts tests/usecases/auth-container-paths.test.ts
  • bun run --cwd packages/api test tests/auth-github-login-stream.test.ts
  • bun run typecheck
  • bun run --cwd packages/api typecheck
  • bun run --cwd packages/lib test tests/usecases/auth-github-status.test.ts tests/usecases/github-token-preflight.test.ts
  • bun run --cwd packages/api test tests/auth.test.ts

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Безопасность

    • Централизована политика GitHub‑scopes: токены проверяются и отклоняются при наличии запрещённого scope delete_repo.
    • При интерактивном входе добавлен шаг удаления опасного scope перед получением и сохранением токена.
    • Улучшены тексты ошибок для случаев запрещённых или непроверенных scopes.
  • Исправления ошибок

    • Расширена валидация OAuth‑scopes и передача статуса токена.
    • Гарантировано, что небезопасные токены не сохраняются.
  • Тесты

    • Добавлены и расширены тесты покрытия политики scopes и сценариев логина.

Walkthrough

PR добавляет централизованную политику GitHub OAuth scope и интегрирует её в валидацию токенов и flow авторизации (lib, app, api). Токены с запрещённым scope delete_repo удаляются/отклоняются на нескольких этапах; изменения покрыты unit и интеграционными тестами.

Changes

GitHub Scope Policy Implementation and Integration

Layer / File(s) Summary
Scope policy (app & lib)
packages/app/src/lib/usecases/github-scope-policy.ts, packages/lib/src/usecases/github-scope-policy.ts
Добавлены defaultGithubScopes, githubRepositoryDeleteScope, сообщения и функции normalizeGithubScopes, parseGithubOauthScopesHeader, hasGithubRepositoryDeleteScope.
Token validation (app & lib)
packages/app/src/lib/usecases/github-token-validation.ts, packages/lib/src/usecases/github-token-validation.ts
Добавлено поле oauthScopes в GithubTokenValidationResult; validateGithubToken парсит x-oauth-scopes и возвращает oauthScopes для всех статусов.
Lib & App auth-flow integration
packages/lib/src/usecases/auth-github.ts, packages/app/src/lib/usecases/auth-github.ts
Добавлены runGithubRemoveDeleteRepoScope и rejectGithubTokenWithRepositoryDeleteScope; удалена локальная нормализация scopes; прокидывание oauthScopes в статусы; вызовы удаления/проверки scopes в интерактивном и неинтерактивном flow.
API service integration
packages/api/src/services/auth-github-login-stream.ts
Обновлены импорты; GithubSetupError теперь включает AuthError; toApiError сопоставляет теги ошибок; в finalizeGithubLogin добавлен вызов удаления delete_repo и последующая проверка токена.
Tests (policy & integration)
packages/lib/tests/usecases/github-scope-policy.test.ts, packages/lib/tests/usecases/auth-container-paths.test.ts, packages/api/tests/auth-github-login-stream.test.ts
Добавлены unit-тесты политики scope; расширены интеграционные тесты с мокаутом fetch, проверкой порядка container-команд и отказов при delete_repo или отсутствии OAuth scopes; API-тест проверяет порядок вывода пост-login сообщений.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed Название точно описывает основное изменение: удаление GitHub repository delete scope из генерируемых и хранящихся токенов.
Description check ✅ Passed Описание содержит Summary, Why, Validation и закрывает связанную issue #288, но не соответствует шаблону репозитория (отсутствует «Source TZ / Issues» и «Requirements Alignment» блоки).
Linked Issues check ✅ Passed PR полностью реализует требование из #288: централизует нормализацию GitHub OAuth scopes, удаляет delete_repo из генерируемых/ручных токенов и валидирует scopes перед сохранением.
Out of Scope Changes check ✅ Passed Все изменения в PR соответствуют целям #288 (исключение delete_repo scope) и улучшению безопасности токенов, без посторонних модификаций.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Requirements Alignment ✅ Passed Все требования issue #288 реализованы: нормализация scope централизована, удаляет delete_repo, gh auth refresh вызывается перед чтением, X-OAuth-Scopes валидируется, тесты полные, функции не изменены.
Security Regression ✅ Passed Анализ не выявил регрессий безопасности. Токены валидируются перед persistence, delete_repo фильтруется, command injection защищен array args, credentials не логируются.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 837ba27 and ad205fd.

📒 Files selected for processing (10)
  • packages/api/src/services/auth-github-login-stream.ts
  • packages/api/tests/auth-github-login-stream.test.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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 use any, unknown, eslint-disable, ts-ignore, or as type 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 through pipe() and Effect.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), @throws Never (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.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/lib/tests/usecases/github-scope-policy.test.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/lib/tests/usecases/github-scope-policy.test.ts
  • packages/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/catch for 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.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/lib/tests/usecases/github-scope-policy.test.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/lib/tests/usecases/github-scope-policy.test.ts
  • packages/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.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/lib/tests/usecases/github-scope-policy.test.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/lib/tests/usecases/github-scope-policy.test.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/lib/tests/usecases/github-scope-policy.test.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/lib/tests/usecases/github-scope-policy.test.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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

Comment thread packages/api/src/services/auth-github-login-stream.ts Outdated
Comment thread packages/api/src/services/auth-github-login-stream.ts Outdated
Comment thread packages/app/src/lib/usecases/auth-github.ts
Comment thread packages/app/src/lib/usecases/github-scope-policy.ts
Comment thread packages/lib/src/usecases/github-token-validation.ts Outdated
Comment thread packages/lib/tests/usecases/auth-container-paths.test.ts
Comment thread packages/lib/tests/usecases/github-scope-policy.test.ts
@rikohomeless
Copy link
Copy Markdown
Contributor Author

rikohomeless commented May 14, 2026

AI Session Backup

Commit: fa08627
Status: success
Files: 7 (4.15 MB)
Links: README | Manifest

git status

On branch issue-288
Your branch is up to date with 'fork/issue-288'.

nothing to commit, working tree clean

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between ad205fd and fa08627.

📒 Files selected for processing (9)
  • packages/api/src/services/auth-github-login-stream.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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 use any, unknown, eslint-disable, ts-ignore, or as type 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 through pipe() and Effect.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), @throws Never (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.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/api/src/services/auth-github-login-stream.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/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/catch for 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.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/api/src/services/auth-github-login-stream.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/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.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/api/src/services/auth-github-login-stream.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/api/src/services/auth-github-login-stream.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/api/src/services/auth-github-login-stream.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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.ts
  • packages/app/src/lib/usecases/github-token-validation.ts
  • packages/lib/src/usecases/github-token-validation.ts
  • packages/app/src/lib/usecases/auth-github.ts
  • packages/app/src/lib/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/api/src/services/auth-github-login-stream.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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 работает идемпотентно: если scope delete_repo не был запрошен при логине, команда успешно завершится без ошибки, просто не имея эффекта для этого scope. Никаких неожиданных поведений или ошибок при отсутствии scope, плюс существующая валидация на Line 263 обеспечивает дополнительную защиту.

Comment thread packages/lib/src/usecases/auth-github.ts
Comment thread packages/lib/src/usecases/github-scope-policy.ts Outdated
Comment thread packages/lib/tests/usecases/github-scope-policy.test.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ 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

📥 Commits

Reviewing files that changed from the base of the PR and between fa08627 and b652da7.

📒 Files selected for processing (4)
  • packages/lib/src/usecases/auth-github.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/tests/usecases/auth-container-paths.test.ts
  • packages/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 use any, unknown, eslint-disable, ts-ignore, or as type 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 through pipe() and Effect.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), @throws Never (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.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/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.ts
  • packages/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/catch for 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.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/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.ts
  • packages/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.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/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.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/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.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/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.ts
  • packages/lib/src/usecases/github-scope-policy.ts
  • packages/lib/src/usecases/auth-github.ts
  • packages/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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant