Teammate/quota runtime faq fixes - #4573
Closed
jkjk02 wants to merge 18 commits into
Closed
Conversation
# Conflicts: # web/default/src/components/layout/types.ts # web/default/src/features/wallet/components/affiliate-rewards-card.tsx # web/default/src/features/wallet/index.tsx # web/default/src/hooks/use-sidebar-data.ts
add api request url card to keys page
Contributor
WalkthroughThis PR centralizes role permission handling by introducing Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
model/user.go (1)
130-145: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse the repo JSON wrapper in this touched path.
generateDefaultSidebarConfigForRole()still marshals withjson.Marshal, but this codebase requires Go business code to go throughcommon/json.go. Please switch this function tocommon.Marshalwhile touching it.♻️ Suggested fix
- configBytes, err := json.Marshal(defaultConfig) + configBytes, err := common.Marshal(defaultConfig)As per coding guidelines, "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go... Do NOT directly import or callencoding/jsonin business code."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/user.go` around lines 130 - 145, In generateDefaultSidebarConfigForRole replace the direct call to json.Marshal with the repo wrapper by calling common.Marshal(defaultConfig) (assigning to configBytes, err as before), and ensure the file imports the common package instead of using encoding/json directly; preserve the existing error handling using the returned err and any subsequent logic that follows configBytes.controller/user.go (1)
433-443:⚠️ Potential issue | 🟠 MajorRemove hardcoded admin access and respect
SidebarModulesAdminconfiguration.The
generateDefaultSidebarConfigfunction grants full admin section access (setting: true) to all users with root permission without consulting theSidebarModulesAdminconfiguration. This bypasses the sidebar management system designed to provide granular admin permission control. Additionally,json.Marshalon line ~495 should usecommon.Marshal()fromcommon/json.goper coding guidelines.Refactor to:
- Read
SidebarModulesAdminfromcommon.OptionMapand parse it to determine which admin modules are actually enabled- Only include admin sections that are configured as accessible
- Use
common.Marshal()instead of directjson.Marshal()This also applies to the permissions assignment at line 440.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/user.go` around lines 433 - 443, The current code in generateDefaultSidebarConfig grants full admin access to root users and hardcodes admin module settings; change it to read the "SidebarModulesAdmin" value from common.OptionMap, parse it into the admin modules map, and only include admin sections present/enabled in that parsed configuration when building sidebar_modules for both root and non-root branches (do not unconditionally set setting:true or admin:true). Replace any direct json.Marshal calls in this function with common.Marshal() to produce JSON per project conventions. Update the permissions assignment where sidebar_modules is set so it uses the parsed admin modules map (or an empty map if the option is missing/invalid) instead of hardcoded values.web/default/src/features/keys/components/api-keys-mutate-drawer.tsx (2)
156-168:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid resetting the create form when
useStatus()finishes loading.
defaultUseAutoGroupis async, so this effect reruns when the status query resolves or refetches. If the drawer is already open in create mode,form.reset(...)will wipe whatever the user has typed so far. Limit this reset to the initial open, or guard it behind a dirty-state check.Suggested guard
- } else if (open && !isUpdate) { + } else if (open && !isUpdate && !form.formState.isDirty) { // For create, reset to defaults form.reset(getApiKeyFormDefaultValues(defaultUseAutoGroup)) } - }, [open, isUpdate, currentRow, form, defaultUseAutoGroup]) + }, [open, isUpdate, currentRow, form, defaultUseAutoGroup, form.formState.isDirty])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/keys/components/api-keys-mutate-drawer.tsx` around lines 156 - 168, The effect in useEffect that calls form.reset (via getApiKey(...).then(...) and getApiKeyFormDefaultValues(defaultUseAutoGroup)) is rerunning when the async defaultUseAutoGroup resolves and unintentionally wipes user input; change the guard so reset only happens on the initial open transition or when the form is pristine: detect the open transition (previousOpen false -> open true) or check form.isDirty() before calling form.reset, and keep the existing branches for isUpdate/currentRow and create mode (getApiKey, form.reset(transformApiKeyToFormDefaults(...)), and getApiKeyFormDefaultValues) but skip resetting if the drawer was already open or the form is dirty.
128-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t drive group availability off a localized
descstring.Filtering out entries whose
descis'用户分组'can leave the combobox with no valid options, andgroupis now a required field. It also makes the behavior depend on backend copy text, so any wording change silently changes which groups users are allowed to select. Use a stable flag/key from the API instead, or at least preserve the current value in the options list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/keys/components/api-keys-mutate-drawer.tsx` around lines 128 - 147, The code filters group options by the localized info.desc === '用户分组', which is unstable; change the predicate on groupsRaw to use a stable API flag/key (e.g., info.hidden === true or info.isUserGroup === true) instead of info.desc, and avoid dropping the currently selected group: after building groups from Object.entries(groupsRaw), ensure the form's current group value (e.g., the component prop or form initial value named group/currentGroup) is preserved by adding it back if missing (use groups.some(g => g.value === currentGroup) and push/unshift the missing option from groupsRaw). Also keep the existing defaultUseAutoGroup logic that injects the 'auto' option into groups.
🧹 Nitpick comments (4)
docs/private-deploy-sop.md (1)
270-271: ⚡ Quick winValidate compose syntax before restart.
Add
docker compose config -qbeforeup -dto fail fast on malformeddocker-compose.ymledits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/private-deploy-sop.md` around lines 270 - 271, Update the restart command so it validates the compose file first: run "docker compose config -q" (or "sudo docker compose config -q" if sudo is required) and only proceed to "sudo docker compose up -d new-api" when the validation succeeds; in practice, add the validation step immediately before the existing "sudo docker compose up -d new-api" command to fail fast on malformed docker-compose.yml edits.web/default/src/i18n/locales/ja.json (1)
2069-2069: 💤 Low valueConsider alternative translation for "Model Square"
The translation "モデル広場" (model hiroba/square) is a literal translation that may not convey the intended meaning clearly in a technical context. Consider these alternatives:
- "モデルスクエア" (katakana for "Model Square") - More common for feature names in Japanese UX
- "モデル一覧" (model list) - If this refers to a model listing/catalog
- "モデルマーケット" (model market) - If this is a marketplace concept
The current translation is grammatically correct but may confuse users expecting a more standard UI term.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/i18n/locales/ja.json` at line 2069, The current Japanese translation for the key "Model Square" uses a literal phrase "モデル広場" which may be unclear in a UX/technical context; update the value for the "Model Square" key in the ja.json locale to a more standard UI term such as "モデルスクエア" (katakana), "モデル一覧" (if it represents a listing), or "モデルマーケット" (if it's a marketplace) depending on the intended meaning—replace the string "モデル広場" with the chosen alternative while keeping the JSON key "Model Square" unchanged.web/default/src/features/pricing/components/model-details.tsx (1)
466-470: ⚡ Quick winCouple
routeFromandbackPathin a single typed contract.Line 466-Line 470 currently allows invalid pairings (e.g. pricing route + model-square back path), which can leak incompatible search state into Line 500. A discriminated union will prevent accidental misuse at compile time.
♻️ Proposed typing refinement
-type ModelDetailsProps = { - embedded?: boolean - routeFrom?: '/pricing/$modelId/' | '/_authenticated/model-square/$modelId/' - backPath?: '/pricing' | '/model-square' -} +type ModelDetailsProps = + | { + embedded?: boolean + routeFrom?: '/pricing/$modelId/' + backPath?: '/pricing' + } + | { + embedded?: boolean + routeFrom: '/_authenticated/model-square/$modelId/' + backPath?: '/model-square' + }Also applies to: 474-477, 500-501
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/pricing/components/model-details.tsx` around lines 466 - 470, The props type ModelDetailsProps allows invalid routeFrom/backPath combinations; replace it with a discriminated union that couples routeFrom and backPath into matching pairs (e.g. one variant where routeFrom: '/pricing/$modelId/' and backPath: '/pricing', and another variant where routeFrom: '/_authenticated/model-square/$modelId/' and backPath: '/model-square'), then update any references to ModelDetailsProps (including the other occurrences mentioned around the component and where search state is derived) so the compiler enforces valid pairings and prevents leaking incompatible search state into the logic that reads these props.web/default/src/custom/site.ts (1)
17-35: ⚡ Quick winUse scoped i18n keys here instead of raw labels.
titleKeyis acting as a translation key, so adding values like'Model Square'and'Status Monitor'keeps pushing the locale files toward inconsistent ad-hoc keys. Prefer semantically scoped keys such asnav.modelSquare/nav.statusMonitorhere and in the locale JSON.Based on learnings: Applies to
web/default/**/*.{ts,tsx}: Use hierarchical, semantically clear translation keys with consistent naming (e.g.,dashboard.overview.title).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/custom/site.ts` around lines 17 - 35, Replace literal label strings used as translation keys in customSidebarLinks and customTopNavLinks with scoped, semantic i18n keys (e.g., change titleKey: 'Model Square' to titleKey: 'nav.modelSquare' and 'Status Monitor' to 'nav.statusMonitor'); update the corresponding locale JSON entries under those scoped keys (and follow the project convention like dashboard.overview.title when appropriate) so lookups remain consistent across web/default/**/*.{ts,tsx}; ensure each object in customSidebarLinks and customTopNavLinks uses the new hierarchical keys for titleKey and verify consumers call the i18n lookup with those keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/constants.go`:
- Around line 175-184: Change HasRootPermission to only return true for the
actual root role (role == RoleRootUser) and add a new HasAdminPermission(role
int) bool that returns role >= RoleAdminUser; update EffectiveRole to rely on
the (new) HasRootPermission so only true root maps to RoleRootUser. After this
change, replace uses that intend “admin-area” access with HasAdminPermission
(e.g., admin menus/routes), but keep same-or-higher protections and root-only
checks using HasRootPermission (so callers like controller/twofa.go and
controller/custom_oauth.go that must remain root-only will not treat admins as
root).
In `@docs/private-deploy-sop.md`:
- Around line 329-344: The docs currently instruct to run git push origin
private/custom-ui after syncing upstream, which contradicts the PR-only rule for
the private/custom-ui branch; update the Upstream-sync instructions for the
private/custom-ui flow to remove the direct push and instead instruct the user
to push their local changes to a feature/sync branch (or the same branch if your
policy allows remote branches) and open a pull request for merging into
private/custom-ui—reference the branch name private/custom-ui in the text and
replace the final "git push origin private/custom-ui" step with a short line
telling readers to push their sync branch and create a PR for merging into
private/custom-ui per repo governance.
- Around line 255-267: The current loop replaces the first image: line
containing "new-api" which can hit the wrong service; instead locate the new-api
service block first by finding the service header line (match r'^\s*new-api:')
and note its indentation, then scan subsequent lines within that block (stop
when encountering a line with indentation less than or equal to the service
header or a new top-level service) and replace the image: line inside that block
only; if no new-api header or no image: line in the block is found, raise a
clear error.
In `@middleware/auth.go`:
- Around line 182-186: RootAuth() was relaxed from RoleRootUser to RoleAdminUser
causing sensitive endpoints to be exposed; restore/controller-level enforcement
by adding explicit role checks in the two handlers: in GetChannelKey and
FetchModels call authHelper or directly verify c.GetInt("role") (or equivalent)
against RoleRootUser (not RoleAdminUser) and abort with 403 when below
RoleRootUser, ensuring no other code path returns secrets for non-root users.
Also fix the misleading HasRootPermission()—either change its implementation to
return role >= RoleRootUser or rename it to HasAdminOrRootPermission() and
update all call sites to use the correct semantic; document any intentional
deviation if you opt to keep admin access.
In `@web/default/src/components/layout/components/nav-group.tsx`:
- Around line 142-146: The code currently converts query params with
Object.fromEntries(new URLSearchParams(search)) which forces all values to
strings and breaks Zod-based route validateSearch schemas; replace those
conversions in nav-group.tsx (the spots using Object.fromEntries(new
URLSearchParams(search))) by passing the raw search string into the route's
validateSearch (or calling route.validateSearch?.(search) and using its result)
so the route-level validator can coerce types correctly (e.g.,
booleans/enums/numbers) and only fall back to undefined when validation fails.
In `@web/default/src/components/layout/components/top-nav.tsx`:
- Around line 18-25: splitHref currently uses href.split('?') which loses parts
after the first '?' in query values; change splitHref to find the first '?' with
href.indexOf('?') and use slice to set pathname = href.slice(0, idx) (or the
whole href if idx === -1) and search = idx === -1 ? '' : href.slice(idx + 1) so
the entire query string (including any additional '?' characters in parameter
values) is preserved when passed to the Link components referenced in the file
(splitHref).
In `@web/default/src/features/dashboard/components/overview/summary-cards.tsx`:
- Around line 30-38: The translation keys for singular vs plural are
inconsistently cased in the duration formatter (variables days, hours, minutes
and the parts.push calls using t(...)), which can break i18n lookups; update the
three calls to t(...) to use a consistent key casing (e.g., use 'day'/'days',
'hour'/'hours', 'minute'/'minutes' or 'Day'/'Days'/'Hour'/'Hours' consistently)
for both singular and plural branches and ensure your translation files define
the chosen keys.
In `@web/default/src/features/keys/components/api-key-group-combobox.tsx`:
- Line 140: The ChevronsUpDown icon (and the other decorative icon rendered on
the same component around line 165) are purely visual and should be hidden from
assistive tech; update the JSX where ChevronsUpDown and the other decorative
icon are rendered in the ApiKeyGroupCombobox component to include
aria-hidden="true" (and ensure they are not focusable) so screen readers ignore
them while preserving visual appearance.
In `@web/default/src/features/pricing/components/model-details.tsx`:
- Line 554: The ArrowLeft icon in the Back button is decorative and causing
redundant screen-reader output; update the JSX where ArrowLeft is rendered (in
model-details.tsx, inside the Back button with visible text "Back") to add
aria-hidden="true" to the ArrowLeft element so assistive tech ignores the icon
while the button text remains the accessible label.
In `@web/default/src/features/pricing/components/pricing-table.tsx`:
- Around line 69-73: The handleRowClick callback currently calls
onModelClick(model.model_name || '') which can pass an empty string; change
handleRowClick to first check the PricingModel's model_name and no-op if it's
falsy (undefined/empty) so onModelClick is only invoked with a valid id. Update
the logic inside handleRowClick (referencing handleRowClick, PricingModel, and
onModelClick) and keep the useCallback dependency array unchanged.
In `@web/default/src/features/wallet/components/affiliate-rewards-card.tsx`:
- Around line 25-26: The savedAmount calculation is inverted: instead of
computing the discounted price, set savedAmount to originalPrice *
numericDiscount so it represents the amount saved (use the existing variables
originalPrice and numericDiscount); update the expression in
affiliate-rewards-card.tsx where savedAmount is defined (currently using
originalPrice * (1 - numericDiscount)) to use originalPrice * numericDiscount,
and ensure numericDiscount is the expected fraction (0-1) before the calculation
if needed.
In `@web/default/src/hooks/use-top-nav-links.ts`:
- Around line 76-80: The Pricing nav item currently uses href
'/model-square?view=table' regardless of auth and only sets disabled from
pricing.requireAuth; update the logic in useTopNavLinks (where links.push is
called for the Pricing item) to prevent guests from navigating to an auth-gated
route by either (a) marking the item disabled when pricing.requireAuth is true
and the user is not authenticated (use your auth flag/isAuthenticated), or (b)
when the user is unauthenticated, change the href to a safe entry such as the
login/authorize flow with a next/redirect param to '/model-square?view=table' so
clicks route to authentication first; ensure you reference pricing.requireAuth
and the auth state used elsewhere in use-top-nav-links when implementing this
change.
In `@web/default/src/i18n/locales/ja.json`:
- Around line 2069-2071: Add the missing dynamic title keys "Model Square" and
"Status Monitor" to the STATIC_I18N_KEYS array in src/i18n/static-keys.ts so the
extractor picks them up; specifically update the STATIC_I18N_KEYS constant to
include the exact key strings "Model Square" and "Status Monitor" (translations
already exist in ja.json) to align with their use as dynamic titleKey values in
site.ts.
In `@web/default/src/lib/roles.ts`:
- Around line 21-24: getRoleLabelKey currently normalizes ROLE.ADMIN to show the
SUPER_ADMIN label; restore direct label mapping by removing the special-case (do
not map ROLE.ADMIN to ROLE.SUPER_ADMIN) and return ROLE_LABEL_KEYS[role as
RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE] in getRoleLabelKey. Add a separate
helper function canAccessSystemSettings(role?: number): boolean that performs
the permission normalization (e.g., (role ?? DEFAULT_ROLE) >= ROLE.ADMIN) and
update route/menu gating call sites to use canAccessSystemSettings(...) instead
of relying on getRoleLabelKey for access checks.
In `@web/default/src/routes/_authenticated/model-square/`$modelId/index.tsx:
- Around line 6-16: The search schema modelSquareDetailsSearchSchema is missing
the caller's "view" param so opening ModelDetails from the table view loses
state; update modelSquareDetailsSearchSchema to include view:
z.string().optional() (or a z.enum of allowed views if you want stricter typing)
so the parsed route/search preserves and returns the original ?view value when
ModelDetails (and its navigation code that reads the search params) navigates
back.
In `@web/default/src/routes/_authenticated/status-monitor.tsx`:
- Around line 1-3: Replace the hardcoded iframe title with a localized string:
import and call useTranslation() in the component that defines the route (the
module using createFileRoute) and change title='Status Monitor' to
title={t('statusMonitor.title')} (or a suitable key), and similarly wrap any
other user-facing strings in that component (lines ~10-19) with t('...'); ensure
you add the import { useTranslation } from 'react-i18next' and use the t
function from const { t } = useTranslation() so AppHeader/Main/iframe use
localized text.
- Around line 16-20: The iframe embedding STATUS_MONITOR_URL lacks
isolation/privacy attributes; update the JSX <iframe> element to include a
restrictive sandbox attribute (e.g., sandbox with only the minimal needed flags
such as "allow-scripts" only if the monitor requires scripts, otherwise an empty
sandbox) and add referrerPolicy="no-referrer" (or "same-origin" if required by
the monitor) to prevent leaking the parent referrer; modify the iframe that uses
STATUS_MONITOR_URL and keep the existing title and className while choosing the
minimal sandbox flags required by the external monitor.
---
Outside diff comments:
In `@controller/user.go`:
- Around line 433-443: The current code in generateDefaultSidebarConfig grants
full admin access to root users and hardcodes admin module settings; change it
to read the "SidebarModulesAdmin" value from common.OptionMap, parse it into the
admin modules map, and only include admin sections present/enabled in that
parsed configuration when building sidebar_modules for both root and non-root
branches (do not unconditionally set setting:true or admin:true). Replace any
direct json.Marshal calls in this function with common.Marshal() to produce JSON
per project conventions. Update the permissions assignment where sidebar_modules
is set so it uses the parsed admin modules map (or an empty map if the option is
missing/invalid) instead of hardcoded values.
In `@model/user.go`:
- Around line 130-145: In generateDefaultSidebarConfigForRole replace the direct
call to json.Marshal with the repo wrapper by calling
common.Marshal(defaultConfig) (assigning to configBytes, err as before), and
ensure the file imports the common package instead of using encoding/json
directly; preserve the existing error handling using the returned err and any
subsequent logic that follows configBytes.
In `@web/default/src/features/keys/components/api-keys-mutate-drawer.tsx`:
- Around line 156-168: The effect in useEffect that calls form.reset (via
getApiKey(...).then(...) and getApiKeyFormDefaultValues(defaultUseAutoGroup)) is
rerunning when the async defaultUseAutoGroup resolves and unintentionally wipes
user input; change the guard so reset only happens on the initial open
transition or when the form is pristine: detect the open transition
(previousOpen false -> open true) or check form.isDirty() before calling
form.reset, and keep the existing branches for isUpdate/currentRow and create
mode (getApiKey, form.reset(transformApiKeyToFormDefaults(...)), and
getApiKeyFormDefaultValues) but skip resetting if the drawer was already open or
the form is dirty.
- Around line 128-147: The code filters group options by the localized info.desc
=== '用户分组', which is unstable; change the predicate on groupsRaw to use a stable
API flag/key (e.g., info.hidden === true or info.isUserGroup === true) instead
of info.desc, and avoid dropping the currently selected group: after building
groups from Object.entries(groupsRaw), ensure the form's current group value
(e.g., the component prop or form initial value named group/currentGroup) is
preserved by adding it back if missing (use groups.some(g => g.value ===
currentGroup) and push/unshift the missing option from groupsRaw). Also keep the
existing defaultUseAutoGroup logic that injects the 'auto' option into groups.
---
Nitpick comments:
In `@docs/private-deploy-sop.md`:
- Around line 270-271: Update the restart command so it validates the compose
file first: run "docker compose config -q" (or "sudo docker compose config -q"
if sudo is required) and only proceed to "sudo docker compose up -d new-api"
when the validation succeeds; in practice, add the validation step immediately
before the existing "sudo docker compose up -d new-api" command to fail fast on
malformed docker-compose.yml edits.
In `@web/default/src/custom/site.ts`:
- Around line 17-35: Replace literal label strings used as translation keys in
customSidebarLinks and customTopNavLinks with scoped, semantic i18n keys (e.g.,
change titleKey: 'Model Square' to titleKey: 'nav.modelSquare' and 'Status
Monitor' to 'nav.statusMonitor'); update the corresponding locale JSON entries
under those scoped keys (and follow the project convention like
dashboard.overview.title when appropriate) so lookups remain consistent across
web/default/**/*.{ts,tsx}; ensure each object in customSidebarLinks and
customTopNavLinks uses the new hierarchical keys for titleKey and verify
consumers call the i18n lookup with those keys.
In `@web/default/src/features/pricing/components/model-details.tsx`:
- Around line 466-470: The props type ModelDetailsProps allows invalid
routeFrom/backPath combinations; replace it with a discriminated union that
couples routeFrom and backPath into matching pairs (e.g. one variant where
routeFrom: '/pricing/$modelId/' and backPath: '/pricing', and another variant
where routeFrom: '/_authenticated/model-square/$modelId/' and backPath:
'/model-square'), then update any references to ModelDetailsProps (including the
other occurrences mentioned around the component and where search state is
derived) so the compiler enforces valid pairings and prevents leaking
incompatible search state into the logic that reads these props.
In `@web/default/src/i18n/locales/ja.json`:
- Line 2069: The current Japanese translation for the key "Model Square" uses a
literal phrase "モデル広場" which may be unclear in a UX/technical context; update
the value for the "Model Square" key in the ja.json locale to a more standard UI
term such as "モデルスクエア" (katakana), "モデル一覧" (if it represents a listing), or
"モデルマーケット" (if it's a marketplace) depending on the intended meaning—replace the
string "モデル広場" with the chosen alternative while keeping the JSON key "Model
Square" unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c230e2c-ef8d-4173-b9b9-399d790b8ecf
⛔ Files ignored due to path filters (4)
web/default/public/favicon-custom.icois excluded by!**/*.icoweb/default/public/favicon.icois excluded by!**/*.icoweb/default/public/logo-custom.pngis excluded by!**/*.pngweb/default/public/logo.pngis excluded by!**/*.png
📒 Files selected for processing (48)
.gitignorecommon/constants.gocontroller/custom_oauth.gocontroller/twofa.gocontroller/user.godocs/private-deploy-sop.mdmiddleware/auth.gomodel/user.goweb/default/index.htmlweb/default/src/components/layout/components/nav-group.tsxweb/default/src/components/layout/components/top-nav.tsxweb/default/src/components/layout/components/workspace-switcher.tsxweb/default/src/components/layout/types.tsweb/default/src/components/profile-dropdown.tsxweb/default/src/custom/site.tsweb/default/src/features/dashboard/components/overview/summary-cards.tsxweb/default/src/features/dashboard/hooks/use-dashboard-config.tsxweb/default/src/features/keys/components/api-key-group-combobox.tsxweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/keys/components/api-request-url-card.tsxweb/default/src/features/keys/constants.tsweb/default/src/features/keys/index.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/keys/lib/index.tsweb/default/src/features/pricing/components/model-details.tsxweb/default/src/features/pricing/components/pricing-table.tsxweb/default/src/features/pricing/hooks/use-filters.tsweb/default/src/features/pricing/index.tsxweb/default/src/features/wallet/components/affiliate-rewards-card.tsxweb/default/src/features/wallet/components/dialogs/payment-confirm-dialog.tsxweb/default/src/features/wallet/components/recharge-form-card.tsxweb/default/src/features/wallet/index.tsxweb/default/src/features/wallet/lib/format.tsweb/default/src/hooks/use-sidebar-data.tsweb/default/src/hooks/use-top-nav-links.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/lib/constants.tsweb/default/src/lib/roles.tsweb/default/src/routeTree.gen.tsweb/default/src/routes/_authenticated/model-square/$modelId/index.tsxweb/default/src/routes/_authenticated/model-square/index.tsxweb/default/src/routes/_authenticated/status-monitor.tsxweb/default/src/routes/_authenticated/system-settings/route.tsx
Comment on lines
+175
to
+184
| func HasRootPermission(role int) bool { | ||
| return role >= RoleAdminUser | ||
| } | ||
|
|
||
| func EffectiveRole(role int) int { | ||
| if HasRootPermission(role) { | ||
| return RoleRootUser | ||
| } | ||
| return role | ||
| } |
Contributor
There was a problem hiding this comment.
Split admin-access from true root overrides before this leaks more privileges.
HasRootPermission() now returns true for RoleAdminUser, so any caller that used to special-case only RoleRootUser now treats admins as equivalent to root. In this PR that already widens hierarchy bypasses in controller/twofa.go Line 523 and controller/custom_oauth.go Lines 504 and 563, which means an admin can operate on peer/root accounts. Keep a separate “admin-area access” helper and reserve HasRootPermission() for the actual root role.
🔒 Suggested direction
+func HasAdminPermission(role int) bool {
+ return role >= RoleAdminUser
+}
+
func HasRootPermission(role int) bool {
- return role >= RoleAdminUser
+ return role == RoleRootUser
}Then update only menu/route/admin-area gates to use HasAdminPermission, and keep same-or-higher-user protections on HasRootPermission.
📝 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.
Suggested change
| func HasRootPermission(role int) bool { | |
| return role >= RoleAdminUser | |
| } | |
| func EffectiveRole(role int) int { | |
| if HasRootPermission(role) { | |
| return RoleRootUser | |
| } | |
| return role | |
| } | |
| func HasAdminPermission(role int) bool { | |
| return role >= RoleAdminUser | |
| } | |
| func HasRootPermission(role int) bool { | |
| return role == RoleRootUser | |
| } | |
| func EffectiveRole(role int) int { | |
| if HasRootPermission(role) { | |
| return RoleRootUser | |
| } | |
| return role | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common/constants.go` around lines 175 - 184, Change HasRootPermission to only
return true for the actual root role (role == RoleRootUser) and add a new
HasAdminPermission(role int) bool that returns role >= RoleAdminUser; update
EffectiveRole to rely on the (new) HasRootPermission so only true root maps to
RoleRootUser. After this change, replace uses that intend “admin-area” access
with HasAdminPermission (e.g., admin menus/routes), but keep same-or-higher
protections and root-only checks using HasRootPermission (so callers like
controller/twofa.go and controller/custom_oauth.go that must remain root-only
will not treat admins as root).
Comment on lines
+255
to
+267
| sudo python3 - <<PY | ||
| from pathlib import Path | ||
| image = "$IMAGE" | ||
| path = Path("docker-compose.yml") | ||
| text = path.read_text() | ||
| lines = text.splitlines() | ||
| for idx, line in enumerate(lines): | ||
| if line.strip().startswith("image:") and "new-api" in line: | ||
| lines[idx] = f" image: {image}" | ||
| break | ||
| else: | ||
| raise SystemExit("new-api image line not found") | ||
| path.write_text("\n".join(lines) + "\n") |
Contributor
There was a problem hiding this comment.
Compose image replacement logic can target the wrong service.
This loop updates the first image: line containing "new-api", which is ambiguous and can modify a non-target service when multiple images match. Please scope replacement to the new-api service block explicitly.
Suggested safer replacement logic
sudo python3 - <<PY
from pathlib import Path
image = "$IMAGE"
path = Path("docker-compose.yml")
text = path.read_text()
lines = text.splitlines()
-for idx, line in enumerate(lines):
- if line.strip().startswith("image:") and "new-api" in line:
- lines[idx] = f" image: {image}"
- break
+in_new_api = False
+for idx, line in enumerate(lines):
+ stripped = line.strip()
+ if stripped.startswith("new-api:"):
+ in_new_api = True
+ continue
+ if in_new_api and stripped and not line.startswith(" "):
+ in_new_api = False
+ if in_new_api and stripped.startswith("image:"):
+ indent = line[: len(line) - len(line.lstrip(" "))]
+ lines[idx] = f"{indent}image: {image}"
+ break
else:
raise SystemExit("new-api image line not found")
path.write_text("\\n".join(lines) + "\\n")
PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/private-deploy-sop.md` around lines 255 - 267, The current loop replaces
the first image: line containing "new-api" which can hit the wrong service;
instead locate the new-api service block first by finding the service header
line (match r'^\s*new-api:') and note its indentation, then scan subsequent
lines within that block (stop when encountering a line with indentation less
than or equal to the service header or a new top-level service) and replace the
image: line inside that block only; if no new-api header or no image: line in
the block is found, raise a clear error.
Comment on lines
+329
to
+344
| git switch private/custom-ui | ||
| git pull --ff-only origin private/custom-ui | ||
| git merge upstream/main | ||
| ``` | ||
|
|
||
| 如果有冲突： | ||
|
|
||
| 1. 只解决和私人定制相关的冲突。 | ||
| 2. 不要顺手改无关文件。 | ||
| 3. 解决后运行检查。 | ||
| 4. 提交 merge commit。 | ||
| 5. 推送到私人仓库： | ||
|
|
||
| ```bash | ||
| git push origin private/custom-ui | ||
| ``` |
Contributor
There was a problem hiding this comment.
Upstream-sync section conflicts with the PR-only rule for private/custom-ui.
This section ends with git push origin private/custom-ui, which contradicts Line 15–17 (“don’t direct-push; use PR”). Keep governance consistent here to avoid bypassing review during sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/private-deploy-sop.md` around lines 329 - 344, The docs currently
instruct to run git push origin private/custom-ui after syncing upstream, which
contradicts the PR-only rule for the private/custom-ui branch; update the
Upstream-sync instructions for the private/custom-ui flow to remove the direct
push and instead instruct the user to push their local changes to a feature/sync
branch (or the same branch if your policy allows remote branches) and open a
pull request for merging into private/custom-ui—reference the branch name
private/custom-ui in the text and replace the final "git push origin
private/custom-ui" step with a short line telling readers to push their sync
branch and create a PR for merging into private/custom-ui per repo governance.
Comment on lines
182
to
186
| func RootAuth() func(c *gin.Context) { | ||
| return func(c *gin.Context) { | ||
| authHelper(c, common.RoleRootUser) | ||
| authHelper(c, common.RoleAdminUser) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all routes using RootAuth() have controller-level HasRootPermission checks
echo "=== Routes using RootAuth ==="
rg -n 'RootAuth\(\)' --type go
echo ""
echo "=== Controllers with HasRootPermission checks ==="
rg -n 'HasRootPermission' --type go -A 2 -B 2Repository: QuantumNous/new-api
Length of output: 5003
🏁 Script executed:
#!/bin/bash
# 1. Get RoleAdminUser and RoleRootUser definitions
echo "=== Role definitions ==="
rg -n 'RoleAdminUser|RoleRootUser' common/constants.go -A 1 -B 1
echo ""
echo "=== Handler functions for affected routes ==="
# Check GetChannelKey handler
echo "GetChannelKey handler:"
rg -n 'func.*GetChannelKey' controller/ -A 10 | head -20
echo ""
echo "FetchModels handler:"
rg -n 'func.*FetchModels' controller/ -A 10 | head -20Repository: QuantumNous/new-api
Length of output: 2182
🏁 Script executed:
#!/bin/bash
# Get full GetChannelKey implementation
echo "=== GetChannelKey full implementation ==="
sed -n '385,430p' controller/channel.go
echo ""
echo "=== FetchModels full implementation ==="
sed -n '973,1050p' controller/channel.go
echo ""
echo "=== Check for handlers using optionRoute, customOAuthRoute, performanceRoute, ratioSyncRoute ==="
# These are likely controller functions without explicit names, search for them
rg -n 'func.*\(c \*gin\.Context\)' controller/ -A 5 | grep -A 5 -i 'option\|performance\|ratio' | head -30Repository: QuantumNous/new-api
Length of output: 4272
RootAuth() change creates a security regression without adequate controller-level protection.
This change lowers the privilege requirement from RoleRootUser (100) to RoleAdminUser (10)—a 10x reduction. Two sensitive endpoints now have inadequate protection:
GetChannelKeyhandler (controller/channel.go:385): Exposes all channel API keys to any admin user, with no internal permission checks.FetchModelshandler (controller/channel.go:973): Allows any admin user to test external API endpoints with arbitrary credentials, with no internal permission checks.
Additionally, HasRootPermission() (common/constants.go:175) is dangerously misleading—it checks role >= RoleAdminUser, not actual root user status. This invites future bugs where developers assume the function enforces root-level access.
Required actions:
- Add explicit permission checks within
GetChannelKeyandFetchModelshandlers to restrict sensitive operations toRoleRootUseronly. - Either rename
HasRootPermission()toHasAdminOrRootPermission()or fix its implementation to match its name. - If the intent is genuinely to grant admin-level access to these endpoints, document the security implications and risk acceptance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@middleware/auth.go` around lines 182 - 186, RootAuth() was relaxed from
RoleRootUser to RoleAdminUser causing sensitive endpoints to be exposed;
restore/controller-level enforcement by adding explicit role checks in the two
handlers: in GetChannelKey and FetchModels call authHelper or directly verify
c.GetInt("role") (or equivalent) against RoleRootUser (not RoleAdminUser) and
abort with 403 when below RoleRootUser, ensuring no other code path returns
secrets for non-root users. Also fix the misleading HasRootPermission()—either
change its implementation to return role >= RoleRootUser or rename it to
HasAdminOrRootPermission() and update all call sites to use the correct
semantic; document any intentional deviation if you opt to keep admin access.
Comment on lines
+142
to
+146
| search={ | ||
| search | ||
| ? Object.fromEntries(new URLSearchParams(search)) | ||
| : undefined | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Routes with typed/non-string validateSearch fields =="
rg -n "validateSearch|z\\.boolean\\(|z\\.number\\(|z\\.enum\\(" web/default/src/routes -A2 -B2
echo
echo "== Navigation URL definitions that include query strings =="
rg -n "url:\\s*['\"][^'\"]*\\?" web/default/src
echo
echo "== Current nav query conversion call sites =="
rg -n "Object\\.fromEntries\\(new URLSearchParams\\(search\\)\\)" web/default/src/components/layout/components/nav-group.tsxRepository: QuantumNous/new-api
Length of output: 13438
Search param coercion to string breaks typed route validateSearch schemas.
At lines 142 and 215, Object.fromEntries(new URLSearchParams(search)) forces all values to strings. Multiple routes expect non-string types: z.boolean() in wallet, model-square, and pricing routes; z.enum() in model-square and pricing routes; z.number() in keys, usage-logs, models, channels, and users routes. URLSearchParams coercion causes Zod validation to fail silently, triggering .catch() defaults instead of preserving the intended filter state.
Suggested fix
function splitUrl(url: string) {
const [pathname, search = ''] = url.split('?')
return {
pathname,
search,
}
}
+
+function parseSearchObject(search: string): Record<string, unknown> | undefined {
+ if (!search) return undefined
+ return Object.fromEntries(
+ Array.from(new URLSearchParams(search).entries()).map(([key, value]) => {
+ try {
+ return [key, JSON.parse(value)]
+ } catch {
+ return [key, value]
+ }
+ })
+ )
+}
@@
<Link
to={pathname}
- search={
- search
- ? Object.fromEntries(new URLSearchParams(search))
- : undefined
- }
+ search={parseSearchObject(search)}
onClick={() => setOpenMobile(false)}
>
@@
<Link
to={pathname}
- search={
- search
- ? Object.fromEntries(new URLSearchParams(search))
- : undefined
- }
+ search={parseSearchObject(search)}
onClick={() => setOpenMobile(false)}
>Also applies to: 215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/components/layout/components/nav-group.tsx` around lines 142
- 146, The code currently converts query params with Object.fromEntries(new
URLSearchParams(search)) which forces all values to strings and breaks Zod-based
route validateSearch schemas; replace those conversions in nav-group.tsx (the
spots using Object.fromEntries(new URLSearchParams(search))) by passing the raw
search string into the route's validateSearch (or calling
route.validateSearch?.(search) and using its result) so the route-level
validator can coerce types correctly (e.g., booleans/enums/numbers) and only
fall back to undefined when validation fails.
Comment on lines
+2069
to
2071
| "Model Square": "モデル広場", | ||
| "Status Monitor": "ステータス監視", | ||
| "Model Pricing": "モデル料金", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if new i18n keys are registered in static-keys.ts or used as string literals
# Search for the new keys in static-keys.ts
echo "=== Checking static-keys.ts ==="
rg -n "Model Square|Status Monitor|Original price|Pricing Information|Recharge rate and discount tiers|Recharge Rate|Recharge Discounts|No recharge discounts configured" web/default/src/i18n/static-keys.ts
# Search for usage as t() string literals in the codebase
echo -e "\n=== Checking t() usage in components ==="
rg -n -C2 "t\(['\"](?:Model Square|Status Monitor|Original price|Pricing Information|Recharge rate and discount tiers|Recharge Rate|Recharge Discounts|No recharge discounts configured)['\"]" web/default/src/Repository: QuantumNous/new-api
Length of output: 3580
🏁 Script executed:
# Search for "Model Square" and "Status Monitor" usage anywhere in the codebase
echo "=== Searching for Model Square usage ==="
rg -n "Model Square" web/default/src/
echo -e "\n=== Searching for Status Monitor usage ==="
rg -n "Status Monitor" web/default/src/
echo -e "\n=== Examining static-keys.ts file ==="
head -50 web/default/src/i18n/static-keys.ts
echo -e "\n=== Checking ja.json context around lines 2069-2071 ==="
sed -n '2065,2075p' web/default/src/i18n/locales/ja.json
echo -e "\n=== Checking ja.json context around lines 3866-3871 ==="
sed -n '3862,3875p' web/default/src/i18n/locales/ja.jsonRepository: QuantumNous/new-api
Length of output: 3559
Add "Model Square" and "Status Monitor" to STATIC_I18N_KEYS
These keys are used as dynamic titleKey values in web/default/src/custom/site.ts and won't be picked up by the t('...') extraction scanner. They must be registered in src/i18n/static-keys.ts per the design pattern for dynamic configuration labels.
The other keys ("Original price", "Pricing Information", "Recharge rate and discount tiers", "Recharge Rate", "Recharge Discounts", "No recharge discounts configured") are already properly used with t('...') string literals in components and are correctly extracted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/i18n/locales/ja.json` around lines 2069 - 2071, Add the
missing dynamic title keys "Model Square" and "Status Monitor" to the
STATIC_I18N_KEYS array in src/i18n/static-keys.ts so the extractor picks them
up; specifically update the STATIC_I18N_KEYS constant to include the exact key
strings "Model Square" and "Status Monitor" (translations already exist in
ja.json) to align with their use as dynamic titleKey values in site.ts.
Comment on lines
21
to
24
| export function getRoleLabelKey(role?: number): string { | ||
| if ((role ?? DEFAULT_ROLE) >= ROLE.ADMIN) return ROLE_LABEL_KEYS[ROLE.SUPER_ADMIN] | ||
|
|
||
| return ROLE_LABEL_KEYS[role as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE] |
Contributor
There was a problem hiding this comment.
Keep role labeling separate from permission normalization.
This makes every ROLE.ADMIN user render as “Super Admin”. getRoleLabel() is used in the profile header and mobile user display, so admins now get a higher privilege label than they actually have. Preserve the direct label map here and add a separate helper for ROLE.ADMIN+ access checks.
🪪 Suggested fix
+export function canAccessSystemSettings(role?: number): boolean {
+ return (role ?? DEFAULT_ROLE) >= ROLE.ADMIN
+}
+
export function getRoleLabelKey(role?: number): string {
- if ((role ?? DEFAULT_ROLE) >= ROLE.ADMIN) return ROLE_LABEL_KEYS[ROLE.SUPER_ADMIN]
-
- return ROLE_LABEL_KEYS[role as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE]
+ const resolvedRole = role ?? DEFAULT_ROLE
+ return ROLE_LABEL_KEYS[resolvedRole as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE]
}Then switch the route/menu gating call sites to canAccessSystemSettings(...).
📝 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.
Suggested change
| export function getRoleLabelKey(role?: number): string { | |
| if ((role ?? DEFAULT_ROLE) >= ROLE.ADMIN) return ROLE_LABEL_KEYS[ROLE.SUPER_ADMIN] | |
| return ROLE_LABEL_KEYS[role as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE] | |
| export function canAccessSystemSettings(role?: number): boolean { | |
| return (role ?? DEFAULT_ROLE) >= ROLE.ADMIN | |
| } | |
| export function getRoleLabelKey(role?: number): string { | |
| const resolvedRole = role ?? DEFAULT_ROLE | |
| return ROLE_LABEL_KEYS[resolvedRole as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/lib/roles.ts` around lines 21 - 24, getRoleLabelKey currently
normalizes ROLE.ADMIN to show the SUPER_ADMIN label; restore direct label
mapping by removing the special-case (do not map ROLE.ADMIN to ROLE.SUPER_ADMIN)
and return ROLE_LABEL_KEYS[role as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE]
in getRoleLabelKey. Add a separate helper function
canAccessSystemSettings(role?: number): boolean that performs the permission
normalization (e.g., (role ?? DEFAULT_ROLE) >= ROLE.ADMIN) and update route/menu
gating call sites to use canAccessSystemSettings(...) instead of relying on
getRoleLabelKey for access checks.
Comment on lines
+6
to
+16
| const modelSquareDetailsSearchSchema = z.object({ | ||
| search: z.string().optional(), | ||
| sort: z.string().optional(), | ||
| vendor: z.string().optional(), | ||
| group: z.string().optional(), | ||
| quotaType: z.string().optional(), | ||
| endpointType: z.string().optional(), | ||
| tag: z.string().optional(), | ||
| tokenUnit: z.enum(['M', 'K']).optional(), | ||
| rechargePrice: z.boolean().optional(), | ||
| }) |
Contributor
There was a problem hiding this comment.
Preserve the caller’s view search param in the detail route.
This schema drops view, but ModelDetails navigates back with the current route search and the model-square entry link already uses ?view=table. Opening a model from table view will therefore lose that state and send the user back to the default view instead of the one they came from.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/routes/_authenticated/model-square/`$modelId/index.tsx around
lines 6 - 16, The search schema modelSquareDetailsSearchSchema is missing the
caller's "view" param so opening ModelDetails from the table view loses state;
update modelSquareDetailsSearchSchema to include view: z.string().optional() (or
a z.enum of allowed views if you want stricter typing) so the parsed
route/search preserves and returns the original ?view value when ModelDetails
(and its navigation code that reads the search params) navigates back.
Comment on lines
+1
to
+3
| import { createFileRoute } from '@tanstack/react-router' | ||
| import { AppHeader, Main } from '@/components/layout' | ||
|
|
Contributor
There was a problem hiding this comment.
Localize the iframe title instead of hardcoding English.
title='Status Monitor' should come from t(...) so it updates with active language.
Suggested fix
import { createFileRoute } from '@tanstack/react-router'
import { AppHeader, Main } from '@/components/layout'
+import { useTranslation } from 'react-i18next'
@@
function StatusMonitor() {
+ const { t } = useTranslation()
+
return (
@@
<iframe
src={STATUS_MONITOR_URL}
- title='Status Monitor'
+ title={t('Status Monitor')}
className='h-full w-full border-0'
/>Also applies to: 10-19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/routes/_authenticated/status-monitor.tsx` around lines 1 - 3,
Replace the hardcoded iframe title with a localized string: import and call
useTranslation() in the component that defines the route (the module using
createFileRoute) and change title='Status Monitor' to
title={t('statusMonitor.title')} (or a suitable key), and similarly wrap any
other user-facing strings in that component (lines ~10-19) with t('...'); ensure
you add the import { useTranslation } from 'react-i18next' and use the t
function from const { t } = useTranslation() so AppHeader/Main/iframe use
localized text.
Comment on lines
+16
to
+20
| <iframe | ||
| src={STATUS_MONITOR_URL} | ||
| title='Status Monitor' | ||
| className='h-full w-full border-0' | ||
| /> |
Contributor
There was a problem hiding this comment.
Harden the external iframe embedding.
The iframe loads third-party content without sandbox and referrerPolicy, which weakens isolation/privacy guarantees.
Suggested hardening
<iframe
src={STATUS_MONITOR_URL}
title='Status Monitor'
className='h-full w-full border-0'
+ loading='lazy'
+ referrerPolicy='no-referrer'
+ sandbox='allow-scripts allow-same-origin'
/>📝 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.
Suggested change
| <iframe | |
| src={STATUS_MONITOR_URL} | |
| title='Status Monitor' | |
| className='h-full w-full border-0' | |
| /> | |
| <iframe | |
| src={STATUS_MONITOR_URL} | |
| title='Status Monitor' | |
| className='h-full w-full border-0' | |
| loading='lazy' | |
| referrerPolicy='no-referrer' | |
| sandbox='allow-scripts allow-same-origin' | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/routes/_authenticated/status-monitor.tsx` around lines 16 -
20, The iframe embedding STATUS_MONITOR_URL lacks isolation/privacy attributes;
update the JSX <iframe> element to include a restrictive sandbox attribute
(e.g., sandbox with only the minimal needed flags such as "allow-scripts" only
if the monitor requires scripts, otherwise an empty sandbox) and add
referrerPolicy="no-referrer" (or "same-origin" if required by the monitor) to
prevent leaking the parent referrer; modify the iframe that uses
STATUS_MONITOR_URL and keep the existing title and className while choosing the
minimal sandbox flags required by the external monitor.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features
Improvements
Localization
Documentation