Repository navigation
fix(settings): add missing home page pin keys to updateSettingsSchema - #2915
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds three new optional configuration settings (pinProviderQuotaToHome, showQuickStartOnHome, and showProviderTopologyOnHome) to the updateSettingsSchema in src/shared/validation/settingsSchemas.ts. The review feedback notes that critical security-impacting keys (localOnlyManageScopeBypassEnabled and localOnlyManageScopeBypassPrefixes) are missing from the schema, which silently prevents them from being updated. Additionally, the reviewer requests that unit tests be added to validate these new settings keys, as required by the repository style guide.
| pinProviderQuotaToHome: z.boolean().optional(), | ||
| showQuickStartOnHome: z.boolean().optional(), | ||
| showProviderTopologyOnHome: z.boolean().optional(), |
There was a problem hiding this comment.
In addition to the home page pin keys, the security-impacting settings keys localOnlyManageScopeBypassEnabled and localOnlyManageScopeBypassPrefixes (referenced in src/app/api/settings/route.ts and src/types/settings.ts) are also completely missing from updateSettingsSchema.
Because they are missing, any PATCH request to /api/settings attempting to update these keys will have them silently stripped by Zod. Consequently, the security-impacting password re-auth gate in the route handler is never triggered for them, and they can never be persisted to the database.
Please add these keys to the schema to restore their functionality.
| pinProviderQuotaToHome: z.boolean().optional(), | |
| showQuickStartOnHome: z.boolean().optional(), | |
| showProviderTopologyOnHome: z.boolean().optional(), | |
| pinProviderQuotaToHome: z.boolean().optional(), | |
| showQuickStartOnHome: z.boolean().optional(), | |
| showProviderTopologyOnHome: z.boolean().optional(), | |
| localOnlyManageScopeBypassEnabled: z.boolean().optional(), | |
| localOnlyManageScopeBypassPrefixes: z.array(z.string().max(200)).optional(), |
| hideEndpointNgrokTunnel: z.boolean().optional(), | ||
| autoRefreshProviderQuota: z.boolean().optional(), | ||
| autoRefreshProviderQuotaInterval: z.number().int().min(10).max(3600).optional(), | ||
| pinProviderQuotaToHome: z.boolean().optional(), |
There was a problem hiding this comment.
According to the Repository Style Guide (Rule 9), tests must always be included when changing production code under src/. Please add corresponding unit tests (e.g., in tests/) to verify that the new settings keys are correctly validated and parsed by updateSettingsSchema.
References
- Always include tests when changing production code (src/, open-sse/, electron/, bin/). (link)
|
Addressed both review points:
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The PR correctly adds 5 new optional settings to
All schema validations are appropriate and consistent with existing patterns. The unit tests comprehensively cover:
The existing review comments have been addressed by the author. All changes are minimal, focused, and follow the established codebase patterns. Files Reviewed (2 files)
Reviewed by laguna-m.1-20260312:free · 539,979 tokens |
diegosouzapw
left a comment
There was a problem hiding this comment.
Approved after local verification, quality checks and merging into release/v3.8.7.
…gs-schema-validation fix(settings): add missing home page pin keys to updateSettingsSchema
Problem
The "Pin Information to Home Page" toggles in the Appearance settings tab (Provider Quota Limits, Quick Start, Provider Topology) were non-functional. Changing any of these toggles had no effect on the visibility of the corresponding sections on the Home page.
Root Cause
The three settings keys —
pinProviderQuotaToHome,showQuickStartOnHome, andshowProviderTopologyOnHome— were missing from theupdateSettingsSchemaZod schema insettingsSchemas.ts. When the Appearance tab sent a PATCH request to/api/settings, Zod's default.strip()mode silently removed these unrecognized keys from the parsed body. TheupdateSettings()function therefore never received them, and they were never persisted to the database. Since the Home page reads these settings from the database on mount, the changes were invisible.Solution
Added the three missing keys to the
updateSettingsSchemaZod schema, enabling them to pass through validation and be persisted correctly.These keys already existed in the general settings schema (
schemas.ts) — the gap was isolated to the PATCH-specific validation schema.Changes
pinProviderQuotaToHome: z.boolean().optional()toupdateSettingsSchemashowQuickStartOnHome: z.boolean().optional()toupdateSettingsSchemashowProviderTopologyOnHome: z.boolean().optional()toupdateSettingsSchema