Conversation
WalkthroughThis pull request introduces a resource-blocking authorization system and applies it across multiple routes. It adds three new functions to Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (3)
src/lib/helpers/project.ts (2)
65-79: Missing validation for emptyresourceIdparameter.If
resourceIdis an empty string,normalizedId.trim()will be"", andisResourceBlockedwill attempt to match a block with an empty ID. This could lead to unexpected behavior if callers pass falsy values. Consider adding early-return guard:🛡️ Proposed defensive check
export function isResourceBlocked( project: Models.Project | null | undefined, resourceType: string, resourceId: string ): boolean { const normalizedType = resourceType.trim().toLowerCase(); const normalizedId = resourceId.trim(); + + if (!normalizedType || !normalizedId) { + return false; + } return (project?.blocks ?? []).some((block) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/helpers/project.ts` around lines 65 - 79, The function isResourceBlocked currently normalizes resourceId but doesn't guard against an empty string, allowing matches against empty IDs; update isResourceBlocked to trim and normalize resourceId first and if the resulting normalizedId is an empty string (or falsy) return false immediately before iterating blocks (leave normalization of resourceType as-is), so callers passing an empty or whitespace resourceId cannot incorrectly match a block.
81-95: Guard function useserror()which throws - ensure callers handle appropriately.The
guardResourceBlockfunction calls SvelteKit'serror(403, ...)which throws an exception. This is the correct pattern for SvelteKit load functions. However, the function signature doesn't indicate it throws (noneverreturn type for the blocked path).Additionally, the function lacks a return statement after the guard check - this is fine since
error()throws, but adding an explicit return type annotation would improve clarity:📝 Consider explicit return type
export function guardResourceBlock( project: Models.Project | null | undefined, resourceType: string | string[], resourceId: string -) { +): void { const resourceTypes = Array.isArray(resourceType) ? resourceType : [resourceType]; const isBlocked = resourceTypes.some((type) => isResourceBlocked(project, type, resourceId)); if (isBlocked) { error(403, { type: 'general_resource_blocked', message: 'This resource page cannot be accessed.' }); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/helpers/project.ts` around lines 81 - 95, guardResourceBlock calls SvelteKit's error(...) which throws; update the function signature for guardResourceBlock to include an explicit return type (e.g., ": void") and add a brief JSDoc noting it will throw a 403 via error(...) when isResourceBlocked(...) returns true so callers (load functions) can handle that exception; keep the existing call to error(...) and the use of isResourceBlocked and resourceTypes unchanged.src/routes/(console)/project-[region]-[project]/auth/user-[user]/+layout.ts (1)
11-11: Create aResourceTypeenum for compile-time validation of resource type strings.The
guardResourceBlockfunction acceptsresourceTypeas an untyped string parameter. WhileisResourceBlockednormalizes the type string withtrim().toLowerCase(), it performs strict equality comparison (type === normalizedType). A typo like'user'instead of'users'would pass compilation, be normalized to'user', then silently fail to match the actual block data containing'users', allowing a blocked resource to be accessed undetected.Defining a
ResourceTypeenum in$lib/helpers/project.tswith the values used across all layout files ('users','sites','databases','topics','tables','collections','providers','messages','buckets','files','functions','teams') would provide compile-time safety across all 11 call sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(console)/project-[region]-[project]/auth/user-[user]/+layout.ts at line 11, The call to guardResourceBlock(project, 'users', params.user) (and other similar calls) uses raw strings for resourceType which allows typos like 'user' to compile but fail matching at runtime; to fix, add a ResourceType enum in $lib/helpers/project.ts containing the canonical values ('users','sites','databases','topics','tables','collections','providers','messages','buckets','files','functions','teams'), change guardResourceBlock and isResourceBlocked to accept ResourceType (or a typed alias) instead of string, and update all 11 call sites to pass ResourceType.<...> enum members (e.g., ResourceType.Users) so the compiler enforces correct resource type names and prevents silent mismatches in isResourceBlocked and guardResourceBlock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/helpers/project.ts`:
- Around line 65-79: The function isResourceBlocked currently normalizes
resourceId but doesn't guard against an empty string, allowing matches against
empty IDs; update isResourceBlocked to trim and normalize resourceId first and
if the resulting normalizedId is an empty string (or falsy) return false
immediately before iterating blocks (leave normalization of resourceType as-is),
so callers passing an empty or whitespace resourceId cannot incorrectly match a
block.
- Around line 81-95: guardResourceBlock calls SvelteKit's error(...) which
throws; update the function signature for guardResourceBlock to include an
explicit return type (e.g., ": void") and add a brief JSDoc noting it will throw
a 403 via error(...) when isResourceBlocked(...) returns true so callers (load
functions) can handle that exception; keep the existing call to error(...) and
the use of isResourceBlocked and resourceTypes unchanged.
In `@src/routes/`(console)/project-[region]-[project]/auth/user-[user]/+layout.ts:
- Line 11: The call to guardResourceBlock(project, 'users', params.user) (and
other similar calls) uses raw strings for resourceType which allows typos like
'user' to compile but fail matching at runtime; to fix, add a ResourceType enum
in $lib/helpers/project.ts containing the canonical values
('users','sites','databases','topics','tables','collections','providers','messages','buckets','files','functions','teams'),
change guardResourceBlock and isResourceBlocked to accept ResourceType (or a
typed alias) instead of string, and update all 11 call sites to pass
ResourceType.<...> enum members (e.g., ResourceType.Users) so the compiler
enforces correct resource type names and prevents silent mismatches in
isResourceBlocked and guardResourceBlock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40a40f3b-0f6b-4f4a-854b-ab6eb7f305b7
📒 Files selected for processing (14)
AGENTS.mdCLAUDE.mdsrc/lib/helpers/project.tssrc/routes/(console)/project-[region]-[project]/auth/teams/team-[team]/+layout.tssrc/routes/(console)/project-[region]-[project]/auth/user-[user]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.tssrc/routes/(console)/project-[region]-[project]/functions/function-[function]/+layout.tssrc/routes/(console)/project-[region]-[project]/messaging/message-[message]/+layout.tssrc/routes/(console)/project-[region]-[project]/messaging/providers/provider-[provider]/+layout.tssrc/routes/(console)/project-[region]-[project]/messaging/topics/topic-[topic]/+layout.tssrc/routes/(console)/project-[region]-[project]/sites/site-[site]/+layout.tssrc/routes/(console)/project-[region]-[project]/storage/bucket-[bucket]/+layout.tssrc/routes/(console)/project-[region]-[project]/storage/bucket-[bucket]/file-[file]/+layout.ts
Greptile SummaryThis PR fixes a bug where any resource-level block (e.g. a blocked bucket or function) was incorrectly causing the entire project console to show the "Project is currently blocked" overlay. The root cause was that What changed:
Key observations:
Confidence Score: 5/5Safe to merge — the core logic is correct, the error-page integration was already in place, and both findings are P2 quality-of-life concerns rather than present defects. All findings are P2: (1) the missing src/lib/helpers/project.ts — the two new exported helpers are the heart of the change and warrant attention for the paused-status guard and resource-type string hardcoding. Important Files Changed
Reviews (1): Last reviewed commit: "can be projects too" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export function isResourceBlocked( | ||
| project: Models.Project | null | undefined, | ||
| resourceType: string, | ||
| resourceId: string | ||
| ): boolean { | ||
| const normalizedType = resourceType.trim().toLowerCase(); | ||
| const normalizedId = resourceId.trim(); | ||
|
|
||
| return (project?.blocks ?? []).some((block) => { | ||
| const type = block.resourceType?.trim().toLowerCase(); | ||
| const id = block.resourceId?.trim(); | ||
|
|
||
| return type === normalizedType && id === normalizedId; | ||
| }); |
There was a problem hiding this comment.
isResourceBlocked skips the paused-project guard
isProjectBlocked explicitly returns false for paused projects (project?.status !== 'paused') so that the "project paused" overlay is shown instead of the blocked-project overlay. isResourceBlocked has no equivalent guard, so a paused project that also carries a resource-level block will trigger a 403 error page instead of the paused overlay when a user navigates to that resource.
If paused projects should never surface resource-level 403s (and let the parent layout's paused UI take over), consider mirroring the status guard:
export function isResourceBlocked(
project: Models.Project | null | undefined,
resourceType: string,
resourceId: string
): boolean {
if (project?.status === 'paused') return false;
const normalizedType = resourceType.trim().toLowerCase();
const normalizedId = resourceId.trim();
return (project?.blocks ?? []).some((block) => {
const type = block.resourceType?.trim().toLowerCase();
const id = block.resourceId?.trim();
return type === normalizedType && id === normalizedId;
});
}| export function guardResourceBlock( | ||
| project: Models.Project | null | undefined, | ||
| resourceType: string | string[], | ||
| resourceId: string | ||
| ) { | ||
| const resourceTypes = Array.isArray(resourceType) ? resourceType : [resourceType]; | ||
| const isBlocked = resourceTypes.some((type) => isResourceBlocked(project, type, resourceId)); | ||
|
|
||
| if (isBlocked) { | ||
| error(403, { | ||
| type: 'general_resource_blocked', | ||
| message: 'This resource page cannot be accessed.' | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The resource type strings passed to guardResourceBlock across all layout files ('teams', 'users', 'databases', 'functions', 'buckets', 'files', 'messages', 'providers', 'topics', 'sites', 'tables', 'collections') are compared case-insensitively against block.resourceType. If the backend sends different casing or naming, the check silently passes and the block is never enforced.
Consider centralising these strings in an enum or const object alongside Dependencies in $lib/constants, e.g.:
export const BlockResourceType = {
TEAMS: 'teams',
USERS: 'users',
DATABASES: 'databases',
FUNCTIONS: 'functions',
BUCKETS: 'buckets',
FILES: 'files',
MESSAGES: 'messages',
PROVIDERS: 'providers',
TOPICS: 'topics',
SITES: 'sites',
TABLES: 'tables',
COLLECTIONS: 'collections',
} as const;This makes them easy to audit against the API contract and prevents typos across the 11 call-sites.
2 participants
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Chores