-
Notifications
You must be signed in to change notification settings - Fork 191
fix: base paths #2386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: base paths #2386
Conversation
ConsoleProject ID: Sites (2)
Note Cursor pagination performs better than offset pagination when loading further pages. |
WalkthroughThis PR replaces hard-coded "/console" path segments with the dynamic base path from $app/paths across the app. Updates include navigation hrefs, redirects, conditional path checks, image sources, font preloads, and payment confirmation redirect URLs. A local variable named "base" in planSummary.svelte was renamed to "basePlan" to avoid shadowing. package.json updates @sveltejs/kit from ^2.36.2 to ^2.42.1. vercel.json is removed. No exported/public API signatures change, aside from initializing hideBillingHeaderRoutes with base-aware paths. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/routes/(console)/create-organization/+page.svelte (1)
86-97
: Bug: double “/console” after base causes bad navigation.These two navigations will produce “${base}/console/…”; if base itself is “/console”, the final URL becomes “/console/console/...”.
- await preloadData(`${base}/console/organization-${org.$id}`); - await goto(`${base}/console/organization-${org.$id}`); + await preloadData(`${base}/organization-${org.$id}`); + await goto(`${base}/organization-${org.$id}`);src/routes/(console)/apply-credit/+page.svelte (1)
101-108
: Bug: mismatch betweenorg
param set on redirect andid
param read on return.On return (
type=payment_confirmed
), you readid
, but you setorg
in the return URL. This can passnull
tovalidateOrganization
, breaking post‑payment validation.Apply this diff:
- if (type === 'payment_confirmed') { - const organizationId = page.url.searchParams.get('id'); - collaborators = page.url.searchParams.get('invites').split(','); + if (type === 'payment_confirmed') { + const organizationId = page.url.searchParams.get('org'); + const invitesParam = page.url.searchParams.get('invites'); + collaborators = invitesParam ? invitesParam.split(',') : []; await sdk.forConsole.billing.validateOrganization(organizationId, collaborators); }And consider guarding for missing
organizationId
with a user-visible error.Also applies to: 162-173
src/routes/(public)/auth/+layout.svelte (1)
1-7
: Fix compile error: add missingbase
import in src/routes/(public)/auth/+layout.svelte
base
is used in the template (img src) but not imported — add the import at the top of the script and re-run the check.Apply diff:
<script lang="ts"> import { app } from '$lib/stores/app'; import { loading } from '$routes/store'; import { Typography } from '@appwrite.io/pink-svelte'; + import { base } from '$app/paths'; loading.set(false); </script>
Re-run: pnpm run check.
🧹 Nitpick comments (5)
src/routes/(console)/organization-[organization]/billing/+page.ts (1)
8-8
: Base-path adoption LGTM; revisit 301 vs 302.Importing base and using it in the redirect is correct. Consider using 302 to avoid client/proxy caching a permissions-based redirect.
- return redirect(301, `${base}/organization-${organization.$id}`); + return redirect(302, `${base}/organization-${organization.$id}`);Also applies to: 14-14
src/routes/+layout.svelte (1)
166-175
: Nit: unify attribute expression style for consistency.Use the same pattern as the stylesheet lines to reduce cognitive load.
- <link rel="preload" as="style" type="text/css" href="{base}/fonts/main.css" /> + <link rel="preload" as="style" type="text/css" href={`${base}/fonts/main.css`} /> ... - <link rel="preload" as="style" type="text/css" href="{base}/fonts/cloud.css" /> + <link rel="preload" as="style" type="text/css" href={`${base}/fonts/cloud.css`} />src/routes/(console)/organization-[organization]/+page.ts (1)
7-7
: Base-path redirect LGTM; consider non-cacheable status.Import is correct. Prefer 302 to avoid caching logic-based redirects.
- return redirect(301, `${base}/organization-${params.organization}/billing`); + return redirect(302, `${base}/organization-${params.organization}/billing`);Also applies to: 12-13
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (1)
304-310
: Avoid injecting HTML via@html
for the “Usage details” link.Building an HTML string and checking
includes('<a href=')
is brittle. Prefer rendering a link element directly from structured data.Example (conceptual):
- item: `<a href="${base}/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`, + item: { + type: 'link', + href: `${base}/project-${String(project.region || 'default')}-${project.projectId}/settings/usage`, + label: 'Usage details' + },Then, in the cell renderer, branch on an object shape to render
<a>
without@html
.src/lib/components/sidebar.svelte (1)
138-161
: Guard progress-card link whenproject
is undefined.When
progressCard
exists butproject
hasn’t loaded, the href renders withundefined
segments.Apply this diff to gate rendering:
- {#if progressCard} + {#if progressCard && project} <Tooltip placement="right" disabled={state !== 'icons'}> <a class="progress-card" href={`${base}/project-${project?.region}-${project?.$id}/get-started`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
-
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
-
package.json
(1 hunks) -
src/lib/components/sidebar.svelte
(7 hunks) -
src/routes/(console)/apply-credit/+page.svelte
(1 hunks) -
src/routes/(console)/create-organization/+page.svelte
(1 hunks) -
src/routes/(console)/onboarding/create-project/+page.svelte
(1 hunks) -
src/routes/(console)/organization-[organization]/+page.ts
(1 hunks) -
src/routes/(console)/organization-[organization]/billing/+page.ts
(1 hunks) -
src/routes/(console)/organization-[organization]/billing/planSummary.svelte
(2 hunks) -
src/routes/(console)/organization-[organization]/domains/domain-[domain]/settings/changeOrganization.svelte
(2 hunks) -
src/routes/(public)/auth/+layout.svelte
(1 hunks) -
src/routes/(public)/template-[template]/+layout.svelte
(1 hunks) -
src/routes/+layout.svelte
(1 hunks) -
vercel.json
(0 hunks)
💤 Files with no reviewable changes (1)
- vercel.json
🧰 Additional context used
🧬 Code graph analysis (2)
src/routes/(console)/organization-[organization]/billing/+page.ts (3)
src/routes/(console)/organization-[organization]/+page.ts (1)
load
(9-43)src/routes/(console)/project-[region]-[project]/+page.ts (1)
load
(6-15)src/routes/(console)/project-[region]-[project]/overview/+page.ts (1)
load
(5-7)
src/routes/(console)/organization-[organization]/+page.ts (3)
src/routes/(console)/organization-[organization]/billing/+page.ts (1)
load
(10-89)src/routes/(console)/project-[region]-[project]/+page.ts (1)
load
(6-15)src/routes/(console)/project-[region]-[project]/overview/+page.ts (1)
load
(5-7)
🪛 GitHub Actions: Tests
src/routes/(public)/template-[template]/+layout.svelte
[error] 14-14: svelte-check error: Cannot find name 'base'. (ts). Step: 'pnpm run check' (svelte-kit sync && svelte-check).
[error] 20-20: svelte-check error: Cannot find name 'base'. (ts). Step: 'pnpm run check' (svelte-kit sync && svelte-check).
src/routes/(public)/auth/+layout.svelte
[error] 19-19: svelte-check error: Cannot find name 'base'. (ts). Step: 'pnpm run check' (svelte-kit sync && svelte-check).
[error] 25-25: svelte-check error: Cannot find name 'base'. (ts). Step: 'pnpm run check' (svelte-kit sync && svelte-check).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (6)
src/routes/(console)/create-organization/+page.svelte (1)
146-147
: Correct: payment confirmation redirect now base-aware.src/routes/(console)/onboarding/create-project/+page.svelte (1)
85-96
: LGTM: logos now resolve via base.Also applies to: 91-96
src/routes/(console)/organization-[organization]/domains/domain-[domain]/settings/changeOrganization.svelte (1)
3-3
: LGTM: navigation now respects base path.Also applies to: 24-25
src/routes/(console)/apply-credit/+page.svelte (1)
173-179
: Base-aware return_url for confirmPayment looks good.Using
${base}
prevents broken redirects under a non-root base path.Please confirm
confirmPayment
accepts a relative path and doesn’t require an absolute URL in some environments (e.g., Stripe-hosted 3DS redirect flows). If it needs absolute, construct vianew URL(
${base}/apply-credit?${params}, page.url)
or use the public origin from config.src/routes/(console)/organization-[organization]/billing/planSummary.svelte (1)
149-158
: Good rename to avoid shadowingbase
; return list updated accordingly.Renaming the local
base
object tobasePlan
prevents collisions with the importedbase
from$app/paths
.Also applies to: 315-315
src/lib/components/sidebar.svelte (1)
45-46
: Base-path updates across sidebar links look consistent.All internal hrefs correctly use
${base}
; import added. This prevents broken navigation under a non-root base path.Also applies to: 142-147, 166-180, 196-210, 227-251, 257-270, 317-328
</script> | ||
|
||
{#if show && $organization?.$id && $organization?.billingPlan === BillingPlan.FREE && !page.url.pathname.includes('/console/account')} | ||
{#if show && $organization?.$id && $organization?.billingPlan === BillingPlan.FREE && !page.url.pathname.includes(base + '/account')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
route.id
?
!page?.params.organization && | ||
!page.url.pathname.includes('/console/account') && | ||
!page.url.pathname.includes('/console/card') && | ||
!page.url.pathname.includes('/console/onboarding')} | ||
showHeader={!page.url.pathname.includes('/console/onboarding/create-project')} | ||
showFooter={!page.url.pathname.includes('/console/onboarding/create-project')} | ||
!page.url.pathname.includes(base + '/account') && | ||
!page.url.pathname.includes(base + '/card') && | ||
!page.url.pathname.includes(base + '/onboarding')} | ||
showHeader={!page.url.pathname.includes(base + '/onboarding/create-project')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe here as well but not a priority.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/routes/(console)/organization-[organization]/change-plan/+page.svelte (1)
272-276
: Guard against undefined collaborators before join()collaborators can be undefined; calling join() will throw during 3DS/confirm flow.
- params.append('invites', collaborators.join(',')); + params.append('invites', (collaborators ?? []).join(','));src/routes/(console)/apply-credit/+page.svelte (1)
170-174
: Prevent possible crash when collaborators is unsetjoin() on an undefined collaborators array will throw in the 402/3DS path.
- params.append('invites', collaborators.join(',')); + params.append('invites', (collaborators ?? []).join(','));src/routes/(console)/create-organization/+page.svelte (2)
88-92
: Bug: double “/console” in path (${base}/console/...
) breaks navigation.
base
already encodes the deployment subpath (e.g.,/console
). Appending another/console
yields/console/console/...
.- await preloadData(`${base}/console/organization-${org.$id}`); - await goto(`${base}/console/organization-${org.$id}`); + await preloadData(`${base}/organization-${org.$id}`); + await goto(`${base}/organization-${org.$id}`);
76-83
: Null‑safety:get('invites').split(',')
can throw when param is missing.Guard against
null
and filter empties.- const organizationId = page.url.searchParams.get('id'); - const invites = page.url.searchParams.get('invites').split(','); + const organizationId = page.url.searchParams.get('id'); + const invitesParam = page.url.searchParams.get('invites') ?? ''; + const invites = invitesParam ? invitesParam.split(',').filter(Boolean) : [];src/routes/(console)/organization-[organization]/billing/+page.ts (1)
24-29
: Type mismatch:billingAddressPromise
can benull
Variable is typed as
Promise<Address>
but is assignednull
and may resolve tonull
via.catch(() => null)
. This will trip type‑checking and is fragile inPromise.all
.Apply this diff:
- const billingAddressPromise: Promise<Address> = billingAddressId - ? sdk.forConsole.billing - .getOrganizationBillingAddress(organization.$id, billingAddressId) - .catch(() => null) - : null; + const billingAddressPromise: Promise<Address | null> = billingAddressId + ? sdk.forConsole.billing + .getOrganizationBillingAddress(organization.$id, billingAddressId) + .catch(() => null) + : Promise.resolve(null);src/routes/(console)/organization-[organization]/billing/planSummary.svelte (1)
321-324
: Fix NaN whenavailableCredit
isundefined
Math.min(..., availableCredit)
yieldsNaN
whenavailableCredit
isundefined
, which propagates tototalAmount
.Apply this diff:
- $: creditsApplied = Math.min( - currentAggregation?.amount ?? currentPlan?.price ?? 0, - availableCredit - ); + $: creditsApplied = Math.min( + currentAggregation?.amount ?? currentPlan?.price ?? 0, + availableCredit ?? 0 + );
🧹 Nitpick comments (6)
src/routes/(public)/template-[template]/+page.ts (1)
8-16
: Encode the redirect query value to avoid breaking the query stringIf url.search contains its own params, redirect= can be truncated. Encode the full path.
- redirect(302, base + '/login?redirect=' + url.pathname + url.search); + redirect(302, `${base}/login?redirect=${encodeURIComponent(url.pathname + url.search)}`);src/routes/+layout.svelte (1)
166-168
: Nit: unify attribute binding style for consistencyUse one style (template literal or mustache-in-string) for both preload links to match the stylesheet lines below.
- <link rel="preload" as="style" type="text/css" href="{base}/fonts/main.css" /> + <link rel="preload" as="style" type="text/css" href={`${base}/fonts/main.css`} /> ... - <link rel="preload" as="style" type="text/css" href="{base}/fonts/cloud.css" /> + <link rel="preload" as="style" type="text/css" href={`${base}/fonts/cloud.css`} />Also applies to: 172-175
src/routes/(public)/auth/+layout.svelte (1)
2-2
: Use$app/paths.assets
for static images (future CDN/asset-host compatibility).Assets may be served from a different host than
base
. Preferassets
for images.- import { base } from '$app/paths'; + import { base, assets } from '$app/paths'; @@ - src="{base}/images/appwrite-logo-dark.svg" + src="{assets}/images/appwrite-logo-dark.svg" @@ - src="{base}/images/appwrite-logo-light.svg" + src="{assets}/images/appwrite-logo-light.svg"Also applies to: 20-27
src/routes/(public)/template-[template]/+layout.svelte (1)
2-2
: Use$app/paths.assets
for static images.Same rationale as in auth layout.
- import { base } from '$app/paths'; + import { base, assets } from '$app/paths'; @@ - src="{base}/images/appwrite-logo-dark.svg" + src="{assets}/images/appwrite-logo-dark.svg" @@ - src="{base}/images/appwrite-logo-light.svg" + src="{assets}/images/appwrite-logo-light.svg"Also applies to: 15-22
src/lib/components/billing/alerts/newDevUpgradePro.svelte (1)
26-26
: Path check: preferstartsWith
overincludes
to avoid false positives.Avoid matching routes like
/accounting
.-{#if show && $organization?.$id && $organization?.billingPlan === BillingPlan.FREE && !page.url.pathname.includes(base + '/account')} +{#if show && $organization?.$id && $organization?.billingPlan === BillingPlan.FREE && !page.url.pathname.startsWith(`${base}/account`)}src/routes/(console)/organization-[organization]/billing/+page.ts (1)
14-14
: Use 302 (temporary) instead of 301 for auth/scope redirects301 can be cached by browsers/proxies and “stick” even after scopes change. Prefer 302 to avoid stale redirects.
Apply this diff:
- return redirect(301, `${base}/organization-${organization.$id}`); + return redirect(302, `${base}/organization-${organization.$id}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
package.json
(1 hunks)src/lib/components/billing/alerts/newDevUpgradePro.svelte
(1 hunks)src/lib/components/sidebar.svelte
(7 hunks)src/lib/stores/billing.ts
(1 hunks)src/routes/(console)/+layout.svelte
(2 hunks)src/routes/(console)/apply-credit/+page.svelte
(1 hunks)src/routes/(console)/create-organization/+page.svelte
(1 hunks)src/routes/(console)/onboarding/create-project/+page.svelte
(1 hunks)src/routes/(console)/organization-[organization]/+page.ts
(1 hunks)src/routes/(console)/organization-[organization]/billing/+page.ts
(1 hunks)src/routes/(console)/organization-[organization]/billing/planSummary.svelte
(2 hunks)src/routes/(console)/organization-[organization]/change-plan/+page.svelte
(1 hunks)src/routes/(console)/organization-[organization]/domains/domain-[domain]/settings/changeOrganization.svelte
(2 hunks)src/routes/(public)/(guest)/education/+page.svelte
(1 hunks)src/routes/(public)/auth/+layout.svelte
(2 hunks)src/routes/(public)/template-[template]/+layout.svelte
(2 hunks)src/routes/(public)/template-[template]/+page.ts
(1 hunks)src/routes/+layout.svelte
(1 hunks)vercel.json
(0 hunks)
💤 Files with no reviewable changes (1)
- vercel.json
🧰 Additional context used
🧬 Code graph analysis (3)
src/routes/(console)/organization-[organization]/+page.ts (3)
src/routes/(console)/organization-[organization]/billing/+page.ts (1)
load
(10-89)src/routes/(console)/project-[region]-[project]/+page.ts (1)
load
(6-15)src/routes/(console)/project-[region]-[project]/overview/+page.ts (1)
load
(5-7)
src/routes/(public)/template-[template]/+page.ts (1)
src/lib/system.ts (1)
isCloud
(24-24)
src/routes/(console)/organization-[organization]/billing/+page.ts (3)
src/routes/(console)/organization-[organization]/+page.ts (1)
load
(9-43)src/routes/(console)/project-[region]-[project]/+page.ts (1)
load
(6-15)src/routes/(console)/project-[region]-[project]/overview/+page.ts (1)
load
(5-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (16)
src/routes/(console)/onboarding/create-project/+page.svelte (1)
85-96
: LGTM — base-aware asset URLsSwitching logo src to {base}-prefixed paths is correct and consistent with the PR’s goal.
src/routes/(console)/organization-[organization]/change-plan/+page.svelte (1)
280-281
: LGTM — confirmPayment return_url uses baseUsing
${base}/change-plan?...
aligns with the new base-path strategy.src/routes/(console)/organization-[organization]/domains/domain-[domain]/settings/changeOrganization.svelte (1)
3-3
: LGTM — navigation respects base pathImporting base and using it in goto() keeps post-move navigation correct under non-root deployments.
Also applies to: 24-25
src/routes/(console)/organization-[organization]/+page.ts (1)
7-13
: LGTM — redirect updated to use baseThe redirect target now honors the configured base path.
src/routes/(console)/apply-credit/+page.svelte (1)
175-179
: LGTM — confirmPayment return_url now base-awareThe return URL uses
${base}
; implicit URLSearchParams toString() is fine.package.json (1)
57-57
: Confirm SvelteKit bump compatibility and finish '/console' auditpackage.json contains @sveltejs/kit ^2.42.1; svelte ^5.25.3; vite ^7.0.6; @sveltejs/adapter-static ^3.0.8; @sentry/sveltekit ^8.38.0 — versions present. Repo-wide search for literal '/console' was inconclusive due to grep/xargs errors; many route files live under src/routes/(console)/ (expected). Re-run these checks to complete the audit:
# find literal '/console' (exclude node_modules and .svelte_kit) rg -n --hidden -S --type-add 'svelte:*.svelte' --glob '!node_modules/**' --glob '!.svelte_kit/**' --fixed-strings '/console' || true # confirmPayment calls that pass '/console' as an arg rg -n --hidden -S --type-add 'svelte:*.svelte' --glob '!node_modules/**' --glob '!.svelte_kit/**' -E "confirmPayment\([^)]*['\"]/console" -C2 || true # goto/redirect calls with '/console' literal rg -n --hidden -S --type-add 'svelte:*.svelte' --glob '!node_modules/**' --glob '!.svelte_kit/**' -E "(goto|redirect)\s*\(\s*['\"]/console" -C1 || true # return_url / returnUrl containing '/console' rg -n --hidden -S --type-add 'svelte:*.svelte' --glob '!node_modules/**' --glob '!.svelte_kit/**' -E "(return[_ ]?url|returnUrl).*['\"]/console" -C2 || true # local 'base' shadowing and imports from $app/paths rg -n --hidden -S --type-add 'svelte:*.svelte' --glob '!node_modules/**' --glob '!.svelte_kit/**' -E '^\s*(let|const)\s+base\s*=' || true rg -n --hidden -S --type-add 'svelte:*.svelte' --glob '!node_modules/**' --glob '!.svelte_kit/**' -E "import\s*\{[^}]*\bbase\b[^}]*\}\s*from\s*['\"]\\\$app/paths['\"]" || truesrc/routes/(public)/(guest)/education/+page.svelte (1)
10-18
: LGTM: base-aware OAuth2 redirects.The success/failure redirects correctly incorporate
base
.src/routes/(console)/create-organization/+page.svelte (2)
146-147
: LGTM: payment confirmation redirect now base-aware.Redirect target is correct and consistent with other routes.
1-175
: Repo-wide sweep for hard-coded/console
paths — verify locally.Stray
${base}/console/...
exists in src/routes/(console)/create-organization/+page.svelte; automated sandbox search was inconclusive (rg skipped files / regex errors). Run locally to confirm no other occurrences:
rg -n --hidden --no-ignore-vcs -g '!node_modules/' -g '!dist/' '/console' src || truesrc/lib/stores/billing.ts (1)
623-623
: LGTM: hide‑header routes now base‑aware.Constant looks correct and consistent with other path updates.
src/routes/(console)/+layout.svelte (2)
85-89
: LGTM: command disabled condition is base‑aware.Condition reads clearly and maintains prior behavior.
339-343
: LGTM: layout visibility flags updated tobase
paths.Side nav, header, footer checks are consistent.
src/lib/components/sidebar.svelte (1)
142-147
: LGTM: all project routes correctly prefixed withbase
.Navigation will work under subpath deployments.
Also applies to: 166-170, 196-203, 227-249, 257-269, 317-327
src/routes/(console)/organization-[organization]/billing/+page.ts (1)
8-8
: Base path adoption — LGTMImporting
base
from$app/paths
makes this route base‑aware and consistent with the rest of the PR.src/routes/(console)/organization-[organization]/billing/planSummary.svelte (2)
149-159
: Rename tobasePlan
— LGTMAvoids shadowing the imported
base
and clarifies intent.
315-315
: Return list — LGTMUpdating to
[basePlan, ...addons, ...projects, ...noProjects]
matches the rename and order.
id: `usage-details`, | ||
cells: { | ||
item: `<a href="/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`, | ||
item: `<a href="${base}/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`, | ||
usage: '', | ||
price: '' | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Avoid raw {@html} injection for link rendering (XSS risk)
Building an <a>
as a string and rendering it via {@html}
bypasses Svelte escaping. If any interpolated value is unexpected, this becomes an injection vector. Render links in markup instead and encode path segments.
Apply this diff for the row data to carry a typed link (keeps change localized to this block):
{
- id: `usage-details`,
- cells: {
- item: `<a href="${base}/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`,
- usage: '',
- price: ''
- }
+ id: `usage-details`,
+ link: {
+ href: `${base}/project-${encodeURIComponent(String(project.region || 'default'))}-${encodeURIComponent(String(project.projectId))}/settings/usage`,
+ label: 'Usage details'
+ },
+ cells: {
+ item: 'Usage details',
+ usage: '',
+ price: ''
+ }
}
Then replace the {@html ...}
branch to render the anchor safely (remove the <a href>
string check entirely):
{#if col.id === 'item' && child.id === 'usage-details' && child.link}
<a href={child.link.href} style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">
{child.link.label}
</a>
{:else if col.id === 'usage'}
<!-- existing usage rendering -->
{:else}
<Typography.Text variant="m-400" color="--fgcolor-neutral-secondary">
{child.cells?.[col.id] ?? ''}
</Typography.Text>
{/if}
Also applies to: 393-396
🏁 Script executed:
#!/bin/bash
# Find other risky raw HTML injections and string-built anchors.
rg -n --type=svelte -C2 '\{@html' src
rg -n --type=svelte -C2 '<a href="\$\{base\}[^"]*\$\{' src
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Search for raw HTML injections in Svelte files
rg -n '\{@html' -g '*.svelte' -C2 src
# Search for string-built anchors referencing base in Svelte files
rg -n '<a href="\$\{base\}[^"]*\$\{' -g '*.svelte' -C2 src
Length of output: 8559
Avoid raw {@html} injection for link rendering (XSS risk)
Don’t build anchor tags as strings and inject via {@html}; encode path segments and render the in markup. Fix the row data and the template branch below.
File: src/routes/(console)/organization-[organization]/billing/planSummary.svelte (rows ~304–310; template ~392–396)
Apply this diff for the row data:
{
- id: `usage-details`,
- cells: {
- item: `<a href="${base}/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`,
- usage: '',
- price: ''
- }
+ id: `usage-details`,
+ link: {
+ href: `${base}/project-${encodeURIComponent(String(project.region || 'default'))}-${encodeURIComponent(String(project.projectId))}/settings/usage`,
+ label: 'Usage details'
+ },
+ cells: {
+ item: 'Usage details',
+ usage: '',
+ price: ''
+ }
}
Replace the {@html} branch with a safe anchor render (remove the <a href>
string check):
{#if col.id === 'item' && child.id === 'usage-details' && child.link}
<a href={child.link.href} style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">
{child.link.label}
</a>
{:else if col.id === 'usage'}
<!-- existing usage rendering -->
{:else}
<Typography.Text variant="m-400" color="--fgcolor-neutral-secondary">
{child.cells?.[col.id] ?? ''}
</Typography.Text>
{/if}
Audit other raw {@html} usages found by the search and confirm they’re intentionally sanitized (examples found: src/routes/.../configuration.svelte, settingsFormInput.svelte, popoverContent.svelte, create.svelte, src/lib/components/bottomModalAlert.svelte, billing/alerts/selectProjectCloud.svelte, csvImportBox.svelte, commandCenter/panels/ai.svelte).
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/billing/planSummary.svelte
around lines 304–310 and the template branch near 392–396, you are currently
injecting a raw anchor tag string into the row cell (rendered via {@html}) which
is an XSS risk; change the row data for the usage-details row to provide a
structured link object (e.g., child.link = { href: base + '/project-' +
encodeURIComponent(String(project.region || 'default')) + '-' +
encodeURIComponent(project.projectId) + '/settings/usage', label: 'Usage
details' }) instead of an HTML string, then replace the {@html} branch in the
template with a safe conditional that renders a real <a> element using
child.link.href and child.link.label (no raw HTML), removing the string-check
branch and keeping the existing usage/other branches; also audit other files
listed in the comment for raw {@html} usage and ensure any remaining instances
are intentionally sanitized or similarly refactored to structured/link-safe
rendering.
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
Bug Fixes
Chores