You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(api): accept dashboard sessions on preset item routes - #18386
With REQUIRE_API_KEY=true, a signed-in Playground user can list and create presets, but GET, PUT, and DELETE /api/playground/presets/{id} reject the same dashboard session with 401 Authentication required. The collection and item routes apply different credential rules (see #18385).
Change
Use one preset-auth guard for the collection and item routes. A valid API key or management/dashboard session is accepted on either route when API-key enforcement is enabled.
Check an explicitly supplied API key first: an invalid key remains a 401, even alongside a valid dashboard cookie. With no API key, require a real dashboard session under enforcement, including when dashboard login is disabled; keep the anonymous-allowed behavior when enforcement is off.
Cover item GET/PUT/DELETE with real signed-session, valid-key, missing-credential, invalid-key, and login-disabled cases.
The PR is not CI-green. The completed Oct 27 attempt (CI, Quality, API typecheck) had 13 failed checks, 39 passed, 5 skipped; newer attempts on the same head are in progress, so these counts are historical, not a live status. The CI checkout is the synthetic merge of this head into f1a7e12; the diff changes only the two preset routes, their new auth helper, and the new integration test. Both integration-test shards passed in that attempt, and the new four-case regression passes locally.
Reproduced on the exact base, independent of the preset-auth diff:
Fast Quality Gates: check:build-scope counts 12,160 files against a 12,000 limit; the same tracked-file calculation at f1a7e12 is already 12,158, before this PR's two TypeScript files. Its open-sse-typecheck failures (TS2345 in chatCore.ts and sseParser/geminiResponse.ts) also appear in API Route Typecheck; neither source file is changed here.
Lint: check-test-tla-order finds top-level await after tests in three unchanged unit-test files. i18n Glossary finds usage.quotaAuthRequiredHint using 提供商 instead of 提供者 in unchanged zh-CN.json; the offending value is present on f1a7e12 (as is the corresponding 供應商 in zh-TW.json).
Unit shards 3/5/8 and fast-path 1/3/4 fail in unchanged tests covering i18n, catalog counts/model limits, chat sanitization, the providers page, the CI script entrypoint, GitHub catalog fallback, and a documentation mirror. The i18n, chatCore-sanitization, and SiliconFlow failures were replayed on f1a7e12 and fail identically there (same six tests, same assertion messages); the file://${process.argv[1]} entrypoint flagged by fast-path is likewise present on the exact base. The overlapping failures in regular and fast-path shards corroborate these signatures.
Gate / CI and Gate / Quality summarize failed child jobs; they are not separate preset-auth failures.
Full-suite Vitest failure not reproduced in isolation: CI reports 14 failures across six unchanged UI/i18n test files, including 5-second timeouts. Running those exact six files separately with two workers yields 64/64 passing on f1a7e12 and 64/64 passing on the PR head. The full CI run on the exact base is unavailable, so the full-suite failure (possibly load/order-sensitive) cannot conclusively be attributed to upstream or this PR. None of these tests exercise the preset routes/helper.
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
Bug Fixes
Standardized authentication for playground preset lists and individual presets. Valid API keys or dashboard sessions can authorize access, while invalid API keys and missing required credentials are rejected.
API-key requirements remain enforced when dashboard login is disabled.
Preset collection and item routes now delegate authentication decisions to a shared helper. Integration tests cover dashboard-session and API-key authentication across preset operations.
Adds a helper that validates a supplied API key or checks management authentication when no key is supplied.
Route delegation and integration coverage src/app/api/playground/presets/route.ts, src/app/api/playground/presets/[id]/route.ts, tests/integration/presets-item-dashboard-auth.test.ts
The collection and item routes delegate authentication to the helper. Integration tests cover dashboard sessions, valid and invalid API keys, and missing credentials.
Preset item routes now accept dashboard sessions. No change-specific blocking risk was established, though reported CI failures remain unresolved.
Security architecture risk: Low · up to 0ba0d
The change aligns preset-item access with existing dashboard permissions while preserving rejection of invalid API keys. No introduced security vulnerability was established. Remaining uncertainty concerns browser-request enforcement across configuration and deployment modes, which the added tests do not exercise end to end.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The newly accepted dashboard authority can read, update, or delete any preset in the instance’s shared preset table, rather than only presets associated with a session. Existing dashboard collection access already listed this shared data and permitted creation. The inspected model does not establish per-user or per-tenant isolation.
Trust Boundaries and Controls
observed — The source proxy matches API requests and invokes authorization enforcement. For management requests classified with a dashboard-session subject, unsafe mutations pass an origin check or the permitted session-bound CSRF-token fallback before reaching handlers.
inferred — When dashboard login is disabled, preset requests can receive an anonymous proxy subject before cookie verification, so the dashboard-subject origin/CSRF branch is not universal. This does not establish a browser attack: PUT/DELETE require cross-origin preflight, and the inspected shared CORS code does not grant credentialed cross-origin access. Deployment-specific behavior was not exercised.
Hardening Proposals
proposed — Add proxy-level regression coverage for cookie-authenticated item mutations with accepted and rejected origins/CSRF tokens, including API-key enforcement combined with disabled dashboard login. This would verify the composition of route authentication and browser controls rather than treating direct-handler success as end-to-end assurance.
Pre-merge checks: 5 · 2
Failed checks: 2 warnings
Check name
Status
Explanation
Resolution
Description check
The description covers the problem, the change, and the CI diagnosis, and it links the issue (Fixes #18385). It does not follow the repository template. The template sections Related Issues (only the …
Restructure the description to match the template headings: Summary, Related Issues, Target Branch, Validation, Tests Added Or Updated, Coverage Notes, and Reviewer Notes. Under Target Branch, confirm the base is the active release/v3.8.x b…
Docstring Coverage
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 5 passed
Check name
Status
Explanation
Title check
The title follows Conventional Commits with type fix and scope api, and its subject matches the main change: preset item routes now accept dashboard sessions. The title is accurate and specific.
Linked Issues check
Issue [#18385] requires the same credential rules for preset collection and item routes. GET, PUT, and DELETE now use checkPlaygroundPresetAuth. The helper accepts a valid API key, checks a ma…
Out of Scope Changes check
The changes stay within issue [#18385]. They extract the shared preset authentication guard, apply it to the collection and item routes, and add regression tests for the required credential cases. No …
Tests For Production Code
PASS: The PR changes production code under src/ in the preset routes and adds src/lib/api/playgroundPresetAuth.ts. It also adds tests/integration/presets-item-dashboard-auth.test.ts, which cover…
No Ai Attribution
No prohibited AI attribution appears in the reviewed commits or PR body. The three commits have no commit trailers, including no Co-Authored-By trailer. The authored PR description contains no “Genera…
Full details: Linked Issues check
Explanation
Issue [#18385] requires the same credential rules for preset collection and item routes. GET, PUT, and DELETE now use checkPlaygroundPresetAuth. The helper accepts a valid API key, checks a management/dashboard session when no key is supplied, rejects an invalid supplied key before session fallback, and requires authentication when REQUIRE_API_KEY is enabled. The new integration tests cover signed-session, valid-key, missing-credential, invalid-key, and dashboard-login-disabled cases for all three item methods.
Full details: Out of Scope Changes check
Explanation
The changes stay within issue [#18385]. They extract the shared preset authentication guard, apply it to the collection and item routes, and add regression tests for the required credential cases. No unrelated product behavior or files are changed.
Full details: Tests For Production Code
Explanation
PASS: The PR changes production code under src/ in the preset routes and adds src/lib/api/playgroundPresetAuth.ts. It also adds tests/integration/presets-item-dashboard-auth.test.ts, which covers the changed GET, PUT, and DELETE routes with dashboard sessions, valid keys, missing credentials, invalid keys, and disabled-login enforcement.
Full details: No Ai Attribution
Explanation
No prohibited AI attribution appears in the reviewed commits or PR body. The three commits have no commit trailers, including no Co-Authored-By trailer. The authored PR description contains no “Generated with <AI tool>” footer.
Full details: Description check
Explanation
The description covers the problem, the change, and the CI diagnosis, and it links the issue (Fixes #18385). It does not follow the repository template. The template sections Related Issues (only the closing reference is given), Target Branch checklist, Validation checklist, Tests Added Or Updated, Coverage Notes, and Reviewer Notes are missing or only partly filled. The Summary heading is replaced by Problem and Change. Coverage for src/ changes is described only in general terms.
Resolution
Restructure the description to match the template headings: Summary, Related Issues, Target Branch, Validation, Tests Added Or Updated, Coverage Notes, and Reviewer Notes. Under Target Branch, confirm the base is the active release/v3.8.x branch and complete the checklist. Under Validation, select the change type and tick the completed checks, including npm run lint. Under Tests Added Or Updated, list tests/integration/presets-item-dashboard-auth.test.ts and any other changed test files. Under Coverage Notes, explain which tests cover the new src/lib/api/playgroundPresetAuth.ts helper and the two route changes. Under Reviewer Notes, call out the REQUIRE_API_KEY and dashboard-login-disabled behavior as the risky area.
Full details: Docstring Coverage
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 4 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
With
REQUIRE_API_KEY=true, a signed-in Playground user can list and create presets, butGET,PUT, andDELETE /api/playground/presets/{id}reject the same dashboard session with401 Authentication required. The collection and item routes apply different credential rules (see #18385).Change
GET/PUT/DELETEwith real signed-session, valid-key, missing-credential, invalid-key, and login-disabled cases.Fixes #18385
CI diagnosis (head
8c4513f, basef1a7e12)The PR is not CI-green. The completed Oct 27 attempt (CI, Quality, API typecheck) had 13 failed checks, 39 passed, 5 skipped; newer attempts on the same head are in progress, so these counts are historical, not a live status. The CI checkout is the synthetic merge of this head into
f1a7e12; the diff changes only the two preset routes, their new auth helper, and the new integration test. Both integration-test shards passed in that attempt, and the new four-case regression passes locally.Reproduced on the exact base, independent of the preset-auth diff:
Fast Quality Gates:check:build-scopecounts 12,160 files against a 12,000 limit; the same tracked-file calculation atf1a7e12is already 12,158, before this PR's two TypeScript files. Itsopen-sse-typecheckfailures (TS2345inchatCore.tsandsseParser/geminiResponse.ts) also appear inAPI Route Typecheck; neither source file is changed here.Lint:check-test-tla-orderfinds top-levelawaitafter tests in three unchanged unit-test files.i18n Glossaryfindsusage.quotaAuthRequiredHintusing提供商instead of提供者in unchangedzh-CN.json; the offending value is present onf1a7e12(as is the corresponding供應商inzh-TW.json).f1a7e12and fail identically there (same six tests, same assertion messages); thefile://${process.argv[1]}entrypoint flagged by fast-path is likewise present on the exact base. The overlapping failures in regular and fast-path shards corroborate these signatures.Gate / CIandGate / Qualitysummarize failed child jobs; they are not separate preset-auth failures.Full-suite Vitest failure not reproduced in isolation: CI reports 14 failures across six unchanged UI/i18n test files, including 5-second timeouts. Running those exact six files separately with two workers yields 64/64 passing on
f1a7e12and 64/64 passing on the PR head. The full CI run on the exact base is unavailable, so the full-suite failure (possibly load/order-sensitive) cannot conclusively be attributed to upstream or this PR. None of these tests exercise the preset routes/helper.