feat: runtime hardening, delivery seam, i18n and logout fixes - #6284
feat: runtime hardening, delivery seam, i18n and logout fixes#6284xvyimu wants to merge 38 commits into
Conversation
- decreaseTokenQuota skips remain floor for unlimited_quota tokens - batch quota debit guards user quota>=0 and negative token delta - billing session refresh and relay origin coverage - remove obsolete pre_consume_quota.go (functions unreferenced)
- session refresh reloads role/status/group from DB with nil guards - TokenAuthReadOnly rejects disabled/expired tokens - request trace middleware and coverage
- adaptive metrics record with concurrency guard and half-open recovery - channel circuit breaker, metrics and score modules - affinity usage settings and empty-group guards
- perf metric model with model_name equality index - /api/perf-metrics/* behind UserAuth() (401 for unauthenticated) - normalize utility with test coverage
- consolidate env var reads into common/env.go - init sequence cleanup in common/init.go - context key constants and API router alignment - SESSION_SECURE default true (overridden by start script)
- Markdown sanitize with allowlist (no svg/data/javascript/iframe-same-origin) - Auth redirect safeRedirect to prevent open redirect - OTP flow component hydration and route fixes - lib/api.ts custom fetch with session verification - About page iframe sandbox removal
- Pricing columns memoization and per-page perf badge map - Filters<->URL debounced sync - Bulk actions skip-confirm dialog - Channel mutate drawer split (skip-auto-test field extraction) - model-name/mock-badge helpers and filters test
- Creem/Waffo payment hooks with proper type safety - Payment lib extraction from components - All locale files (en/zh/fr/ru/ja/vi) populated for pricing, wallet, billing - Clean up dead locale keys
- log model cleanup - xAI relay text coverage and behavioral test - theme setting default fallback to 'default'
Rebase onto upstream renamed filterChannelsByRequestPath to filterChannelsByRequestPathAndModel; keep adaptive candidate selection aligned with Advanced Custom route matching.
Add restrictive CORS, bounded outbound HTTP clients, immutable asset caching, cursor log pagination, trace indexing, runtime plane separation, Prometheus/RUM observability, reproducible deployment defaults, and CI quality gates. Includes regression tests, bundle budget enforcement, operational runbooks, and the mobile setup layout fix discovered during browser validation.
Keep the monorepo and embedded dual-theme binary as the default path,
while adding an opt-in pure backend build (-tags frontend_external) and
FRONTEND_MODE={auto,embedded,redirect,disabled}.
Ship Dockerfile.backend plus a same-origin Nginx frontend image under
deploy/separated, extend the quality gate, and document the decision in
ADR 0001. Local Go router tests and both build modes passed; Docker image
builds were not run on this host.
nginx -t failed in CI because a static upstream block resolved backend:3000 at config load time inside a single-container job. Use variable proxy_pass with a configurable resolver (Docker DNS by default) and point the CI syntax check at 127.0.0.1:3000.
Add FRONTEND_MODE docs to .env.example, Makefile targets for pure backend and separated images, and portable smoke scripts. Resolve the frontend nginx base digest at quality-image build time via NGINX_IMAGE build-arg. Cover disabled 404 and master auto-mode asset requirements with tests.
Switch Dockerfile.dev to -tags frontend_external and FRONTEND_MODE=disabled so docker-compose.dev no longer needs placeholder web dist. Default the frontend Nginx base image ARG to the CI-pinned digest, and add scripts/local-ops.ps1 for Windows check/backend/inventory helpers.
Use zhCN/zh-CN as the interface and backend i18n fallback so local and China-facing deployments open in Chinese when no stored preference or Accept-Language match is available. Classic theme already defaulted to zh-CN.
fallbackLng alone never wins when navigator or a cached i18nextLng=en is present. Detect only localStorage, one-time migrate historical en caches to zhCN/zh-CN, and set default HTML lang to zh-CN. Classic theme gets the same migration.
Locale JSON stores UI strings under a translation object plus a few top-level keys. The custom backend passed the whole file into the defaultNS, so t(Home) never resolved and Chinese looked identical to English keys. Flatten translation before handing resources to i18next.
Unregistered /metrics and other relay/ops prefixes were served as the embedded index.html with HTTP 200, which breaks monitoring semantics and hides missing routes. Treat known backend prefixes as non-SPA and return API 404 JSON instead, with regression tests.
Record the readonly production cookie/HTTPS checklist, GitHub OAuth callback domain audit (including localhost server_address/passkey drift), and a no-code Cloudflare HSTS enablement runbook after API automation was blocked by token scope and dashboard challenges.
Add SecurityHeaders middleware that sets Strict-Transport-Security when TLS is present or X-Forwarded-Proto is https (Cloudflare Tunnel). Local HTTP keeps no HSTS. Also sets nosniff, Referrer-Policy, and SAMEORIGIN frame options.
Note that options were updated in the live SQLite database and HSTS is served by the app; GitHub OAuth callback still needs a manual Developer Settings change.
User confirmed Authorization callback URL is https://incc.qzz.io/oauth/github. Homepage URL still points at localhost and should be updated separately.
Homepage URL for the newapi OAuth App was changed from localhost to https://incc.qzz.io/ via the GitHub settings UI; callback remains correct.
Logout now expires the session cookie with MaxAge=-1 instead of leaving an empty Secure cookie. The UI clears auth/session-verified state and navigates to /sign-in instead of reloading the authenticated route, which previously caused getSelf 401 / Session expired loops.
Add a signing-out guard so concurrent 401s from in-flight queries do not toast Session expired or race navigation. Clear GET dedupe cache, expire the session cookie server-side, and hard-replace to /sign-in after local auth reset.
Capture the 2026-07-18 production health/logout regression results and disk cleanup notes, and refresh the operator-facing project overview.
Harden runtime, release traceability, and legacy frontend rendering
WalkthroughThis PR introduces a frontend/backend delivery seam (build tags, run modes, separated Nginx deployment), adaptive channel balancing with circuit breakers and metrics, security/session middleware hardening, channel-testing and perf-metrics/logging improvements with trace IDs, and matching web app features: skip-auto-test controls, live-metrics pricing, cursor-paginated usage logs, safe-redirect auth flow, and new HTML sanitizers for both frontends. ChangesDelivery Seam, Build & CI
Estimated code review effort: 5 (Critical) | ~180 minutes Reliability: Outbound HTTP, Adaptive Balancing & Circuit Breaker
Estimated code review effort: 5 (Critical) | ~150 minutes Security Middleware, Auth Session & System Tasks
Estimated code review effort: 4 (Complex) | ~90 minutes Channel Testing, Perf Metrics, Logging & Trace
Estimated code review effort: 5 (Critical) | ~150 minutes Web Default App: Channels, Pricing, Usage Logs, Auth, Sanitization
Estimated code review effort: 4 (Complex) | ~90 minutes Web Classic App & Shared Web Tooling
Estimated code review effort: 2 (Simple) | ~20 minutes Operations Documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant Relay as controller.Relay
participant Adaptive as service.AdaptiveSelectChannel
participant Score as service.ScoreCandidates
participant Circuit as service.ChannelCircuitBreaker
participant Metrics as service.ChannelMetrics
Relay->>Adaptive: select channel(group, model)
Adaptive->>Score: ScoreCandidates(candidates)
Score->>Circuit: IsCircuitOpen / AcquireCircuitPermit
Score->>Metrics: GetMetrics(channelID)
Adaptive-->>Relay: selected channel + permit
Relay->>Adaptive: RecordAdaptiveResult(status, latency)
Adaptive->>Circuit: RecordCircuitSuccess/Failure
Possibly related issues
Possibly related PRs
Suggested labels Suggested reviewers Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
web/default/src/features/channels/components/data-table-bulk-actions.tsx (1)
81-86: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestore the sensitive-write guard for bulk deletion.
Users lacking
SENSITIVE_WRITEcan now open and execute bulk deletion, while single-row deletion remains permission-gated. Keep both paths consistent and prevent the request client-side.Also applies to: 226-237, 305-307
🤖 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 `@web/default/src/features/channels/components/data-table-bulk-actions.tsx` around lines 81 - 86, Restore the SENSITIVE_WRITE permission guard for the bulk deletion flow around handleDeleteAll and its related bulk-delete UI/actions. Prevent users without this permission from opening or executing bulk deletion, matching the existing single-row deletion behavior and preserving the current authorized flow.web/default/src/features/pricing/components/pricing-toolbar.tsx (1)
208-235: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExpose the metrics-source filter on mobile.
This control is inside
hidden ... sm:flex, and the mobile filter sheet has no equivalent. Users below thesmbreakpoint cannot enable or disable live-only metrics.🤖 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 `@web/default/src/features/pricing/components/pricing-toolbar.tsx` around lines 208 - 235, Make the metrics-source SegmentedControl using liveMetricsOnly and handleLiveMetricsOnlyChange available below the sm breakpoint instead of restricting it to the hidden sm:flex toolbar group. Add or reuse an equivalent control in the mobile filter sheet so mobile users can switch between all metrics and live-only metrics, while preserving the existing desktop behavior and labels.web/default/src/features/pricing/components/pricing-columns.tsx (1)
53-72: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore selected-group pricing across both views.
The public
selectedGroupinput no longer reaches card or table price calculations, causing displayed prices to ignore the group selected by the user.
web/default/src/features/pricing/components/pricing-columns.tsx#L53-L72: restoreselectedGroupinPricingColumnsOptionsand pass it to every ratio/formatting path.web/default/src/features/pricing/components/pricing-table.tsx#L33-L57: consumeprops.selectedGroupand forward it tousePricingColumns.web/default/src/features/pricing/components/model-card-grid.tsx#L29-L39: forwardprops.selectedGroupthrough the card pricing path.🤖 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 `@web/default/src/features/pricing/components/pricing-columns.tsx` around lines 53 - 72, The selectedGroup value is being dropped before price calculations, so both pricing views ignore the user’s selected group. In web/default/src/features/pricing/components/pricing-columns.tsx lines 53-72, restore selectedGroup in PricingColumnsOptions and pass it through every ratio and formatting path; in web/default/src/features/pricing/components/pricing-table.tsx lines 33-57, forward props.selectedGroup to usePricingColumns; and in web/default/src/features/pricing/components/model-card-grid.tsx lines 29-39, forward props.selectedGroup through the card pricing path.web/default/src/features/pricing/lib/filters.ts (1)
218-224: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDeduplicating before filtering can hide the only matching pricing record. Preserve filter semantics by filtering the complete model set first, then deduplicating the matches.
web/default/src/features/pricing/lib/filters.ts#L218-L224: movededupePricingModelsafter search/vendor/group/quota/endpoint/tag filtering and before sorting.web/default/src/features/pricing/lib/filters.test.ts#L70-L88: replace the empty query with a case where duplicate records differ by path or filter metadata and verify the matching record survives.As per coding guidelines, tests must protect real behavior and explicit regressions.
🤖 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 `@web/default/src/features/pricing/lib/filters.ts` around lines 218 - 224, Move dedupePricingModels in the pricing filter pipeline so filterBySearch, filterByVendor, filterByGroup, filterByQuotaType, filterByEndpointType, and filterByTag run on the complete model set before deduplication; keep deduplication before sorting. In web/default/src/features/pricing/lib/filters.test.ts lines 70-88, replace the empty-query case with duplicate records that differ in path or filter metadata and assert the matching record remains after filtering.Source: Coding guidelines
web/default/src/features/pricing/hooks/use-filters.ts (1)
58-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSync local filters with router search changes.
useStateonly seeds fromsearchon mount, so back/forward navigation can leavefilterStatestale and the debounce will write the old filters back into the URL. Add router-to-state syncing and a test for history navigation.🤖 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 `@web/default/src/features/pricing/hooks/use-filters.ts` around lines 58 - 79, Update the filter state initialization flow in the hook containing filterState and syncTimer so changes to the router search, including browser back/forward navigation, replace the corresponding local filterState values instead of retaining stale state. Reuse the existing search-to-filter mapping, preserve the liveMetricsOnly default behavior, and add a test covering history navigation and preventing stale filters from being written back to the URL.service/http_client.go (1)
81-108: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not fail open when the shared client is uninitialized.
Both nil branches return a bare
http.Client, dropping the shared transport and redirect validation. Line 103 therefore makes an explicitly SSRF-protected request silently use the default unprotected client. Initialize the corresponding protected/general client or return an error instead.🤖 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 `@service/http_client.go` around lines 81 - 108, The nil fallback in GetSSRFProtectedHTTPClientWithTimeout must not return a bare default http.Client, since that bypasses the shared transport and SSRF protections. Ensure the protected client is initialized or otherwise reuse the corresponding initialized client before applying the timeout; if initialization cannot be guaranteed, change the API to return an error rather than failing open.relay/channel/ali/image.go (1)
241-247: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winExit early if the request context is canceled.
The polling loop ignores context cancellation from
updateTaskand continues totime.Sleepand retry. If the client disconnects,updateTaskwill fail immediately with a context error, but this loop will continue to block and retry, causing a goroutine leak for up to 200 seconds.Check if the context is canceled and return early to immediately clean up resources when the client disconnects.
🛠️ Proposed fix
rsp, err, body := updateTask(c.Request.Context(), info, taskID) responseBody = body if err != nil { logger.LogWarn(c, "asyncTaskWait UpdateTask err: "+err.Error()) + if c.Request.Context().Err() != nil { + return nil, nil, c.Request.Context().Err() + } time.Sleep(time.Duration(waitSeconds) * time.Second) continue }🤖 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 `@relay/channel/ali/image.go` around lines 241 - 247, Update the polling loop around updateTask in the async task wait flow to detect cancellation from c.Request.Context() before sleeping and retrying. When the context is canceled, return immediately; preserve the existing warning, delay, and retry behavior for other updateTask errors.web/default/src/routes/(auth)/sign-in.tsx (1)
25-31: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winUse a validation function for
validateSearch.TanStack Router's
validateSearchexpects a validation function, not a direct Zod schema object. Passing the schema object directly will cause a TypeScript error and a runtimeTypeErrorwhen the router attempts to call it as a function.
web/default/src/routes/(auth)/sign-in.tsx#L25-L31: Wrap the schema using a validator adapter or pass a function (e.g.,validateSearch: (search) => searchSchema.parse(search)).web/default/src/routes/(auth)/otp.tsx#L24-L29: Apply the exact same correction tootpSearchSchema.🤖 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 `@web/default/src/routes/`(auth)/sign-in.tsx around lines 25 - 31, The validateSearch configuration in sign-in.tsx must use a callable validator instead of passing searchSchema directly; wrap it with the appropriate adapter or a function that parses the search input. Apply the same correction to otpSearchSchema in web/default/src/routes/(auth)/otp.tsx, while preserving each route’s existing schema validation behavior.
🟠 Major comments (33)
.env.example-86-90 (1)
86-90: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not recommend trusting the entire Docker private range by default.
172.16.0.0/12may include unrelated or compromised containers that can forgeX-Forwarded-ForandX-Real-IP. Document using the exact compose network subnet or reverse-proxy source range, and require that the backend is unreachable from untrusted containers.🤖 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 @.env.example around lines 86 - 90, Update the TRUSTED_PROXY_CIDRS documentation to avoid recommending the entire 172.16.0.0/12 Docker range; instruct users to specify the exact Compose network subnet or reverse-proxy source range, and state that the backend must be unreachable from untrusted containers.service/channel_adaptive.go-49-62 (1)
49-62: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPropagate the resolved auto group back to the selector.
getCandidateChannelscan replace"auto"with a concrete group, but only returns the channel list. The caller therefore scores metrics/affinity under"auto"and may return"auto"asselectGroupeven though the candidates came from another group. Return the resolved group alongside the channels and use it for scoring, filtering, selection metadata, and the result.This also breaks the downstream expectation that selection and result recording use the same resolved metric group, as shown by
controller/relay.goLines 231-255.Also applies to: 181-211
🤖 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 `@service/channel_adaptive.go` around lines 49 - 62, Update getCandidateChannels to return the concrete resolved group together with the channel list. In its caller, capture that group and use it instead of the requested “auto” value for ScoreCandidates, affinity/metric filtering, selection metadata, and the returned selectGroup/result. Ensure downstream selection and result recording consistently use the same resolved metric group.service/channel_affinity.go-212-218 (1)
212-218: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the LRU index when affinity-key deletion fails.
DeleteManyerrors are only logged, after which the entire index is deleted. Any surviving Redis keys then become permanently unindexed and no longer count towardMaxEntries. Only delete the index after all cache entries were successfully removed.🤖 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 `@service/channel_affinity.go` around lines 212 - 218, Update DeleteMany so the Redis LRU index is cleared only after every affinity-key deletion succeeds; if any deletion returns an error, log it and retain the existing index. Remove or relocate the unconditional Del call identified by channelAffinityRedisLRUIndex while preserving the current timeout and error-reporting behavior.service/channel_affinity.go-795-836 (1)
795-836: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftKeep the
HybridCachememory tier coherent with Redis writes and evictions.The Lua script updates Redis directly, bypassing the cache’s local memory layer. After switching channels, an instance can continue serving the previous channel until its local TTL expires; entries evicted from Redis can likewise remain locally available. Route indexed writes through a cache API that updates memory and Redis, or add explicit local update/invalidation for the written key and returned eviction victims.
🤖 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 `@service/channel_affinity.go` around lines 795 - 836, Update setChannelAffinityWithLimit to keep HybridCache memory synchronized with the Redis Lua operation: write the channel value through a cache API that updates both tiers, and remove or invalidate every local entry corresponding to eviction victims returned by the Redis operation. Preserve the existing TTL and maxEntries behavior while ensuring switched keys cannot serve stale local values.setting/operation_setting/channel_affinity_setting.go-119-135 (1)
119-135: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not let the broad trace rule shadow specialized affinity rules.
This rule is evaluated before the Codex and Claude rules. Once it extracts
affinity_trace_id, selection returns from that rule even on a cache miss, so specialized parameter templates and retry behavior are skipped. This also affects session-header fallbacks populated by trace middleware. Place the broad rule after specialized rules or support intentional fallthrough/composition.🤖 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 `@setting/operation_setting/channel_affinity_setting.go` around lines 119 - 135, Reorder the “axonhub trace sticky” rule in the channel-affinity configuration so it is evaluated after the specialized Codex and Claude rules. Preserve its existing trace and session-header key sources and behavior, ensuring specialized parameter templates and retry handling take precedence before this broad fallback rule.web/default/src/lib/sanitize-html.test.ts-19-35 (1)
19-35: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse Vitest for unit tests.
As per coding guidelines, unit tests for utility functions in the frontend (
web/default/**/*.test.ts) must be written with Vitest rather than the Node.js built-in test runner.♻️ Proposed refactor
-import assert from 'node:assert/strict' -import { describe, it } from 'node:test' +import { describe, expect, it } from 'vitest' import { normalizeAnchorRel } from './sanitize-html' describe('normalizeAnchorRel', () => { it('forces noopener and noreferrer for blank targets', () => { - assert.equal( + expect( normalizeAnchorRel('_blank', 'opener external'), - 'external noopener noreferrer' - ) + ).toBe('external noopener noreferrer') }) it('leaves non-blank links without forced rel tokens', () => { - assert.equal(normalizeAnchorRel('_self', 'external'), 'external') + expect(normalizeAnchorRel('_self', 'external')).toBe('external') }) })🤖 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 `@web/default/src/lib/sanitize-html.test.ts` around lines 19 - 35, Update the normalizeAnchorRel tests in describe('normalizeAnchorRel') to use Vitest imports and APIs instead of node:test and node:assert/strict, preserving the existing test cases and assertions.Source: Coding guidelines
web/default/src/lib/sanitize-html.ts-245-253 (1)
245-253: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFail closed when DOM sanitization is unavailable.
The regex fallback still permits dangerous constructs such as
iframe[srcdoc], objects, forms, and malformed markup. Returning this result as sanitized HTML can reintroduce XSS in SSR or prerendering environments. Return an empty string, or use an SSR-capable sanitizer.Fail-closed fix
if (!dirty) return '' if (typeof window === 'undefined' || typeof DOMParser === 'undefined') { - return dirty - .replaceAll(/<script[\s\S]*?>[\s\S]*?<\/script>/gi, '') - .replaceAll(/<svg[\s\S]*?>[\s\S]*?<\/svg>/gi, '') - .replaceAll(/on\w+\s*=\s*("[^"]*"|'[^']*'|[^\s>]+)/gi, '') - .replaceAll(/javascript\s*:/gi, '') + return '' }🤖 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 `@web/default/src/lib/sanitize-html.ts` around lines 245 - 253, Update the unavailable-DOM branch in sanitizeHtml to fail closed by returning an empty string instead of applying the regex fallback and returning potentially unsafe HTML. Preserve the existing DOM-based sanitization behavior when DOMParser is available.web/default/src/lib/sanitize-html.ts-194-225 (1)
194-225: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict anchor targets to prevent opener access.
Line 204 is unreachable because
targetis not a URL attribute. The generic allowlist instead preserves arbitrary named targets, and an suppliedrelcan replace the default protections. Only retain_blank, then letnormalizeAnchorReladdnoopener noreferrer.Proposed fix
if (name === 'srcdoc' || name === 'srcset') continue if (name === 'style') continue + if (tag === 'a' && name === 'target') { + if (value.toLowerCase() === '_blank') { + clean.setAttribute('target', '_blank') + } + continue + } if (URL_ATTRS.has(name) || name === 'href' || name === 'src') { if (!isSafeUrl(value)) continue clean.setAttribute(attr.name, value) - if (tag === 'a' && name === 'target') { - clean.setAttribute('target', '_blank') - } continue } ... - name === 'target' || name === 'rel' ||🤖 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 `@web/default/src/lib/sanitize-html.ts` around lines 194 - 225, Update the anchor-attribute handling in the sanitizer so target values are preserved only when they equal "_blank"; remove target-specific handling from the URL-attribute branch and prevent the generic allowlist from retaining arbitrary target values. Reuse normalizeAnchorRel to enforce noopener noreferrer after sanitization, including when an input rel attribute is supplied.relay/channel/xai/text_test.go-9-57 (1)
9-57: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
testify/requirefor these assertions.Replace the manual conditionals with
require.Equal; consider a table test to remove the repeated setup.As per coding guidelines, new Go backend tests must use
testify/requirefor setup and fatal assertions.🤖 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 `@relay/channel/xai/text_test.go` around lines 9 - 57, The tests for mergeXAIStreamUsage use manual fatal conditionals instead of testify/require assertions. Update TestMergeXAIStreamUsagePreservesCachedTokens, TestMergeXAIStreamUsageFallbackPromptCacheHit, and TestMergeXAIStreamUsageFallbackInputDetails to use require.Equal for expected values, and use require for any setup assertions; optionally consolidate the repeated cases into a table-driven test while preserving their existing coverage.Source: Coding guidelines
relay/channel/xai/text.go-53-57 (1)
53-57: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClamp derived completion tokens to zero.
When
TotalTokens < PromptTokens, this stores a negative completion count that can flow into quota settlement.Proposed fix
if src.CompletionTokens > 0 { dst.CompletionTokens = src.CompletionTokens } else if dst.TotalTokens > 0 && dst.PromptTokens > 0 { - dst.CompletionTokens = dst.TotalTokens - dst.PromptTokens + completionTokens := dst.TotalTokens - dst.PromptTokens + if completionTokens < 0 { + completionTokens = 0 + } + dst.CompletionTokens = completionTokens }As per coding guidelines, billing and quota code must never produce negative charges or overflow-derived credits.
🤖 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 `@relay/channel/xai/text.go` around lines 53 - 57, Update the derived CompletionTokens calculation in the token conversion logic so TotalTokens minus PromptTokens is clamped to zero when negative. Preserve the explicitly provided src.CompletionTokens path and ensure quota settlement never receives a negative completion count.Source: Coding guidelines
controller/relay.go-215-254 (1)
215-254: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRelease the adaptive permit when a relay helper panics.
A panic executes
DecChannelConcurrency, but then unwinds pastRecordAdaptiveResult. This can strand a half-open circuit permit and prevent future channel selection. Release it before re-panicking.Proposed fix
func() { - defer service.DecChannelConcurrency(channel.Id) + defer func() { + service.DecChannelConcurrency(channel.Id) + if recovered := recover(); recovered != nil { + service.ReleaseAdaptiveCircuitPermit(c, channel.Id) + panic(recovered) + } + }() switch relayFormat {🤖 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 `@controller/relay.go` around lines 215 - 254, Update the relay helper wrapper around the switch in controller flow to recover panics long enough to call the adaptive-result release logic, including a panic-safe RecordAdaptiveResult invocation for the affected channel and metric group, then re-panic so existing CustomRecovery behavior remains unchanged. Ensure normal execution records the result only once and preserves the existing status, error, latency, and group selection behavior.controller/health_test.go-11-42 (1)
11-42: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winTest the public readiness endpoint contract, not only its helper.
Add handler-level tests for healthy
200, database/Redis503responses, and request timeout behavior. The current tests could pass while/readyzreturns the wrong status or payload.As per coding guidelines, backend tests must protect real behavior and API contracts rather than implementation details.
<coding_guidelines>🤖 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 `@controller/health_test.go` around lines 11 - 42, Add handler-level tests for the public /readyz endpoint, covering a healthy 200 response, database and Redis failures returning 503, and request timeout behavior. Exercise the actual readiness handler and assert HTTP status and response payload rather than testing only the checkReadiness helper.Source: Coding guidelines
service/system_task.go-198-207 (1)
198-207: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftPropagate cancellation into the database operations.
The context currently gates loop iterations but cannot interrupt stale-lock cleanup, pending-task lookup, or task claims. A stalled database call can therefore keep
WaitForSystemTasksblocked past graceful shutdown.Add context-aware model variants backed by GORM
WithContext(ctx)and use them throughout both passes.Also applies to: 303-309, 321-323
🤖 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 `@service/system_task.go` around lines 198 - 207, Propagate ctx through runSystemTaskSchedulerLoop and both scheduler passes so every stale-lock cleanup, pending-task lookup, and task-claim database operation can be canceled during shutdown. Add context-aware model variants using GORM WithContext(ctx), then update the scheduler calls—including ExpireStaleSystemTaskLocks, pending-task retrieval, and claim operations—to use those variants while preserving existing behavior.service/system_task_test.go-127-135 (1)
127-135: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
requirefor the timeout branches
Replace thet.Fatal/t.Fatalfcalls atservice/system_task_test.go:134, 161, and 169withtestify/requireso the test follows the Go backend test rule for fatal assertions.🤖 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 `@service/system_task_test.go` around lines 127 - 135, Replace the timeout-branch t.Fatalf calls in the system task tests, including the branches around worker/scheduler shutdown and the corresponding checks near the other reported locations, with the appropriate testify/require fatal assertion. Preserve each existing timeout message and test behavior, and ensure require is imported or reused through the file’s existing test assertion setup.Source: Coding guidelines
setting/ratio_setting/group_ratio_empty_test.go-3-84 (1)
3-84: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMigrate the new backend tests to the required Testify assertions.
setting/ratio_setting/group_ratio_empty_test.go#L3-L84: import Testify and userequire/assert; also restore mutated package settings witht.Cleanup.setting/user_usable_group_empty_test.go#L3-L16: replace manual fatal assertions withrequire.NoErrorandrequire.Contains.As per coding guidelines, new or substantially rewritten Go backend tests must use
testify/requireandtestify/assert.🤖 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 `@setting/ratio_setting/group_ratio_empty_test.go` around lines 3 - 84, Update setting/ratio_setting/group_ratio_empty_test.go lines 3-84 to import testify/assert and testify/require, replace manual fatal checks with the appropriate assertions, and add t.Cleanup to restore mutated package settings after each test. Update setting/user_usable_group_empty_test.go lines 3-16 to replace manual fatal assertions with require.NoError and require.Contains, preserving each test’s existing behavior.Source: Coding guidelines
model/token.go-430-446 (1)
430-446: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winZero-value quota debits can be reported as insufficient quota. Both paths infer failure solely from
RowsAffected == 0, but supported databases differ in how no-op updates are counted.
model/token.go#L430-L446: returnnilbefore the update whenquota == 0.model/user.go#L1123-L1132: returnnilbefore the update whenquota == 0.🤖 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 `@model/token.go` around lines 430 - 446, Zero-value quota updates can incorrectly return insufficient-quota errors when databases report no affected rows for no-op updates. In model/token.go (lines 430-446), update decreaseTokenQuota to return nil immediately when quota == 0 before executing the database update; apply the same early-return behavior in model/user.go (lines 1123-1132) within the corresponding quota-decrease function.web/default/src/routes/pricing/index.tsx-52-52 (1)
52-52: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
location.searchStrhere.
location.searchis the parsed search object in TanStack Router, so concatenating it into the redirect path turns the return URL into[object Object]and drops the query string.Proposed fix
- search: { redirect: safeRedirect(location.pathname + location.search, '/pricing/') }, + search: { + redirect: safeRedirect( + location.pathname + location.searchStr, + '/pricing/' + ), + },🤖 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 `@web/default/src/routes/pricing/index.tsx` at line 52, Update the redirect construction in the pricing route to use location.searchStr instead of location.search, preserving the raw query string when passing the URL to safeRedirect.main.go-44-49 (1)
44-49: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLoad
.envbefore parsing runtime configuration.
RUN_MODE,APP_PLANE, andNODE_TYPEare read beforeInitResources()loads.env. File-based deployments therefore silently use the defaults, potentially starting unintended planes and workers.Move environment loading and
common.InitEnv()ahead ofparseRuntimeConfig, or split them into an explicit bootstrap step.🤖 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 `@main.go` around lines 44 - 49, Move the environment-loading bootstrap, including common.InitEnv(), before the parseRuntimeConfig call in main so RUN_MODE, APP_PLANE, and NODE_TYPE are populated from .env before parsing. Preserve the existing invalid-configuration handling and ensure InitResources runs only after runtime configuration is resolved.router/main.go-64-66 (1)
64-66: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not discard router configuration failures.
SetRoutersilently ignores invalidFRONTEND_MODE, invalid redirect URLs, and missing embedded assets, leaving a partially configured server. Return the error and update callers to handle it.-func SetRouter(router *gin.Engine, assets ThemeAssets) { - _ = SetRouterForPlane(router, assets, PlaneAll) +func SetRouter(router *gin.Engine, assets ThemeAssets) error { + return SetRouterForPlane(router, assets, PlaneAll) }🤖 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 `@router/main.go` around lines 64 - 66, Update SetRouter to return the error from SetRouterForPlane instead of discarding it, then update every caller of SetRouter to handle or propagate the returned error so invalid FRONTEND_MODE, redirect URLs, and missing assets prevent partially configured startup.router/main.go-138-141 (1)
138-141: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep backend and operational paths out of frontend redirects.
Unlike embedded mode, redirect mode sends unmatched
/metrics,/api/..., and relay paths to the frontend with a cacheable 301. ApplyisNonSPARequestPathbefore redirecting so disabled or unknown backend endpoints retain API-style 404 responses.Proposed fix
router.NoRoute(func(c *gin.Context) { c.Set(middleware.RouteTagKey, "web") + if isNonSPARequestPath(c.Request.RequestURI) { + controller.RelayNotFound(c) + return + } c.Redirect(http.StatusMovedPermanently, fmt.Sprintf("%s%s", frontendBaseURL, c.Request.RequestURI)) })🤖 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 `@router/main.go` around lines 138 - 141, Update the router.NoRoute handler to check isNonSPARequestPath before issuing the frontend redirect. Return the existing API-style 404 response for non-SPA paths such as metrics, API, and relay endpoints, while preserving the current permanent frontend redirect for SPA routes.main.go-299-305 (1)
299-305: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound request-body reads
ReadHeaderTimeoutonly limits header parsing; a client can still drip the request body indefinitely after sending valid headers. Add a configurable positiveReadTimeouthere and extendmain_server_test.goto assert the default and override values.🤖 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 `@main.go` around lines 299 - 305, Add a configurable positive ReadTimeout in newHTTPServer using the established GetEnvOrDefault pattern and a suitable HTTP_READ_TIMEOUT_SECONDS environment key, preserving the existing timeout configuration. Extend main_server_test.go to verify both the default ReadTimeout and its environment-variable override.Source: Linters/SAST tools
model/log.go-695-705 (1)
695-705: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Limitdoes not cap the rows processed byCOUNT(*).
tx.Limit(logSearchCountLimit).Count(&total)still counts the full user history because the limit applies to the aggregate result row. Count a limited-ID subquery instead; otherwise the advertised cap neither bounds cost nor total.🤖 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 `@model/log.go` around lines 695 - 705, Update GetUserLogs so the count is computed from a subquery selecting at most logSearchCountLimit matching log IDs, rather than applying Limit directly to Count. Preserve the existing error handling and return the capped count while keeping the subsequent log retrieval behavior unchanged.model/log.go-983-1002 (1)
983-1002: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude legacy rows when indexed rows also exist.
model/main.goLines 432-435 leaves historical rows withtrace_id = '', but this function returns immediately when it finds any newer indexed row. A trace spanning the migration therefore returns only its new logs. Combine the indexed and legacy predicates—excluding indexed rows from the fallback—or merge both result sets before applying the limit.🤖 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 `@model/log.go` around lines 983 - 1002, Update the log retrieval flow after the indexed query so legacy rows are still included when indexed rows exist, while excluding rows already matched by the trace_id predicate to avoid duplicates. Merge the indexed and legacy results, then apply the requested ordering and limit to the combined set rather than returning early from the len(logs) check.model/log.go-5-7 (1)
5-7: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse the shared JSON wrappers for log cursor encoding. Replace the direct
json.Marshal/json.Unmarshalcalls inmodel/log.go:136-157withcommon.Marshal/common.Unmarshalto match the project JSON policy.🤖 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 `@model/log.go` around lines 5 - 7, Update the log cursor encoding and decoding flow in model/log.go to use the shared common.Marshal and common.Unmarshal wrappers instead of direct json.Marshal and json.Unmarshal calls. Remove the now-unused encoding/json import while preserving the existing cursor behavior and error handling.Source: Coding guidelines
model/log.go-721-780 (1)
721-780: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftUse a unique, non-empty ClickHouse cursor tie-breaker.
(created_at, request_id)is not a total order:request_idmay be empty or duplicated. An empty boundary value is encoded successfully but rejected on the next request, while duplicate tuples can cause rows to be skipped. Persist a stable unique key and include it in both ordering and cursor predicates; add empty/duplicate regression cases.🤖 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 `@model/log.go` around lines 721 - 780, Update applyLogCursor and getLogsByCursor to use a persisted, non-empty unique ClickHouse log key as the cursor tie-breaker instead of request_id; include that same key in clickHouseLogOrder and the cursor predicate so ordering is total and duplicate tuples cannot skip rows. Ensure encode/decode validation rejects missing or invalid tie-breakers, and add regression coverage for empty and duplicate cursor boundaries.service/channel_metrics.go-42-58 (1)
42-58: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftBound and expire metric buckets.
globalSnapshot.metricshas no eviction, while each distinct channel/group/model tuple permanently allocates a bucket. Arbitrary or high-cardinality model names can therefore cause sustained memory growth. Apply a maximum size and periodically evict entries usingLastSeen.🤖 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 `@service/channel_metrics.go` around lines 42 - 58, Update LocalMetricsSnapshot and ensureKey to bound globalSnapshot.metrics and periodically evict stale entries using each bucket’s LastSeen timestamp. Enforce a maximum bucket count, remove expired or least-recently-seen entries before adding new ones, and refresh LastSeen whenever an existing bucket is accessed.service/channel_metrics.go-286-309 (1)
286-309: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winRelease the snapshot lock before Redis I/O.
The deferred
RUnlockholdsglobalSnapshot.muthrough marshaling andRedisSet. A slow Redis call blocksensureKey, delaying requests that encounter a new metric tuple. Copy the rows under the locks, then unlock before serialization and publication.🤖 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 `@service/channel_metrics.go` around lines 286 - 309, Update the metrics snapshot function around globalSnapshot.mu and the row collection to release globalSnapshot.mu immediately after copying the rows, rather than deferring RUnlock through common.Marshal and RedisSet. Preserve per-metric locking while reading each metric, then perform serialization and Redis publication only after the snapshot lock is released.service/channel_circuit.go-213-224 (1)
213-224: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInvalidate the timed-out half-open permit before issuing another.
Resetting
HalfOpenInFlightwithout incrementingGenerationleaves the expired permit valid. If its late result arrives after a replacement probe starts, it can decrement the replacement’s counter and incorrectly close or reopen the circuit.Proposed fix
if cb.HalfOpenInFlight > 0 && !cb.HalfOpenSince.IsZero() && time.Since(cb.HalfOpenSince) > halfOpenProbeTimeout { + cb.Generation++ cb.HalfOpenInFlight = 0 cb.HalfOpenSince = time.Time{} }🤖 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 `@service/channel_circuit.go` around lines 213 - 224, In the half-open timeout handling before the limit check, increment cb.Generation when invalidating an expired probe, alongside resetting HalfOpenInFlight and HalfOpenSince. Ensure the newly issued permit uses the incremented generation so late results from the timed-out permit cannot affect the replacement probe.model/channel_cache.go-132-136 (1)
132-136: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFilter normalized-cache candidates using the requested model.
The normalized name is only the fallback cache key. Passing it to
SupportsPathForModelcan reject or admit an advanced-custom channel differently from the actual request; the legacy path correctly passes the original model at Line 179.Proposed fix
normalizedModel := ratio_setting.FormatMatchingModelName(modelName) -ids = filterChannelsByRequestPathAndModel(group2model2channels[group][normalizedModel], requestPath, normalizedModel) +ids = filterChannelsByRequestPathAndModel(group2model2channels[group][normalizedModel], requestPath, modelName)🤖 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 `@model/channel_cache.go` around lines 132 - 136, Update the normalized-cache fallback in the channel lookup to use normalizedModel only for selecting the cache key, while passing the original modelName to filterChannelsByRequestPathAndModel so candidate filtering evaluates the requested model consistently with the legacy path.controller/channel_auto_test_helpers_test.go-12-93 (1)
12-93: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse Testify assertions in
controller/channel_auto_test_helpers_test.go. Replace the manualt.Fatal/t.Fatalfchecks withrequirefor fatal setup failures andassertfor value comparisons to match the Go test guidelines.🤖 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 `@controller/channel_auto_test_helpers_test.go` around lines 12 - 93, Update the tests in TestDetectProbeModelKind, TestIsChatCapableProbeModel, TestPickAutoTestModel, TestShouldSkipAutoChannelTest, and TestNormalizeChannelTestEndpointInfersImage to use Testify assert/require calls instead of manual t.Fatal and t.Fatalf checks. Use assert for expected values and require only where a failed prerequisite should stop the test, preserving all existing test coverage and messages as appropriate.scripts/build-release.ps1-88-94 (1)
88-94: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate the complete frontend distributions before skipping their builds.
Checking only
index.htmlallows a partialdisttree with missing referenced JS/CSS to be embedded into a release. Require a verified build manifest or validate every local asset referenced by both index files before continuing.🤖 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 `@scripts/build-release.ps1` around lines 88 - 94, Strengthen the validation in the release-build skip branch around the indexPath loop so each frontend distribution is verified as complete, not merely present via index.html. Require the existing build manifest when available, or parse both index files and validate every locally referenced JS/CSS asset under each corresponding dist directory before continuing..github/workflows/quality.yml-21-22 (1)
21-22: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable persisted checkout credentials throughout the quality workflow.
All three jobs execute repository-controlled commands after checkout, while the GitHub token remains stored in Git configuration.
.github/workflows/quality.yml#L21-L22: addwith: persist-credentials: falseto the Go quality checkout..github/workflows/quality.yml#L77-L78: add it to the web quality checkout..github/workflows/quality.yml#L131-L132: add it to the image reproducibility checkout.🤖 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 @.github/workflows/quality.yml around lines 21 - 22, Disable persisted checkout credentials on all three actions/checkout steps in .github/workflows/quality.yml at lines 21-22, 77-78, and 131-132 by adding the persist-credentials: false option under each step’s with configuration.Source: Linters/SAST tools
Dockerfile.backend-24-39 (1)
24-39: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRun the production backend as a non-root user.
The runtime image currently starts
/new-apias root, unnecessarily increasing the impact of an application compromise. Create an unprivileged user and grant it ownership of/data.Proposed fix
FROM debian:bookworm-slim@sha256:f06537653ac770703bc45b4b113475bd402f451e85223f0f2837acbf89ab020a RUN apt-get update \ && apt-get install -y --no-install-recommends ca-certificates tzdata libasan8 wget \ && rm -rf /var/lib/apt/lists/* \ - && update-ca-certificates + && update-ca-certificates \ + && useradd --system --uid 10001 --create-home new-api \ + && mkdir -p /data \ + && chown new-api:new-api /data COPY --from=builder /build/new-api / COPY LICENSE NOTICE THIRD-PARTY-LICENSES.md /licenses/ ENV FRONTEND_MODE=disabled EXPOSE 3000 WORKDIR /data +USER new-api ENTRYPOINT ["/new-api"]🤖 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 `@Dockerfile.backend` around lines 24 - 39, Update the runtime stage around the Dockerfile’s /data setup to create an unprivileged user, grant that user ownership of /data, and configure the container to run /new-api as that user via the existing ENTRYPOINT. Preserve the current runtime image contents and exposed port.Source: Linters/SAST tools
| depends_on: | ||
| # When bun/vite proxies from another host, set CORS or use same-origin proxy only. | ||
| # - CORS_ALLOWED_ORIGINS=http://localhost:5173,http://localhost:3001 | ||
| # - TRUSTED_PROXY_CIDRS=127.0.0.1/32,::1/128 depends_on: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Restore the depends_on: key.
The depends_on: key was accidentally merged into the commented-out TRUSTED_PROXY_CIDRS line. Because environment: is a YAML list sequence (using -), the orphaned redis: and postgres: mapping keys directly below it will cause a critical YAML parsing error (e.g., did not find expected '-' indicator) that completely breaks docker compose up.
Restore depends_on: to its own line so the service dependencies are correctly parsed.
🐛 Proposed fix
- # - TRUSTED_PROXY_CIDRS=127.0.0.1/32,::1/128 depends_on:
+ # - TRUSTED_PROXY_CIDRS=127.0.0.1/32,::1/128
+ depends_on:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # - TRUSTED_PROXY_CIDRS=127.0.0.1/32,::1/128 depends_on: | |
| # - TRUSTED_PROXY_CIDRS=127.0.0.1/32,::1/128 | |
| depends_on: |
🤖 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 `@docker-compose.dev.yml` at line 42, Restore the depends_on: key as its own
YAML mapping line immediately after the environment list, separate from the
commented TRUSTED_PROXY_CIDRS entry, so the existing redis and postgres
dependency mappings parse correctly.
| if (typeof window !== 'undefined') { | ||
| window.localStorage.removeItem('user') | ||
| window.localStorage.removeItem('uid') | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Wrap localStorage operations in a try...catch block.
In environments where storage access is restricted (such as incognito mode or strict privacy settings), calling window.localStorage.removeItem throws a SecurityError. Because these calls are not wrapped in a try...catch, the error will crash auth.reset(), preventing the local authentication state from clearing and interrupting the sign-out flow.
🔒️ Proposed fix to protect storage access
if (typeof window !== 'undefined') {
- window.localStorage.removeItem('user')
- window.localStorage.removeItem('uid')
+ try {
+ window.localStorage.removeItem('user')
+ window.localStorage.removeItem('uid')
+ } catch {
+ // ignore storage errors
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof window !== 'undefined') { | |
| window.localStorage.removeItem('user') | |
| window.localStorage.removeItem('uid') | |
| } | |
| if (typeof window !== 'undefined') { | |
| try { | |
| window.localStorage.removeItem('user') | |
| window.localStorage.removeItem('uid') | |
| } catch { | |
| // ignore storage errors | |
| } | |
| } |
🤖 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 `@web/default/src/stores/auth-store.ts` around lines 98 - 101, Wrap the
localStorage cleanup operations in auth.reset() with a try...catch, covering
both removeItem calls after the existing window availability check. Ensure
storage access errors are caught so reset continues clearing in-memory
authentication state and does not interrupt sign-out.
|
Superseded by a rebased branch onto current QuantumNous/main. The original
Closing this PR in favor of a new one from the rebased branch. |
|
Closing in favor of rebased PR from xvyimu:rebase/upstream-pr-6284 |
2 participants
Summary
/metricsand other ops paths as HTML 200.frontend_external,FRONTEND_MODE, Dockerfile.backend, separated Nginx frontend) while keeping embedded dual-theme as default.translationnamespace unwrap + default language), sign-out cookie expiry, and Session-expired toast races during intentional logout.Notes
mainis based on a customized line and is currently ~38 commits ahead / ~7 behindQuantumNous/new-apimain; review as an integration PR and rebase may be required before merge.livez/readyz, logout cookie Max-Age=0, Chinese UI, OAuth homepage/callback alignment).Test plan
go test ./router/./middleware/go test -tags frontend_external ./livez,/readyz,/metrics404 non-HTML, logout Set-Cookie Max-Age=0Checklist
Summary by CodeRabbit
New Features
Bug Fixes