feat(auth): import Cockpit Tools accounts through safe adapters - #1357
feat(auth): import Cockpit Tools accounts through safe adapters#1357agentHits wants to merge 6 commits into
Conversation
Что сделано: - добавлен общий bounded adapter framework импорта с v1 allowlist для Google Antigravity; - реализованы file/stdin CLI, management API и локализованный GUI file picker; - добавлены live identity/project validation, атомарный identity upsert и secret-free результаты; - обновлены документация и негативные тесты для лимитов, persistence и canary-утечек. Зачем: - заменить узкий старый PR расширяемым контрактом без привязки CLI/API к одному формату реализации; - исключить попадание refresh token в argv, ответы, логи и DOM; - сохранять только проверенные провайдером аккаунты поверх актуального dev. Результат: - импорт поддерживает доказанный Cockpit Tools Antigravity JSON через файл или stdin; - неизвестные провайдеры/форматы fail closed, а Codex/OpenAI не включены в v1; - GUI проверяет размер файла до чтения и строго валидирует успешный DTO до отображения. Проверка: - focused backend/API: 16 pass; - focused CLI: 1 pass; - focused GUI: 4 pass, full GUI: 711 pass; - typecheck, ESLint, GUI/docs builds, privacy scan и diff-check прошли; - полный bun run test: 10331 pass, 7 skip, 1 fail — нерешённый тест lidge-jun#1007 про piped OAuth login URL; Draft PR не заявляет local CI green.
📝 WalkthroughWalkthroughAdded Cockpit Tools Antigravity JSON account import through the OAuth API, CLI, and dashboard. The change validates input and responses, refreshes Google identity, persists credentials, updates caches, reports aggregate results, and documents supported usage. ChangesAntigravity account import
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProviderAuthPanel
participant OAuthImportRoute
participant importAccounts
participant Google
participant OAuthStore
User->>ProviderAuthPanel: Select Cockpit Tools JSON file
ProviderAuthPanel->>OAuthImportRoute: POST account import document
OAuthImportRoute->>importAccounts: Validate and process records
importAccounts->>Google: Refresh token and fetch user identity
Google-->>importAccounts: Normalized email and credentials
importAccounts->>OAuthStore: Insert or update credential
OAuthStore-->>importAccounts: Record outcome
importAccounts-->>OAuthImportRoute: Aggregate import result
OAuthImportRoute-->>ProviderAuthPanel: Safe result counts
ProviderAuthPanel-->>User: Display completion status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
intake: hygiene-blocked
|
⏳ DRAFT
What to do
Review readiness checklist
3/4 boxes ticked. This PR stays in draft until every box above is ticked. |
Что сделано: - добавлен проверенный screenshot панели импорта Google Antigravity без email и токенов. Зачем: - политика репозитория требует встроенное визуальное доказательство для изменения интерфейса; - авторизованная браузерная загрузка GitHub недоступна в текущей среде. Результат: - Draft PR может встроить стабильный raw asset из ветки форка без стороннего хостинга. Проверка: - screenshot визуально проверен основным агентом, QA и reviewer; - на изображении нет credential content, email или token-shaped данных.
Что сделано: - исправлен запуск subprocess-теста через канонический fileURLToPath; - добавлена строгая fail-closed проверка CLI import-result DTO; - добавлены негативные fixtures для counts, индексов, status/code и утечки canary. Зачем: - закрыть сбой lidge-jun#1007 в worktree-путях со скобками; - не допустить отображения или нормализации повреждённого ответа API. Результат: - валидный импорт остаётся совместимым; - повреждённый HTTP 200 завершается фиксированной безопасной ошибкой. Проверка: - bun run test: 10333 pass, 7 skip, 0 fail; - bun run typecheck и bun run privacy:scan; - GUI 711/711, lint/build, docs build; - focused import/API/CLI 113/113.
|
@coderabbitai review |
|
@lidge-jun @Ingwannu — this PR changes OAuth credential admission and persistence. The exact head 57b13d6 has a green local full suite (10333 pass, 7 skip, 0 fail), privacy/typecheck/API/CLI/GUI/docs gates, and independent boundary review with no product finding. Please perform the maintainer security review and, if the boundary is acceptable, apply the maintainer-sponsored label required by MAINTAINERS.md. |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@gui/src/components/provider-workspace/ProviderAuthPanel.tsx`:
- Around line 219-223: Separate the account refresh call in the import flow
around setImportResult and setImportStatus so a rejection from
authHandlers.onRetryAccounts does not enter the import failure catch. Preserve
the validated result and “complete” status after a successful response, while
handling refresh errors independently; add a regression test covering a valid
import followed by a rejected onRetryAccounts call.
In `@gui/src/i18n/ru.ts`:
- Line 1082: Update the Russian translation value for pws.cockpitImportComplete
so the unsupported count uses the grammatically correct noun phrase
“неподдерживаемых — {unsupported}” instead of “не поддерживается —
{unsupported}”, preserving the other summary labels and placeholders.
In `@src/cli/account-extended.ts`:
- Around line 514-520: Update cmdImport to bound the import POST by creating an
AbortController and scheduling cancellation with the existing import timeout
convention, then pass its signal through apiJson options. Ensure the timer is
cleaned up after the request and preserve the existing status === 0 handling via
proxyUnreachable(); if a configurable importTimeoutMs is introduced, add it to
AccountDeps alongside stdinTimeoutMs.
- Around line 474-491: In cmdImport, parse all option flags—wantsJson,
formatArg, fileArg, and fromStdin—before extracting the positional provider with
args.shift(). Preserve the existing admission validation and error behavior,
ensuring options placed before the provider are consumed as flags rather than
treated as the provider.
In `@src/oauth/account-import/index.ts`:
- Around line 2-6: Export ACCOUNT_IMPORT_MAX_REQUEST_BYTES from the
src/oauth/account-import barrel alongside the existing account-import constants,
then update the management OAuth route import to consume it from
../../oauth/account-import instead of ./types. Preserve other imports and keep
internal-only symbols unexported.
In `@tests/cli-account.test.ts`:
- Around line 1681-1688: Expand the account import tests around the existing
source-selection cases to cover both --file and --stdin, neither source option,
and --file with an empty or missing value, asserting each expected rejection.
Add stdin tests using stdinFrom’s isTTY option for stdin_required and
AccountDeps.stdinTimeoutMs for stdin_timeout. Import and use
ACCOUNT_IMPORT_MAX_BYTES in the oversized-input test instead of duplicating 256
* 1024.
🪄 Autofix
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: c2c6d6a6-33b2-48c6-aeac-4db8a2365746
⛔ Files ignored due to path filters (1)
docs-site/public/screenshots/cockpit-account-import.pngis excluded by!**/*.png
📒 Files selected for processing (27)
docs-site/src/content/docs/guides/providers.mddocs-site/src/content/docs/ja/guides/providers.mddocs-site/src/content/docs/ko/guides/providers.mddocs-site/src/content/docs/ru/guides/providers.mddocs-site/src/content/docs/zh-cn/guides/providers.mdgui/src/components/provider-workspace/ProviderAuthPanel.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/tests/provider-account-import.test.tsxsrc/cli/account-extended.tssrc/cli/account.tssrc/oauth/account-import/google-antigravity-adapter.tssrc/oauth/account-import/index.tssrc/oauth/account-import/parser.tssrc/oauth/account-import/registry.tssrc/oauth/account-import/service.tssrc/oauth/account-import/types.tssrc/oauth/google-antigravity.tssrc/oauth/store.tssrc/server/management/oauth-account-routes.tstests/account-import.test.tstests/cli-account.test.tstests/oauth-accounts-api.test.ts
Что сделано: - сохранён успешный GUI-результат при сбое обновления списка аккаунтов; - исправлен порядок CLI-флагов и добавлен ограниченный таймаут import POST; - восстановлена barrel-граница request-size constant; - расширены негативные CLI/GUI fixtures и исправлена русская сводка. Зачем: - закрыть шесть подтверждённых замечаний CodeRabbit без расширения OAuth-контракта; - исключить ложный статус ошибки, зависание CLI и непокрытые admission-ветки. Результат: - import остаётся fail-closed и secret-safe; - все подтверждённые review cases покрыты регрессиями. Проверка: - bun run test: 10337 pass, 7 skip, 0 fail; - focused import/API/CLI: 117/117; - GUI: 712/712, focused 5/5; - typecheck, privacy, lint, build и diff-check.
|
@lidge-jun @Ingwannu — correction to my previous security-review request: the final exact PR head is All six CodeRabbit findings are fixed and resolved. Exact-head local verification is green: the full suite reports 10337 pass, 7 skip, 0 fail, with typecheck, privacy, focused API/CLI/GUI tests, GUI lint/build, and docs build evidence also passing. Please review the credential/OAuth admission and persistence boundary and, if acceptable, apply the maintainer-owned |
Что сделано: - объединена существующая ветка PR lidge-jun#1357 со свежим upstream/dev; - перенесён проверенный v2.11.1 candidate и post-review исправления отмены, DTO и GUI; - добавлена турецкая локализация и устранено предупреждение React Doctor. Зачем: - сохранить историю существующего PR и обновить её обычным fast-forward push без force; - привязать безопасный импорт credentials к актуальной интеграционной ветке. Результат: - двухродительский merge commit сохраняет старый PR head и свежий dev в ancestry; - итоговый diff содержит ровно 32 проверенных пути. Проверка: - runtime: 10434 pass, 7 skip, 0 fail; - GUI: 721 pass, 0 fail; - focused backend: 133 pass, 0 fail; - typecheck, privacy scan, React Doctor, GUI/docs builds и git diff --check: PASS.
|
@lidge-jun @Ingwannu @Wibias — updated maintainer security-review request for the current exact candidate. The branch is now updated to the latest Exact candidate: Local verification on the candidate tree is complete:
This PR changes OAuth credential admission and persistence and therefore requires maintainer security review and the maintainer-owned Could a maintainer please review this exact head and, if the boundary is acceptable, apply |
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Requesting changes on the current reviewed head be0db410b062116aee17adcd1ab2674e6b20485c.
I found two P2 lifecycle/availability issues in the account-import path:
-
Cancellation after a partial commit skips post-write reconciliation/cache invalidation.
importAccounts()can commit one or more records and then collapse a later abort to{ ok: false, status: 408, code: "import_cancelled" }, discarding the already-completed results. The management route returns immediately on!imported.ok, soreconcileLiveStateStores(), model-cache clearing, routed-model inflight clearing, and quota-cache clearing do not run even thoughauth.jsonmay already have changed. Please preserve an internal changed/committed signal (or conservatively reconcile onimport_cancelled) and add a deterministic regression where record 1 commits, record 2 blocks, then the request aborts. -
The 10-minute server deadline does not bound request-body ingestion. The route creates a deadline
AbortController, butreadBoundedJsonRequestBody(req, ...)reads withreq.signal; the deadline controller is only passed later toimportAccounts(). A slow trickling upload can therefore continue beyond the advertised server deadline. Please let the bounded body reader accept a signal override (or otherwise compose the deadline withreq.signal) and ensure a deadline/abort is reported as408 import_cancelled, not400 invalid_document. Add a regression for a body read that remains active until the deadline fires.
The broader security design looks solid: exact provider/format admission before credential traversal, bounded input, provider-derived identity/project validation before persistence, atomic identity upsert, and fixed secret-free API/CLI/GUI projections. I did not find a credential leak or unsafe credential-admission path beyond the lifecycle issues above.
Wibias
added
the
maintainer-sponsored
ChatGPT Codex Connector
github-actions
Bot
removed
the
intake: hygiene-blocked
Что сделано: - добавлен ограниченный импорт учётных записей Google Antigravity из экспорта Cockpit Tools через API, CLI и интерфейс; - отмена и серверный срок теперь охватывают чтение тела и импорт, а частично сохранённые данные согласуются с живым состоянием и кешами; - добавлены проверки ошибок, отмены, конфиденциальности, локализации и пользовательского интерфейса. Зачем: - дать безопасный способ переноса учётных записей без раскрытия токенов и без зависания запроса; - сохранить корректное состояние после отмены, включая редкий случай уже выполненной записи. Риски: - изменения затрагивают OAuth, хранение учётных данных и management API; границы проверены отрицательными сценариями и полным набором тестов. Проверки: - команды и результаты будут повторно привязаны к итоговому объединяющему коммиту перед отправкой; - ограничение: окончательная готовность зависит от новых проверок GitHub CI и повторного maintainer review.
|
@Wibias — both requested P2 lifecycle fixes are now included in exact head
Deterministic regressions cover same-record and later-record post-commit cancellation, route-level partial commit plus cache invalidation, stalled body cancellation, and malformed JSON classification. The branch was fast-forwarded without force through a two-parent bridge containing both the prior PR head Exact-head local verification:
Please re-review the resolved P2 findings on this exact head when convenient. The PR remains draft until the new GitHub CI is green. |
2 participants
Summary
upstream/devthrough a two-parent bridge commit, preserving the previous PR head for a normal fast-forward update without force-push.Verification
Exact candidate:
91556e82d6bce0b947f313666e97637d845d21a6.bun run test— 10,457 pass, 7 platform skips, 0 fail, 49,906 assertions across 650 files.bun test tests/account-import.test.ts tests/oauth-accounts-api.test.ts tests/bounded-body.test.ts— 44 pass, 185 assertions.bun run typecheck.bun run privacy:scan.cd gui && bun test tests— 721 pass, 3,472 assertions across 129 files.cd gui && bun run lintandbun run lint:i18nthrough the pinned ESLint installation.git diff --check.be0db410b062116aee17adcd1ab2674e6b20485cand currentupstream/devd517161aeaa3a974ad3c0360ff0c97b03b4c4520are both ancestors of the exact candidate.upstream/devplus the 34-path Agent Flow checkpoint0d7dc559080f2991d2d1ed1e58fcd8f245d704fcf1479ab793aee1048ab3c273.Dashboard screenshot
Security review
This changes OAuth credential admission and persistence and therefore requires explicit maintainer security review before merge. The maintainer-owned
maintainer-sponsoredlabel is present on the PR.Local independent review found no security release blocker. The implementation uses file/stdin-only secret input, exact provider/format admission, bounded parsing, provider-derived identity and project validation before persistence, atomic duplicate upsert, fixed result codes, strict fail-closed API/CLI/GUI projection, request cancellation with a bounded deadline, canary non-leakage tests, and post-write state/cache reconciliation.
Residual P3: cancellation that arrives after the atomic upsert has already started can produce an intentionally ambiguous
import_cancelledoutcome even though the already-validated credential may have committed. The operation remains identity-idempotent and never persists an unvalidated credential.Checklist
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.