From a7eae3ff81143f25d0e4d92cbb1a9c6dc9902dee Mon Sep 17 00:00:00 2001 From: ajnart Date: Tue, 28 Jul 2026 15:28:58 +0200 Subject: [PATCH] fix v2 hardening review findings --- .github/workflows/deployment-docs.yml | 2 + .../advanced/environment-variables/index.mdx | 15 +- apps/docs/src/pages/workshop/admin/index.tsx | 5 +- .../configuration-request/[token]/route.ts | 91 ++------ apps/workshop/entrypoint.sh | 39 ++-- .../pb_hooks/widget-validator.bundle.cjs | 68 ++++-- apps/workshop/pb_hooks/workshop.pb.js | 10 + .../1784240001_workshop_write_rate_limits.js | 52 ++++- .../1784240002_workshop_access_hardening.js | 1 - ...kshop_rate_limit_rollback_compatibility.js | 61 ++++++ apps/workshop/test.sh | 1 + apps/workshop/tests/entrypoint-url.mjs | 23 ++ apps/workshop/tests/workshop-contracts.mjs | 29 ++- apps/workshop/tests/workshop-oauth-hook.mjs | 61 +++++- .../tests/workshop-rate-limit-migration.mjs | 205 ++++++++++++++++++ apps/workshop/tests/workshop.integration.mjs | 26 +++ apps/workshop/workshop-url.awk | 5 +- e2e/custom-widgets-custom-jsx-v2.spec.ts | 32 +-- e2e/onboarding.spec.ts | 91 ++++---- e2e/shared/actions/onboarding-actions.ts | 2 +- e2e/shared/mock-api-server.ts | 25 ++- .../api/src/custom-widget-configuration.ts | 1 + .../custom-widget/secret-persistence.ts | 190 +++++++++++----- .../router/custom-widget/secret-procedures.ts | 12 +- .../definition-persistence.spec.ts | 98 +++++++++ .../custom-widgets/src/core/url-policy.ts | 8 + .../server/request-dispatcher-lifecycle.ts | 7 +- .../src/test/request-deadline.spec.ts | 25 ++- .../custom-widgets/src/test/schema.spec.ts | 2 + packages/definitions/src/test/docs.spec.ts | 10 +- .../custom-jsx-display-refresh.spec.tsx | 139 ++++++++++++ .../src/custom-api/custom-jsx-display.tsx | 2 +- packages/workshop/src/pocketbase-url.spec.ts | 8 + packages/workshop/src/pocketbase-url.ts | 77 +++++-- packages/workshop/src/schema.spec.ts | 1 + 35 files changed, 1147 insertions(+), 277 deletions(-) create mode 100644 apps/workshop/pb_migrations/1784240003_workshop_rate_limit_rollback_compatibility.js create mode 100644 apps/workshop/tests/workshop-rate-limit-migration.mjs create mode 100644 packages/widgets/src/custom-api/custom-jsx-display-refresh.spec.tsx diff --git a/.github/workflows/deployment-docs.yml b/.github/workflows/deployment-docs.yml index d67047c9b..7ce023e52 100644 --- a/.github/workflows/deployment-docs.yml +++ b/.github/workflows/deployment-docs.yml @@ -45,6 +45,8 @@ jobs: digest: ${{ steps.build.outputs.digest }} steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - name: Set up QEMU uses: docker/setup-qemu-action@v4 diff --git a/apps/docs/docs/advanced/environment-variables/index.mdx b/apps/docs/docs/advanced/environment-variables/index.mdx index 2f682f09d..9cebbefa9 100644 --- a/apps/docs/docs/advanced/environment-variables/index.mdx +++ b/apps/docs/docs/advanced/environment-variables/index.mdx @@ -110,13 +110,14 @@ External redis is currently not supported with Proxmox Community Scripts The advanced deployments environment variables should only be used if you know what you are doing. ::: -| Environment Variable | Description | -| ------------------------ | ----------------------------------------------------------------------------------------------------------------------------------- | -| `DB_MIGRATIONS_DISABLED` | Disable db migrations. For example for helm charts. | -| `DOCS_BASE_URL` | Docusaurus deployment path. It must start with `/` and is normalized with a trailing slash. Default: `/`. | -| `WORKSHOP_PUBLIC_ORIGIN` | Origin used in PocketBase-generated links and email. It must be set explicitly in production. | -| `PB_ALLOWED_ORIGINS` | Browser origins accepted by PocketBase. The central public community defaults to `*` so self-hosted Homarr instances can use OAuth. | -| `WORKSHOP_REQUIRE_OAUTH` | Fail Workshop production readiness when GitHub OAuth is disabled. Both GitHub OAuth credentials are always required together. | +| Environment Variable | Description | +| -------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | +| `DB_MIGRATIONS_DISABLED` | Disable db migrations. For example for helm charts. | +| `DOCS_BASE_URL` | Docusaurus deployment path. It must start with `/` and is normalized with a trailing slash. Default: `/`. | +| `WORKSHOP_PUBLIC_ORIGIN` | Origin used in PocketBase-generated links and email. It must be set explicitly in production. | +| `WORKSHOP_REQUIRE_PUBLIC_ORIGIN` | Require `WORKSHOP_PUBLIC_ORIGIN` at startup. The production Workshop image enables this automatically. | +| `PB_ALLOWED_ORIGINS` | Browser origins accepted by PocketBase. The central public community defaults to `*` so self-hosted Homarr instances can use OAuth. | +| `WORKSHOP_REQUIRE_OAUTH` | Fail Workshop production readiness when GitHub OAuth is disabled. Both GitHub OAuth credentials are always required together. | All public URL variables reject non-HTTP protocols, embedded credentials, query strings, and fragments, and normalize trailing slashes at startup. The Docusaurus-only `WORKSHOP_URL` name is accepted as a deprecated alias for diff --git a/apps/docs/src/pages/workshop/admin/index.tsx b/apps/docs/src/pages/workshop/admin/index.tsx index 10244048a..c9680cf4f 100644 --- a/apps/docs/src/pages/workshop/admin/index.tsx +++ b/apps/docs/src/pages/workshop/admin/index.tsx @@ -1,4 +1,5 @@ import React from "react"; +import BrowserOnly from "@docusaurus/BrowserOnly"; import useDocusaurusContext from "@docusaurus/useDocusaurusContext"; import Layout from "@theme/Layout"; @@ -10,7 +11,9 @@ export default function WorkshopAdminRoute() { const workshopUrl = (siteConfig.customFields?.workshopUrl as string | undefined) ?? ""; return ( - + + {() => } + ); } diff --git a/apps/nextjs/src/app/api/custom-widgets/configuration-request/[token]/route.ts b/apps/nextjs/src/app/api/custom-widgets/configuration-request/[token]/route.ts index 5ea26b7d0..7ca7a4fe3 100644 --- a/apps/nextjs/src/app/api/custom-widgets/configuration-request/[token]/route.ts +++ b/apps/nextjs/src/app/api/custom-widgets/configuration-request/[token]/route.ts @@ -4,17 +4,14 @@ import { NextResponse } from "next/server"; import { claimCustomWidgetConfigurationRequest, completeCustomWidgetConfigurationRequest, + configureCustomWidgetSourceFromRequest, configurePreviewSessionSource, getCustomWidgetConfigurationRequest, invalidateCustomWidgetResponseCache, - parseStoredCustomWidgetDefinition, releaseCustomWidgetConfigurationRequest, - serializeCustomWidgetDefinition, } from "@homarr/api/custom-widget-configuration"; -import { encryptSecret } from "@homarr/common/server"; -import { customWidgetSourceSchema, hasSameCustomWidgetSourceAuthentication } from "@homarr/custom-widgets/core"; -import { and, db, eq, handleTransactionsAsync } from "@homarr/db"; -import { customWidgetDefinitions, customWidgetSecrets } from "@homarr/db/schema"; +import { customWidgetSourceSchema } from "@homarr/custom-widgets/core"; +import { db } from "@homarr/db"; import { requireCustomWidgetAdmin } from "../../admin"; import { readConfigurationRequestBody } from "../body"; @@ -99,19 +96,24 @@ export async function POST(request: NextRequest, context: RouteContext) { ); await invalidateCustomWidgetResponseCache([`custom-jsx:preview:${claimed.target.id}:`]); } else { - const definition = await db.query.customWidgetDefinitions.findFirst({ - where: eq(customWidgetDefinitions.id, claimed.target.id), + const result = await configureCustomWidgetSourceFromRequest(db, { + definitionId: claimed.target.id, + sourceId: claimed.sourceId, + baseUrl: sourceResult.data.baseUrl, + networkScope: sourceResult.data.networkScope, + secrets, + expectedSource: claimed.source, }); - if (!definition) return NextResponse.json({ error: "The custom widget no longer exists." }, { status: 404 }); - const parsed = parseStoredCustomWidgetDefinition(definition); - const currentSource = parsed.sources[claimed.sourceId]; - if (!currentSource) { + if (result.status === "definition-not-found") { + return NextResponse.json({ error: "The custom widget no longer exists." }, { status: 404 }); + } + if (result.status === "source-not-found") { return NextResponse.json( { error: "This API source changed after the setup link was created. Request a new setup link." }, { status: 409 }, ); } - if (!hasSameCustomWidgetSourceAuthentication(currentSource, claimed.source)) { + if (result.status === "binding-changed") { return NextResponse.json( { error: "This API source authentication changed after the setup link was created. Request a new setup link.", @@ -119,69 +121,6 @@ export async function POST(request: NextRequest, context: RouteContext) { { status: 409 }, ); } - const configuredSource = customWidgetSourceSchema.parse({ - ...currentSource, - baseUrl: sourceResult.data.baseUrl, - networkScope: sourceResult.data.networkScope, - }); - const definitionChanges = { - ...serializeCustomWidgetDefinition({ - ...parsed, - sources: { ...parsed.sources, [claimed.sourceId]: configuredSource }, - }), - updatedAt: new Date(), - }; - const secretRows = secrets.map((secret) => ({ - definitionId: claimed.target.id, - sourceId: secret.sourceId, - kind: secret.kind, - encryptedValue: encryptSecret(secret.value), - updatedAt: new Date(), - })); - await handleTransactionsAsync(db, { - async handleAsync(database, schema) { - await database.transaction(async (transaction) => { - await transaction - .update(schema.customWidgetDefinitions) - .set(definitionChanges) - .where(eq(schema.customWidgetDefinitions.id, claimed.target.id)); - for (const secret of secretRows) { - await transaction - .delete(schema.customWidgetSecrets) - .where( - and( - eq(schema.customWidgetSecrets.definitionId, claimed.target.id), - eq(schema.customWidgetSecrets.sourceId, secret.sourceId), - eq(schema.customWidgetSecrets.kind, secret.kind), - ), - ); - await transaction.insert(schema.customWidgetSecrets).values(secret); - } - }); - }, - handleSync(database) { - database.transaction((transaction) => { - transaction - .update(customWidgetDefinitions) - .set(definitionChanges) - .where(eq(customWidgetDefinitions.id, claimed.target.id)) - .run(); - for (const secret of secretRows) { - transaction - .delete(customWidgetSecrets) - .where( - and( - eq(customWidgetSecrets.definitionId, claimed.target.id), - eq(customWidgetSecrets.sourceId, secret.sourceId), - eq(customWidgetSecrets.kind, secret.kind), - ), - ) - .run(); - transaction.insert(customWidgetSecrets).values(secret).run(); - } - }); - }, - }); } await completeCustomWidgetConfigurationRequest(token); diff --git a/apps/workshop/entrypoint.sh b/apps/workshop/entrypoint.sh index b45b64ad8..6f9b7913f 100644 --- a/apps/workshop/entrypoint.sh +++ b/apps/workshop/entrypoint.sh @@ -9,11 +9,34 @@ normalize_http_url() { -f "${WORKSHOP_URL_VALIDATOR_PATH:-/usr/local/lib/workshop-url.awk}" } +normalize_docs_base_url() { + value=$1 + case "$value" in + /*) ;; + *) + echo "DOCS_BASE_URL must start with '/'" >&2 + return 1 + ;; + esac + case "${value%/}/" in + *[!A-Za-z0-9._~/-]* | */./* | */../*) + echo "DOCS_BASE_URL must be a safe URL path without traversal" >&2 + return 1 + ;; + esac + printf '/%s\n' "$(printf '%s' "$value" | sed 's#^/*##; s#/*$##')" +} + if [ "${1:-}" = "--validate-url" ]; then normalize_http_url "${2:-URL}" "${3:-}" exit fi +if [ "${1:-}" = "--validate-docs-base-url" ]; then + normalize_docs_base_url "${2:-}" + exit +fi + HOMARR_WEBSITE_URL=$(normalize_http_url HOMARR_WEBSITE_URL "${HOMARR_WEBSITE_URL:-https://homarr.dev}") WORKSHOP_API_URL=$(normalize_http_url WORKSHOP_API_URL "${WORKSHOP_API_URL:-$HOMARR_WEBSITE_URL}") WORKSHOP_WEB_URL=$(normalize_http_url WORKSHOP_WEB_URL "${WORKSHOP_WEB_URL:-$HOMARR_WEBSITE_URL/workshop}") @@ -32,21 +55,7 @@ if [ "$(id -u)" -eq 0 ]; then fi if [ -d /pb_public ]; then - docs_base_url=${DOCS_BASE_URL:-/} - case "$docs_base_url" in - /*) ;; - *) - echo "DOCS_BASE_URL must start with '/'" >&2 - exit 1 - ;; - esac - case "$docs_base_url" in - *[!A-Za-z0-9._~/-]* | *".."*) - echo "DOCS_BASE_URL must be a safe URL path without traversal" >&2 - exit 1 - ;; - esac - docs_base_url="/$(printf '%s' "$docs_base_url" | sed 's#^/*##; s#/*$##')" + docs_base_url=$(normalize_docs_base_url "${DOCS_BASE_URL:-/}") runtime_directory="/pb_public${docs_base_url%/}" mkdir -p "$runtime_directory" cat >"$runtime_directory/workshop-runtime-config.js" <= 127 && code <= 160 || code === 5760 || code >= 8192 && code <= 8207 || code >= 8232 && code <= 8239 || code >= 8287 && code <= 8303 || code === 12288 || code === 65279; }); } -function isValidIpv6(value) { - if (value.includes("%")) return false; +function normalizeIpv6(value) { + if (value.includes("%")) return null; const address = value; - if (!address.includes(":") || address.includes(":::") || /[^0-9A-Fa-f:.]/u.test(address)) return false; - if (address.indexOf("::") !== address.lastIndexOf("::")) return false; - const groups = address.split(":").filter(Boolean); - let groupCount = groups.length; + if (!address.includes(":") || address.includes(":::") || /[^0-9A-Fa-f:.]/u.test(address)) return null; + if (address.indexOf("::") !== address.lastIndexOf("::")) return null; + if (address.startsWith(":") && !address.startsWith("::") || address.endsWith(":") && !address.endsWith("::")) { + return null; + } + const hasCompression = address.includes("::"); + const [left = "", right = ""] = hasCompression ? address.split("::") : [address, ""]; + if (hasCompression && left.includes(".")) return null; + const leftGroups = parseIpv6Groups(left); + const rightGroups = parseIpv6Groups(right); + if (leftGroups === null || rightGroups === null) return null; + const explicitGroupCount = leftGroups.length + rightGroups.length; + if (!hasCompression && explicitGroupCount !== 8 || hasCompression && explicitGroupCount >= 8) return null; + const groups = hasCompression ? [...leftGroups, ...Array.from({ length: 8 - explicitGroupCount }, () => 0), ...rightGroups] : leftGroups; + let bestStart = -1; + let bestLength = 1; + for (let index = 0; index < groups.length; ) { + if (groups[index] !== 0) { + index += 1; + continue; + } + let end = index + 1; + while (end < groups.length && groups[end] === 0) end += 1; + if (end - index > bestLength) { + bestStart = index; + bestLength = end - index; + } + index = end; + } + const hexadecimalGroups = groups.map((group) => group.toString(16)); + if (bestStart < 0) return hexadecimalGroups.join(":"); + const before = hexadecimalGroups.slice(0, bestStart).join(":"); + const after = hexadecimalGroups.slice(bestStart + bestLength).join(":"); + return `${before}::${after}`; +} +function parseIpv6Groups(value) { + var _a5, _b, _c, _d; + if (!value) return []; + const groups = value.split(":"); + const parsed = []; for (const [index, group] of groups.entries()) { if (group.includes(".")) { - if (index !== groups.length - 1 || !isStrictIpv42(group)) return false; - groupCount += 1; - } else if (!/^[0-9A-Fa-f]{1,4}$/u.test(group)) { - return false; + if (index !== groups.length - 1 || !isStrictIpv42(group)) return null; + const parts = group.split(".").map(Number); + parsed.push(((_a5 = parts[0]) != null ? _a5 : 0) << 8 | ((_b = parts[1]) != null ? _b : 0), ((_c = parts[2]) != null ? _c : 0) << 8 | ((_d = parts[3]) != null ? _d : 0)); + } else { + if (!/^[0-9A-Fa-f]{1,4}$/u.test(group)) return null; + parsed.push(Number.parseInt(group, 16)); } } - return address.includes("::") ? groupCount < 8 : groupCount === 8; + return parsed; } function isStrictIpv42(value) { const parts = value.split("."); diff --git a/apps/workshop/pb_hooks/workshop.pb.js b/apps/workshop/pb_hooks/workshop.pb.js index 35f60534d..f0bcabc28 100644 --- a/apps/workshop/pb_hooks/workshop.pb.js +++ b/apps/workshop/pb_hooks/workshop.pb.js @@ -57,6 +57,16 @@ onRecordAuthWithOAuth2Request((event) => { event.next(); }, "users"); +onRecordUpdateRequest((event) => { + const { rejectRequest } = require(`${__hooks}/workshop-utils.js`); + const originalAvatar = event.record.original().getString("avatar"); + const pendingAvatarFiles = event.record.getUnsavedFiles("avatar"); + if (event.record.getString("avatar") !== originalAvatar || pendingAvatarFiles.length > 0) { + rejectRequest("Workshop avatars are managed by GitHub OAuth"); + } + event.next(); +}, "users"); + onRecordCreateRequest((event) => { try { const { validateAndNormalizeSubmission } = require(`${__hooks}/workshop-utils.js`); diff --git a/apps/workshop/pb_migrations/1784240001_workshop_write_rate_limits.js b/apps/workshop/pb_migrations/1784240001_workshop_write_rate_limits.js index f5ea9431e..970235125 100644 --- a/apps/workshop/pb_migrations/1784240001_workshop_write_rate_limits.js +++ b/apps/workshop/pb_migrations/1784240001_workshop_write_rate_limits.js @@ -15,12 +15,50 @@ const managedLabels = [ "reports:delete", "users:update", ]; +const legacyCreateRules = [ + { label: "submissions:create", audience: "@auth", duration: 60, maxRequests: 10 }, + { label: "votes:create", audience: "@auth", duration: 10, maxRequests: 20 }, + { label: "comments:create", audience: "@auth", duration: 60, maxRequests: 20 }, + { label: "reports:create", audience: "@auth", duration: 60, maxRequests: 5 }, +]; +const legacyCreateLabels = legacyCreateRules.map((rule) => rule.label); +const cloneJson = (value) => JSON.parse(JSON.stringify(value)); const withoutManagedRules = (rules) => rules.filter((rule) => !managedLabels.includes(rule.label)); +const managedRules = (rules) => rules.filter((rule) => managedLabels.includes(rule.label)); +const deriveLegacyWriteRateLimitState = (snapshot) => ({ + enabled: true, + managedRules: managedRules([ + ...snapshot.rateLimits.rules.filter((rule) => !legacyCreateLabels.includes(rule.label)), + ...legacyCreateRules, + ]), +}); + +const findMigrationState = (app) => { + try { + const collection = app.findCollectionByNameOrId("workshop_migration_state"); + const record = app.findFirstRecordByFilter(collection.id, ""); + const snapshot = JSON.parse(record.getString("snapshot")); + if (!snapshot || typeof snapshot !== "object" || Array.isArray(snapshot)) { + throw new Error("Workshop migration state snapshot is invalid"); + } + return { record, snapshot }; + } catch { + throw new Error("Workshop write rate-limit migration state is unavailable"); + } +}; migrate( (app) => { const settings = app.settings(); + const state = findMigrationState(app); + state.snapshot.writeRateLimits = { + enabled: settings.rateLimits.enabled, + managedRules: cloneJson(managedRules(settings.rateLimits.rules)), + }; + state.record.set("snapshot", JSON.stringify(state.snapshot)); + app.save(state.record); + settings.rateLimits.enabled = true; settings.rateLimits.rules = [ ...withoutManagedRules(settings.rateLimits.rules), @@ -42,13 +80,13 @@ migrate( }, (app) => { const settings = app.settings(); - settings.rateLimits.rules = [ - ...withoutManagedRules(settings.rateLimits.rules), - { label: "submissions:create", audience: "@auth", duration: 60, maxRequests: 10 }, - { label: "votes:create", audience: "@auth", duration: 10, maxRequests: 20 }, - { label: "comments:create", audience: "@auth", duration: 60, maxRequests: 20 }, - { label: "reports:create", audience: "@auth", duration: 60, maxRequests: 5 }, - ]; + const snapshot = findMigrationState(app).snapshot; + const state = snapshot.writeRateLimits || deriveLegacyWriteRateLimitState(snapshot); + if (!state || typeof state.enabled !== "boolean" || !Array.isArray(state.managedRules)) { + throw new Error("Workshop write rate-limit rollback snapshot is missing or invalid"); + } + settings.rateLimits.enabled = state.enabled; + settings.rateLimits.rules = [...withoutManagedRules(settings.rateLimits.rules), ...cloneJson(state.managedRules)]; app.save(settings); }, ); diff --git a/apps/workshop/pb_migrations/1784240002_workshop_access_hardening.js b/apps/workshop/pb_migrations/1784240002_workshop_access_hardening.js index 8030d9a55..507f494e2 100644 --- a/apps/workshop/pb_migrations/1784240002_workshop_access_hardening.js +++ b/apps/workshop/pb_migrations/1784240002_workshop_access_hardening.js @@ -9,7 +9,6 @@ const legacyReportUpdateRule = const identityFieldsUnchanged = [ "@request.body.email:changed = false", "@request.body.displayName:changed = false", - "@request.body.avatar:changed = false", "@request.body.avatarUrl:changed = false", "@request.body.githubUsername:changed = false", "@request.body.githubProfileUrl:changed = false", diff --git a/apps/workshop/pb_migrations/1784240003_workshop_rate_limit_rollback_compatibility.js b/apps/workshop/pb_migrations/1784240003_workshop_rate_limit_rollback_compatibility.js new file mode 100644 index 000000000..e551fd0ab --- /dev/null +++ b/apps/workshop/pb_migrations/1784240003_workshop_rate_limit_rollback_compatibility.js @@ -0,0 +1,61 @@ +/// + +const managedLabels = [ + "submissions:create", + "submissions:update", + "submissions:delete", + "votes:create", + "votes:update", + "votes:delete", + "comments:create", + "comments:update", + "comments:delete", + "reports:create", + "reports:update", + "reports:delete", + "users:update", +]; +const legacyCreateRules = [ + { label: "submissions:create", audience: "@auth", duration: 60, maxRequests: 10 }, + { label: "votes:create", audience: "@auth", duration: 10, maxRequests: 20 }, + { label: "comments:create", audience: "@auth", duration: 60, maxRequests: 20 }, + { label: "reports:create", audience: "@auth", duration: 60, maxRequests: 5 }, +]; +const legacyCreateLabels = legacyCreateRules.map((rule) => rule.label); +const managedRules = (rules) => rules.filter((rule) => managedLabels.includes(rule.label)); + +const findMigrationState = (app) => { + try { + const collection = app.findCollectionByNameOrId("workshop_migration_state"); + const record = app.findFirstRecordByFilter(collection.id, ""); + const snapshot = JSON.parse(record.getString("snapshot")); + if (!snapshot || typeof snapshot !== "object" || Array.isArray(snapshot)) { + throw new Error("Workshop migration state snapshot is invalid"); + } + return { record, snapshot }; + } catch { + throw new Error("Workshop rate-limit compatibility state is unavailable"); + } +}; + +migrate( + (app) => { + const state = findMigrationState(app); + if (state.snapshot.writeRateLimits) return; + if (!state.snapshot.rateLimits || !Array.isArray(state.snapshot.rateLimits.rules)) { + throw new Error("Workshop legacy rate-limit snapshot is missing or invalid"); + } + state.snapshot.writeRateLimits = { + enabled: true, + managedRules: managedRules([ + ...state.snapshot.rateLimits.rules.filter((rule) => !legacyCreateLabels.includes(rule.label)), + ...legacyCreateRules, + ]), + }; + state.record.set("snapshot", JSON.stringify(state.snapshot)); + app.save(state.record); + }, + () => { + // Keep the backfill until 1784240001 is rolled back; 1784240000 removes the state collection afterward. + }, +); diff --git a/apps/workshop/test.sh b/apps/workshop/test.sh index b46fed67f..0aa6a8f23 100644 --- a/apps/workshop/test.sh +++ b/apps/workshop/test.sh @@ -13,6 +13,7 @@ ROLLBACK_CONTAINER="" node apps/workshop/tests/workshop-contracts.mjs node apps/workshop/tests/workshop-migration.mjs +node apps/workshop/tests/workshop-rate-limit-migration.mjs node apps/workshop/tests/entrypoint-url.mjs node apps/workshop/tests/workshop-identity.mjs node apps/workshop/tests/workshop-oauth-hook.mjs diff --git a/apps/workshop/tests/entrypoint-url.mjs b/apps/workshop/tests/entrypoint-url.mjs index c7f749d80..ff434a370 100644 --- a/apps/workshop/tests/entrypoint-url.mjs +++ b/apps/workshop/tests/entrypoint-url.mjs @@ -10,6 +10,11 @@ const validate = (value) => env: { ...process.env, WORKSHOP_URL_VALIDATOR_PATH: validator }, }); +const validateDocsBaseUrl = (value) => + spawnSync("sh", [entrypoint, "--validate-docs-base-url", value], { + encoding: "utf8", + }); + for (const [value, expected] of [ ["http://127.0.0.1:8090/", "http://127.0.0.1:8090"], ["https://example.com/docs/", "https://example.com/docs"], @@ -30,6 +35,8 @@ for (const value of [ "https://example.com:65536", "https://::1", "https://[:::1]", + "http://[:1:2:3:4:5:6:7:8]", + "http://[1:2:3:4:5:6:7:8:]", "https://1.2.3.4.5", "http://09.0.0.1", "http://099.0.0.1", @@ -46,4 +53,20 @@ for (const value of [ if (result.status === 0) throw new Error(`Expected invalid runtime URL to be rejected: ${JSON.stringify(value)}`); } +for (const [value, expected] of [ + ["/", "/"], + ["/docs/", "/docs"], + ["/docs..preview", "/docs..preview"], +]) { + const result = validateDocsBaseUrl(value); + if (result.status !== 0 || result.stdout.trim() !== expected) { + throw new Error(`Expected valid DOCS_BASE_URL ${value}: ${result.stderr || result.stdout}`); + } +} + +for (const value of ["docs", "/../docs", "/docs/../private", "/./docs", "/docs?preview=1"]) { + const result = validateDocsBaseUrl(value); + if (result.status === 0) throw new Error(`Expected invalid DOCS_BASE_URL to be rejected: ${JSON.stringify(value)}`); +} + console.log("Workshop runtime URL validation passed"); diff --git a/apps/workshop/tests/workshop-contracts.mjs b/apps/workshop/tests/workshop-contracts.mjs index cc3922cd7..456b0819c 100644 --- a/apps/workshop/tests/workshop-contracts.mjs +++ b/apps/workshop/tests/workshop-contracts.mjs @@ -106,12 +106,29 @@ for (const operation of ["create", "update", "delete"]) { } } if (!hardeningMigration.includes("users:update")) throw new Error("Workshop user file updates must be rate limited"); +for (const required of [ + "writeRateLimits", + "managedRules(settings.rateLimits.rules)", + "settings.rateLimits.enabled = state.enabled", + "...cloneJson(state.managedRules)", +]) { + if (!hardeningMigration.includes(required)) { + throw new Error(`Workshop write rate-limit rollback is missing persistent state restoration: ${required}`); + } +} const accessHardeningMigration = await read("apps/workshop/pb_migrations/1784240002_workshop_access_hardening.js"); +const rateLimitCompatibilityMigration = await read( + "apps/workshop/pb_migrations/1784240003_workshop_rate_limit_rollback_compatibility.js", +); +for (const required of ["writeRateLimits", "legacyCreateRules", "rate-limit compatibility state is unavailable"]) { + if (!rateLimitCompatibilityMigration.includes(required)) { + throw new Error(`Workshop rate-limit compatibility migration is missing: ${required}`); + } +} for (const protectedField of [ "email:changed", "displayName:changed", - "avatar:changed", "avatarUrl:changed", "githubUsername:changed", "githubProfileUrl:changed", @@ -120,6 +137,16 @@ for (const protectedField of [ throw new Error(`Workshop OAuth identity field remains caller-controlled: ${protectedField}`); } } +if (accessHardeningMigration.includes("@request.body.avatar:changed")) { + throw new Error("Workshop access rules must not use the unsupported :changed modifier for file fields"); +} +if ( + !hook.includes('getUnsavedFiles("avatar")') || + !hook.includes('original().getString("avatar")') || + !hook.includes("Workshop avatars are managed by GitHub OAuth") +) { + throw new Error("Workshop avatar updates must be rejected by the user update request hook"); +} for (const protectedField of [ "title:changed", "description:changed", diff --git a/apps/workshop/tests/workshop-oauth-hook.mjs b/apps/workshop/tests/workshop-oauth-hook.mjs index bf192a9c8..61d4985f4 100644 --- a/apps/workshop/tests/workshop-oauth-hook.mjs +++ b/apps/workshop/tests/workshop-oauth-hook.mjs @@ -6,6 +6,7 @@ const nodeRequire = createRequire(import.meta.url); const workshopUtils = nodeRequire("../pb_hooks/workshop-utils.js"); const hookSource = await readFile("apps/workshop/pb_hooks/workshop.pb.js", "utf8"); let oauthHook; +let userUpdateHook; const ignoreHook = () => {}; vm.runInNewContext(hookSource, { @@ -15,7 +16,9 @@ vm.runInNewContext(hookSource, { onRecordAfterCreateSuccess: ignoreHook, onRecordCreateRequest: ignoreHook, onRecordDeleteRequest: ignoreHook, - onRecordUpdateRequest: ignoreHook, + onRecordUpdateRequest: (handler, collection) => { + if (collection === "users") userUpdateHook = handler; + }, onRecordAuthWithOAuth2Request: (handler, collection) => { if (collection !== "users") throw new Error(`OAuth hook targets unexpected collection: ${collection}`); oauthHook = handler; @@ -24,11 +27,17 @@ vm.runInNewContext(hookSource, { if (specifier !== "/pb_hooks/workshop-utils.js") { throw new Error(`OAuth hook loaded unexpected dependency: ${specifier}`); } - return workshopUtils; + return { + ...workshopUtils, + rejectRequest: (message) => { + throw new Error(message); + }, + }; }, }); if (typeof oauthHook !== "function") throw new Error("Workshop OAuth request hook was not registered"); +if (typeof userUpdateHook !== "function") throw new Error("Workshop user update hook was not registered"); const oauth2User = { id: "provider-user-123", @@ -108,4 +117,52 @@ if (ignoredNextCalls !== 1 || Object.keys(ignoredCreateData).length !== 0) { throw new Error("Non-GitHub OAuth providers must pass through without GitHub identity synchronization"); } +let unchangedAvatarNextCalls = 0; +userUpdateHook({ + next: () => { + unchangedAvatarNextCalls += 1; + }, + record: { + getString: () => "provider-avatar.png", + getUnsavedFiles: () => [], + original: () => ({ getString: () => "provider-avatar.png" }), + }, +}); +if (unchangedAvatarNextCalls !== 1) { + throw new Error("User updates that leave the provider avatar unchanged must pass through"); +} + +for (const avatarChange of [ + { + name: "upload", + current: "provider-avatar.png", + original: "provider-avatar.png", + unsavedFiles: [{ name: "forged-avatar.png" }], + }, + { + name: "removal", + current: "", + original: "provider-avatar.png", + unsavedFiles: [], + }, +]) { + let nextCalls = 0; + try { + userUpdateHook({ + next: () => { + nextCalls += 1; + }, + record: { + getString: () => avatarChange.current, + getUnsavedFiles: () => avatarChange.unsavedFiles, + original: () => ({ getString: () => avatarChange.original }), + }, + }); + throw new Error(`Workshop user avatar ${avatarChange.name} was accepted`); + } catch (error) { + if (!String(error).includes("managed by GitHub OAuth")) throw error; + } + if (nextCalls !== 0) throw new Error(`Rejected avatar ${avatarChange.name} continued the update request`); +} + console.log("Workshop OAuth hook contract passed"); diff --git a/apps/workshop/tests/workshop-rate-limit-migration.mjs b/apps/workshop/tests/workshop-rate-limit-migration.mjs new file mode 100644 index 000000000..79896f551 --- /dev/null +++ b/apps/workshop/tests/workshop-rate-limit-migration.mjs @@ -0,0 +1,205 @@ +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import vm from "node:vm"; + +const normalized = (value) => JSON.parse(JSON.stringify(value)); +const managedLabels = [ + "submissions:create", + "submissions:update", + "submissions:delete", + "votes:create", + "votes:update", + "votes:delete", + "comments:create", + "comments:update", + "comments:delete", + "reports:create", + "reports:update", + "reports:delete", + "users:update", +]; +const originalManagedRules = managedLabels.map((label, index) => ({ + label, + audience: index % 2 === 0 ? "@auth" : "", + duration: index + 1, + maxRequests: index + 10, +})); +const originalUnmanagedRule = { + label: "unrelated:create", + audience: "", + duration: 41, + maxRequests: 42, +}; +const stateCollection = { id: "workshop_migration_state" }; +let stateSnapshot = JSON.stringify({ marker: "retain-existing-migration-state" }); +const stateRecord = { + getString(name) { + assert.equal(name, "snapshot"); + return stateSnapshot; + }, + set(name, value) { + assert.equal(name, "snapshot"); + stateSnapshot = value; + }, +}; +const settings = { + rateLimits: { + enabled: false, + rules: [originalUnmanagedRule, ...originalManagedRules], + }, +}; +const app = { + findCollectionByNameOrId(name) { + if (name !== stateCollection.id) throw new Error(`Missing collection ${name}`); + return stateCollection; + }, + findFirstRecordByFilter(collectionId) { + if (collectionId !== stateCollection.id) throw new Error(`Missing state record ${collectionId}`); + return stateRecord; + }, + save() {}, + settings() { + return settings; + }, +}; + +const loadMigration = async (path) => { + let loaded; + const source = await readFile(path, "utf8"); + vm.runInNewContext(source, { + console, + migrate: (up, down) => { + loaded = { up, down }; + }, + }); + if (!loaded) throw new Error(`Workshop migration did not register: ${path}`); + return loaded; +}; + +const migration = await loadMigration("apps/workshop/pb_migrations/1784240001_workshop_write_rate_limits.js"); + +migration.up(app); +const savedState = JSON.parse(stateSnapshot); +assert.equal(savedState.marker, "retain-existing-migration-state"); +assert.deepEqual(savedState.writeRateLimits, { + enabled: false, + managedRules: originalManagedRules, +}); +assert.equal(settings.rateLimits.enabled, true); +assert.deepEqual( + normalized(settings.rateLimits.rules.filter((rule) => !managedLabels.includes(rule.label))), + [originalUnmanagedRule], + "Applying Workshop rate limits changed unrelated rules", +); +assert.deepEqual( + new Set(settings.rateLimits.rules.map((rule) => rule.label)), + new Set([...managedLabels, originalUnmanagedRule.label]), + "Applying Workshop rate limits did not replace every managed label", +); + +const laterUnmanagedRule = { + label: "added-after-migration:update", + audience: "@auth", + duration: 51, + maxRequests: 52, +}; +settings.rateLimits.rules = [ + laterUnmanagedRule, + ...settings.rateLimits.rules.filter((rule) => managedLabels.includes(rule.label)), +]; +migration.down(app); + +assert.equal(settings.rateLimits.enabled, false, "Rollback did not restore the original enabled flag"); +assert.deepEqual( + normalized(settings.rateLimits.rules.filter((rule) => !managedLabels.includes(rule.label))), + [laterUnmanagedRule], + "Rollback changed unrelated rules added after the migration", +); +assert.deepEqual( + normalized(settings.rateLimits.rules.filter((rule) => managedLabels.includes(rule.label))), + originalManagedRules, + "Rollback did not restore every original managed-label rule", +); + +assert.throws( + () => + migration.up({ + ...app, + findCollectionByNameOrId() { + throw new Error("missing"); + }, + }), + /migration state is unavailable/u, + "Migration must fail closed when its persistent rollback snapshot is unavailable", +); + +const legacyOriginalRules = [ + { label: "unrelated:create", audience: "", duration: 71, maxRequests: 72 }, + { label: "submissions:create", audience: "", duration: 73, maxRequests: 74 }, + { label: "comments:update", audience: "@auth", duration: 75, maxRequests: 76 }, +]; +const legacyStateCollection = { id: "workshop_migration_state" }; +let legacyStateSnapshot = JSON.stringify({ + marker: "already-applied-old-0001", + rateLimits: { enabled: false, rules: legacyOriginalRules }, +}); +const legacyStateRecord = { + getString() { + return legacyStateSnapshot; + }, + set(_name, value) { + legacyStateSnapshot = value; + }, +}; +const legacySettings = { + rateLimits: { + enabled: true, + rules: [ + { label: "unrelated:create", audience: "", duration: 71, maxRequests: 72 }, + ...managedLabels.map((label) => ({ label, audience: "@auth", duration: 60, maxRequests: 30 })), + ], + }, +}; +const legacyApp = { + findCollectionByNameOrId() { + return legacyStateCollection; + }, + findFirstRecordByFilter() { + return legacyStateRecord; + }, + save() {}, + settings() { + return legacySettings; + }, +}; +const compatibilityMigration = await loadMigration( + "apps/workshop/pb_migrations/1784240003_workshop_rate_limit_rollback_compatibility.js", +); +compatibilityMigration.up(legacyApp); +compatibilityMigration.down(legacyApp); + +const backfilledState = JSON.parse(legacyStateSnapshot); +assert.equal(backfilledState.marker, "already-applied-old-0001"); +assert.deepEqual(backfilledState.writeRateLimits, { + enabled: true, + managedRules: [ + { label: "comments:update", audience: "@auth", duration: 75, maxRequests: 76 }, + { label: "submissions:create", audience: "@auth", duration: 60, maxRequests: 10 }, + { label: "votes:create", audience: "@auth", duration: 10, maxRequests: 20 }, + { label: "comments:create", audience: "@auth", duration: 60, maxRequests: 20 }, + { label: "reports:create", audience: "@auth", duration: 60, maxRequests: 5 }, + ], +}); + +migration.down(legacyApp); +assert.equal(legacySettings.rateLimits.enabled, true); +assert.deepEqual( + normalized(legacySettings.rateLimits.rules), + [ + { label: "unrelated:create", audience: "", duration: 71, maxRequests: 72 }, + ...backfilledState.writeRateLimits.managedRules, + ], + "An already-applied legacy migration could not be rolled back after the compatibility backfill", +); + +console.log("Workshop write rate-limit migration restoration passed"); diff --git a/apps/workshop/tests/workshop.integration.mjs b/apps/workshop/tests/workshop.integration.mjs index 94a651999..af7bbce8d 100644 --- a/apps/workshop/tests/workshop.integration.mjs +++ b/apps/workshop/tests/workshop.integration.mjs @@ -98,11 +98,37 @@ for (const [field, value] of [ 404, ); } +const forgedAvatar = new FormData(); +forgedAvatar.set( + "avatar", + new Blob( + [ + Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=", + "base64", + ), + ], + { type: "image/png" }, + ), + "forged-avatar.png", +); +const avatarResponse = await fetch(`${baseUrl}/api/collections/users/records/${author.id}`, { + method: "PATCH", + headers: authorSession.headers, + body: forgedAvatar, +}); +const avatarResponseBody = await avatarResponse.json().catch(() => null); +if (avatarResponse.status !== 400 || !JSON.stringify(avatarResponseBody).includes("managed by GitHub OAuth")) { + throw new Error( + `Workshop user avatar upload bypassed the OAuth identity hook: ${avatarResponse.status} ${JSON.stringify(avatarResponseBody)}`, + ); +} const unchangedAuthor = await request(`/api/collections/users/records/${author.id}`, { headers: authorSession.headers, }); if ( unchangedAuthor.displayName !== "Widget Author" || + unchangedAuthor.avatar || unchangedAuthor.avatarUrl || unchangedAuthor.githubUsername || unchangedAuthor.githubProfileUrl diff --git a/apps/workshop/workshop-url.awk b/apps/workshop/workshop-url.awk index e12b2def8..2f2dd04d1 100644 --- a/apps/workshop/workshop-url.awk +++ b/apps/workshop/workshop-url.awk @@ -32,11 +32,14 @@ function valid_hostname(value, labels, count, label_index) { return 1 } -function valid_ipv6(value, percent, address, zone, collapsed, parts, count, groups, part_index) { +function valid_ipv6(value, percent, address, starts_single_colon, ends_single_colon, collapsed, parts, count, groups, part_index) { percent = index(value, "%") if (percent) return 0 address = value if (address !~ /:/ || address ~ /:::/ || address !~ /^[0-9A-Fa-f:.]+$/) return 0 + starts_single_colon = substr(address, 1, 1) == ":" && substr(address, 1, 2) != "::" + ends_single_colon = substr(address, length(address), 1) == ":" && substr(address, length(address) - 1) != "::" + if (starts_single_colon || ends_single_colon) return 0 collapsed = address sub(/::/, "", collapsed) if (collapsed ~ /::/) return 0 diff --git a/e2e/custom-widgets-custom-jsx-v2.spec.ts b/e2e/custom-widgets-custom-jsx-v2.spec.ts index 3526dc6a5..d7f8d72a3 100644 --- a/e2e/custom-widgets-custom-jsx-v2.spec.ts +++ b/e2e/custom-widgets-custom-jsx-v2.spec.ts @@ -6,19 +6,19 @@ import { stringify as stringifySuperJson } from "superjson"; import type { StartedTestContainer } from "testcontainers"; import { describe, test } from "vitest"; -import { eq } from "@homarr/db"; +import { eq } from "drizzle-orm"; import { customWidgetDefinitions, customWidgetSecrets, legacyCustomWidgetDefinitions, legacyCustomWidgetSecrets, -} from "@homarr/db/schema"; -import type { HomarrCustomWidgetV2Input } from "@homarr/custom-widgets/core"; +} from "../packages/db/schema/sqlite"; +import type { HomarrCustomWidgetV2Input } from "../packages/custom-widgets/src/core"; import { createHomarrContainer } from "./shared/create-homarr-container"; import { createSqliteDbFileAsync } from "./shared/e2e-db"; import type { MockApiServer } from "./shared/mock-api-server"; -import { startMockApiServerAsync } from "./shared/mock-api-server"; +import { exposeHostPortToContainersAsync, startMockApiServerAsync } from "./shared/mock-api-server"; import { seedAdminUserAsync } from "./shared/seed-admin-user"; const adminCredentials = { username: "admin", password: "Comp(exP4sswOrd" }; @@ -122,7 +122,7 @@ const startWorkshopMockServerAsync = async (widget: HomarrCustomWidgetV2Input): await new Promise((resolve, reject) => { server.once("error", reject); - server.listen(0, "127.0.0.1", () => { + server.listen(0, "0.0.0.0", () => { server.off("error", reject); resolve(); }); @@ -130,14 +130,20 @@ const startWorkshopMockServerAsync = async (widget: HomarrCustomWidgetV2Input): const address = server.address(); if (!address || typeof address === "string") throw new Error("Workshop mock server did not bind to a TCP port"); - return { - url: `http://127.0.0.1:${address.port}`, - close: () => - new Promise((resolve, reject) => { - server.close((error) => (error ? reject(error) : resolve())); - server.closeAllConnections(); - }), - }; + try { + return { + url: await exposeHostPortToContainersAsync(address.port), + close: () => + new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + server.closeAllConnections(); + }), + }; + } catch (error) { + server.closeAllConnections(); + server.close(); + throw error; + } }; const loginAsAdmin = async (browser: Browser, baseUrl: string, clipboard = false) => { diff --git a/e2e/onboarding.spec.ts b/e2e/onboarding.spec.ts index a8a09773e..941f08335 100644 --- a/e2e/onboarding.spec.ts +++ b/e2e/onboarding.spec.ts @@ -1,4 +1,5 @@ import { chromium } from "@playwright/test"; +import type { Browser } from "@playwright/test"; import { describe, test } from "vitest"; import { OnboardingActions } from "./shared/actions/onboarding-actions"; @@ -16,31 +17,32 @@ describe("Onboarding", () => { }, }).start(); - const browser = await chromium.launch(); - const context = await browser.newContext(); - const page = await context.newPage(); - const actions = new OnboardingActions(page, db); - const assertions = new OnboardingAssertions(page, db); + let browser: Browser | undefined; + try { + browser = await chromium.launch(); + const context = await browser.newContext(); + const page = await context.newPage(); + const actions = new OnboardingActions(page, db); + const assertions = new OnboardingAssertions(page, db); - // Act - await page.goto(`http://${homarrContainer.getHost()}:${homarrContainer.getMappedPort(7575)}`); - await actions.startOnboardingAsync("scratch"); - await actions.processUserStepAsync({ - username: "admin", - password: "Comp(exP4sswOrd", - confirmPassword: "Comp(exP4sswOrd", - }); - await actions.processSettingsStepAsync(); - await actions.processIntegrationsStepAsync(); + // Act + await page.goto(`http://${homarrContainer.getHost()}:${homarrContainer.getMappedPort(7575)}`); + await actions.startOnboardingAsync("scratch"); + await actions.processUserStepAsync({ + username: "admin", + password: "Comp(exP4sswOrd", + confirmPassword: "Comp(exP4sswOrd", + }); + await actions.processSettingsStepAsync(); + await actions.processIntegrationsStepAsync(); - // Assert - await assertions.assertFinishStepVisibleAsync(); - await assertions.assertUserAndAdminGroupInsertedAsync("admin"); - await assertions.assertDbOnboardingStepAsync("finish"); - - // Cleanup - await browser.close(); - await homarrContainer.stop(); + // Assert + await assertions.assertFinishStepVisibleAsync(); + await assertions.assertUserAndAdminGroupInsertedAsync("admin"); + await assertions.assertDbOnboardingStepAsync("finish"); + } finally { + await Promise.allSettled([browser?.close(), homarrContainer.stop()]); + } }, 60_000); test("External provider onboarding setup should be successful", async () => { @@ -60,28 +62,29 @@ describe("Onboarding", () => { }).start(); const externalGroupName = "oidc-admins"; - const browser = await chromium.launch(); - const context = await browser.newContext(); - const page = await context.newPage(); - const actions = new OnboardingActions(page, db); - const assertions = new OnboardingAssertions(page, db); + let browser: Browser | undefined; + try { + browser = await chromium.launch(); + const context = await browser.newContext(); + const page = await context.newPage(); + const actions = new OnboardingActions(page, db); + const assertions = new OnboardingAssertions(page, db); - // Act - await page.goto(`http://${homarrContainer.getHost()}:${homarrContainer.getMappedPort(7575)}`); - await actions.startOnboardingAsync("scratch"); - await actions.processExternalGroupStepAsync({ - name: externalGroupName, - }); - await actions.processSettingsStepAsync(); - await actions.processIntegrationsStepAsync(); + // Act + await page.goto(`http://${homarrContainer.getHost()}:${homarrContainer.getMappedPort(7575)}`); + await actions.startOnboardingAsync("scratch"); + await actions.processExternalGroupStepAsync({ + name: externalGroupName, + }); + await actions.processSettingsStepAsync(); + await actions.processIntegrationsStepAsync(); - // Assert - await assertions.assertFinishStepVisibleAsync(); - await assertions.assertExternalGroupInsertedAsync(externalGroupName); - await assertions.assertDbOnboardingStepAsync("finish"); - - // Cleanup - await browser.close(); - await homarrContainer.stop(); + // Assert + await assertions.assertFinishStepVisibleAsync(); + await assertions.assertExternalGroupInsertedAsync(externalGroupName); + await assertions.assertDbOnboardingStepAsync("finish"); + } finally { + await Promise.allSettled([browser?.close(), homarrContainer.stop()]); + } }, 60_000); }); diff --git a/e2e/shared/actions/onboarding-actions.ts b/e2e/shared/actions/onboarding-actions.ts index 56c09aba7..56d4c02da 100644 --- a/e2e/shared/actions/onboarding-actions.ts +++ b/e2e/shared/actions/onboarding-actions.ts @@ -32,7 +32,7 @@ export class OnboardingActions { } public async processUserStepAsync(input: { username: string; password: string; confirmPassword: string }) { - await this.page.waitForSelector("text=administrator user"); + await this.page.getByLabel("Username").waitFor({ state: "visible" }); await this.page.getByLabel("Username").fill(input.username); await this.page.getByLabel("Password", { exact: true }).fill(input.password); diff --git a/e2e/shared/mock-api-server.ts b/e2e/shared/mock-api-server.ts index 10ccbcaab..55891e779 100644 --- a/e2e/shared/mock-api-server.ts +++ b/e2e/shared/mock-api-server.ts @@ -10,6 +10,19 @@ export interface MockApiServer { close: () => Promise; } +export async function exposeHostPortToContainersAsync(port: number): Promise { + await TestContainers.exposeHostPorts(port); + const client = await getContainerRuntimeClient(); + const forwarder = (await client.container.list()).find( + (container) => container.State === "running" && container.Labels[TESTCONTAINERS_SSHD_LABEL] === "true", + ); + const forwarderAddress = forwarder + ? Object.values(forwarder.NetworkSettings.Networks).find((network) => network.IPAddress)?.IPAddress + : undefined; + if (!forwarderAddress) throw new Error("Failed to locate the Testcontainers host-port forwarder"); + return `http://${forwarderAddress}:${port}`; +} + export async function startMockApiServerAsync(responseBody: unknown): Promise { const body = JSON.stringify(responseBody); const server: Server = await new Promise((resolve) => { @@ -26,18 +39,8 @@ export async function startMockApiServerAsync(responseBody: unknown): Promise container.State === "running" && container.Labels[TESTCONTAINERS_SSHD_LABEL] === "true", - ); - const forwarderAddress = forwarder - ? Object.values(forwarder.NetworkSettings.Networks).find((network) => network.IPAddress)?.IPAddress - : undefined; - if (!forwarderAddress) throw new Error("Failed to locate the Testcontainers host-port forwarder"); - return { - url: `http://${forwarderAddress}:${address.port}`, + url: await exposeHostPortToContainersAsync(address.port), close: () => new Promise((resolve, reject) => { server.close((error) => (error ? reject(error) : resolve())); diff --git a/packages/api/src/custom-widget-configuration.ts b/packages/api/src/custom-widget-configuration.ts index 5b212d7d1..e636f5fd0 100644 --- a/packages/api/src/custom-widget-configuration.ts +++ b/packages/api/src/custom-widget-configuration.ts @@ -5,3 +5,4 @@ export { parseStoredCustomWidgetDefinition, serializeCustomWidgetDefinition, } from "./router/custom-widget/stored-definition"; +export { configureCustomWidgetSourceFromRequest } from "./router/custom-widget/secret-persistence"; diff --git a/packages/api/src/router/custom-widget/secret-persistence.ts b/packages/api/src/router/custom-widget/secret-persistence.ts index d72dfa8b2..73f570510 100644 --- a/packages/api/src/router/custom-widget/secret-persistence.ts +++ b/packages/api/src/router/custom-widget/secret-persistence.ts @@ -1,19 +1,35 @@ +import { TRPCError } from "@trpc/server"; + import { encryptSecret } from "@homarr/common/server"; import type { Database } from "@homarr/db"; import { and, eq, handleTransactionsAsync } from "@homarr/db"; import { customWidgetDefinitions, customWidgetSecrets } from "@homarr/db/schema"; -import type { - CustomWidgetCreateInput, - CustomWidgetSecretKind, - CustomWidgetSource, - HomarrCustomWidgetV2, -} from "@homarr/custom-widgets/core"; +import type { CustomWidgetCreateInput, CustomWidgetSecretKind, CustomWidgetSource } from "@homarr/custom-widgets/core"; +import { customWidgetSourceSchema, hasSameCustomWidgetSourceAuthentication } from "@homarr/custom-widgets/core"; -import { hasSameSecretBinding } from "./secret-policy"; -import { serializeCustomWidgetDefinition } from "./stored-definition"; +import { assertSecretSources, hasSameSecretBinding } from "./secret-policy"; +import { parseStoredCustomWidgetDefinition, serializeCustomWidgetDefinition } from "./stored-definition"; type CustomWidgetSecretInput = CustomWidgetCreateInput["secrets"][number]; +type ConfigureCustomWidgetSourceInput = { + definitionId: string; + sourceId: string; + baseUrl: string; + networkScope?: CustomWidgetSource["networkScope"]; + secrets: CustomWidgetCreateInput["secrets"]; + expectedSource?: CustomWidgetSource; +}; + +type SourceConfigurationIssue = "definition-not-found" | "source-not-found" | "binding-changed"; + +class SourceConfigurationError extends Error { + constructor(readonly issue: SourceConfigurationIssue) { + super(`Custom widget source configuration failed: ${issue}`); + this.name = "SourceConfigurationError"; + } +} + export async function setCustomWidgetSecret(db: Database, definitionId: string, secret: CustomWidgetSecretInput) { const updatedAt = new Date(); const secretRow = { @@ -65,22 +81,39 @@ export async function setCustomWidgetSecret(db: Database, definitionId: string, }); } -export async function configureCustomWidgetSource( +export async function configureCustomWidgetSource(db: Database, input: ConfigureCustomWidgetSourceInput) { + try { + return await persistCustomWidgetSourceConfiguration(db, input); + } catch (error) { + if (!(error instanceof SourceConfigurationError)) throw error; + if (error.issue === "definition-not-found") throw new TRPCError({ code: "NOT_FOUND" }); + if (error.issue === "source-not-found") { + throw new TRPCError({ code: "NOT_FOUND", message: "Widget source not found" }); + } + throw new TRPCError({ + code: "CONFLICT", + message: "Widget source authentication changed while it was being configured", + }); + } +} + +export async function configureCustomWidgetSourceFromRequest( db: Database, - input: { - definitionId: string; - sourceId: string; - definition: HomarrCustomWidgetV2; - previousSource: CustomWidgetSource; - source: CustomWidgetSource; - secrets: CustomWidgetCreateInput["secrets"]; - }, + input: ConfigureCustomWidgetSourceInput & { expectedSource: CustomWidgetSource }, ) { - const definitionChanges = { - ...serializeCustomWidgetDefinition(input.definition), - updatedAt: new Date(), - }; - const bindingChanged = !hasSameSecretBinding(input.previousSource, input.source); + try { + return { + status: "configured" as const, + source: await persistCustomWidgetSourceConfiguration(db, input), + }; + } catch (error) { + if (!(error instanceof SourceConfigurationError)) throw error; + return { status: error.issue } as const; + } +} + +async function persistCustomWidgetSourceConfiguration(db: Database, input: ConfigureCustomWidgetSourceInput) { + let configuredSource: CustomWidgetSource | undefined; const secretRows = input.secrets.map((secret) => ({ definitionId: input.definitionId, sourceId: input.sourceId, @@ -91,11 +124,19 @@ export async function configureCustomWidgetSource( await handleTransactionsAsync(db, { async handleAsync(database, schema) { await database.transaction(async (transaction) => { + const [stored] = await transaction + .select() + .from(schema.customWidgetDefinitions) + .where(eq(schema.customWidgetDefinitions.id, input.definitionId)) + .limit(1) + .for("update"); + const sourceUpdate = prepareSourceUpdate(stored, input); + configuredSource = sourceUpdate.source; await transaction .update(schema.customWidgetDefinitions) - .set(definitionChanges) + .set(sourceUpdate.definitionChanges) .where(eq(schema.customWidgetDefinitions.id, input.definitionId)); - if (bindingChanged) { + if (sourceUpdate.bindingChanged) { await transaction .delete(schema.customWidgetSecrets) .where( @@ -120,39 +161,82 @@ export async function configureCustomWidgetSource( }); }, handleSync(database) { - database.transaction((transaction) => { - transaction - .update(customWidgetDefinitions) - .set(definitionChanges) - .where(eq(customWidgetDefinitions.id, input.definitionId)) - .run(); - if (bindingChanged) { + database.transaction( + (transaction) => { + const stored = transaction + .select() + .from(customWidgetDefinitions) + .where(eq(customWidgetDefinitions.id, input.definitionId)) + .limit(1) + .get(); + const sourceUpdate = prepareSourceUpdate(stored, input); + configuredSource = sourceUpdate.source; transaction - .delete(customWidgetSecrets) - .where( - and( - eq(customWidgetSecrets.definitionId, input.definitionId), - eq(customWidgetSecrets.sourceId, input.sourceId), - ), - ) + .update(customWidgetDefinitions) + .set(sourceUpdate.definitionChanges) + .where(eq(customWidgetDefinitions.id, input.definitionId)) .run(); - } - for (const secret of secretRows) { - transaction - .delete(customWidgetSecrets) - .where( - and( - eq(customWidgetSecrets.definitionId, input.definitionId), - eq(customWidgetSecrets.sourceId, input.sourceId), - eq(customWidgetSecrets.kind, secret.kind), - ), - ) - .run(); - transaction.insert(customWidgetSecrets).values(secret).run(); - } - }); + if (sourceUpdate.bindingChanged) { + transaction + .delete(customWidgetSecrets) + .where( + and( + eq(customWidgetSecrets.definitionId, input.definitionId), + eq(customWidgetSecrets.sourceId, input.sourceId), + ), + ) + .run(); + } + for (const secret of secretRows) { + transaction + .delete(customWidgetSecrets) + .where( + and( + eq(customWidgetSecrets.definitionId, input.definitionId), + eq(customWidgetSecrets.sourceId, input.sourceId), + eq(customWidgetSecrets.kind, secret.kind), + ), + ) + .run(); + transaction.insert(customWidgetSecrets).values(secret).run(); + } + }, + { behavior: "immediate" }, + ); }, }); + if (!configuredSource) throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); + return configuredSource; +} + +function prepareSourceUpdate( + stored: Parameters[0] | undefined, + input: ConfigureCustomWidgetSourceInput, +) { + if (!stored) throw new SourceConfigurationError("definition-not-found"); + const definition = parseStoredCustomWidgetDefinition(stored); + const previousSource = definition.sources[input.sourceId]; + if (!previousSource) throw new SourceConfigurationError("source-not-found"); + if (input.expectedSource && !hasSameCustomWidgetSourceAuthentication(previousSource, input.expectedSource)) { + throw new SourceConfigurationError("binding-changed"); + } + const source = customWidgetSourceSchema.parse({ + ...previousSource, + baseUrl: input.baseUrl, + networkScope: input.networkScope ?? previousSource.networkScope, + }); + assertSecretSources({ [input.sourceId]: source }, input.secrets); + return { + source, + bindingChanged: !hasSameSecretBinding(previousSource, source), + definitionChanges: { + ...serializeCustomWidgetDefinition({ + ...definition, + sources: { ...definition.sources, [input.sourceId]: source }, + }), + updatedAt: new Date(), + }, + }; } export async function clearCustomWidgetSecret( diff --git a/packages/api/src/router/custom-widget/secret-procedures.ts b/packages/api/src/router/custom-widget/secret-procedures.ts index 792703052..447b8e2fa 100644 --- a/packages/api/src/router/custom-widget/secret-procedures.ts +++ b/packages/api/src/router/custom-widget/secret-procedures.ts @@ -80,20 +80,18 @@ export const secretProcedures = { networkScope: input.networkScope ?? current.networkScope, }); assertSecretSources({ [input.sourceId]: source }, input.secrets); - const updated = { ...definition, sources: { ...definition.sources, [input.sourceId]: source } }; - await configureCustomWidgetSource(ctx.db, { + const configuredSource = await configureCustomWidgetSource(ctx.db, { definitionId: input.definitionId, sourceId: input.sourceId, - definition: updated, - previousSource: current, - source, + baseUrl: source.baseUrl, + networkScope: input.networkScope, secrets: input.secrets, }); return { definitionId: input.definitionId, sourceId: input.sourceId, - baseUrl: source.baseUrl, - networkScope: source.networkScope, + baseUrl: configuredSource.baseUrl, + networkScope: configuredSource.networkScope, configuredSecrets: input.secrets.map(({ kind }) => kind), }; }), diff --git a/packages/api/src/router/test/custom-widget/definition-persistence.spec.ts b/packages/api/src/router/test/custom-widget/definition-persistence.spec.ts index 54a383db3..3913e8901 100644 --- a/packages/api/src/router/test/custom-widget/definition-persistence.spec.ts +++ b/packages/api/src/router/test/custom-widget/definition-persistence.spec.ts @@ -14,6 +14,7 @@ import { describe, expect, test } from "vitest"; import { customWidgetRouter } from "../../custom-widget/custom-widget-router"; import { getCustomWidgetConfigurationRequestForUser } from "../../custom-widget/configuration-requests"; +import { configureCustomWidgetSourceFromRequest } from "../../custom-widget/secret-persistence"; import { serializeCustomWidgetDefinition } from "../../custom-widget/stored-definition"; const userId = createId(); @@ -399,6 +400,103 @@ describe("custom widget definition persistence", () => { ).toHaveLength(0); }); + test("uses the persisted source binding from inside the configuration transaction", async () => { + const db = await prepareDatabase(); + const created = await createCaller(db).create({ ...jellyfin, secrets: [secret] }); + const definition = customWidgetDefinitionSchema.parse(jellyfin); + const defaultSource = definition.sources.default; + if (!defaultSource) throw new Error("Jellyfin default source is missing"); + await db + .update(customWidgetDefinitions) + .set( + serializeCustomWidgetDefinition({ + ...definition, + name: "Concurrent definition edit", + sources: { + ...definition.sources, + default: { ...defaultSource, baseUrl: "https://changed.example/api" }, + }, + }), + ) + .where(eq(customWidgetDefinitions.id, created.id)); + + await expect( + configureCustomWidgetSourceFromRequest(db, { + definitionId: created.id, + sourceId: "default", + baseUrl: defaultSource.baseUrl, + networkScope: defaultSource.networkScope, + secrets: [], + expectedSource: defaultSource, + }), + ).resolves.toMatchObject({ + status: "configured", + source: { baseUrl: defaultSource.baseUrl }, + }); + + expect(await createCaller(db).get({ id: created.id })).toMatchObject({ + name: "Concurrent definition edit", + sources: { default: { baseUrl: defaultSource.baseUrl } }, + }); + expect( + await db.query.customWidgetSecrets.findMany({ + where: eq(customWidgetSecrets.definitionId, created.id), + }), + ).toHaveLength(0); + }); + + test("rejects stale setup-link credentials after a concurrent authentication binding change", async () => { + const db = await prepareDatabase(); + const created = await createCaller(db).create({ ...jellyfin, secrets: [] }); + const definition = customWidgetDefinitionSchema.parse(jellyfin); + const expectedSource = definition.sources.default; + if (!expectedSource || typeof expectedSource.auth === "string") { + throw new Error("Jellyfin default source must use header authentication"); + } + await db + .update(customWidgetDefinitions) + .set( + serializeCustomWidgetDefinition({ + ...definition, + name: "Concurrent definition edit", + sources: { + ...definition.sources, + default: { + ...expectedSource, + auth: { ...expectedSource.auth, name: "X-Replaced-Credential" }, + }, + }, + }), + ) + .where(eq(customWidgetDefinitions.id, created.id)); + + await expect( + configureCustomWidgetSourceFromRequest(db, { + definitionId: created.id, + sourceId: "default", + baseUrl: "https://configured.example/api", + networkScope: "public", + secrets: [secret], + expectedSource, + }), + ).resolves.toEqual({ status: "binding-changed" }); + + expect(await createCaller(db).get({ id: created.id })).toMatchObject({ + name: "Concurrent definition edit", + sources: { + default: { + baseUrl: expectedSource.baseUrl, + auth: { name: "X-Replaced-Credential" }, + }, + }, + }); + expect( + await db.query.customWidgetSecrets.findMany({ + where: eq(customWidgetSecrets.definitionId, created.id), + }), + ).toHaveLength(0); + }); + test("clears persisted credentials when a general definition update changes its binding", async () => { const db = await prepareDatabase(); const caller = createCaller(db); diff --git a/packages/custom-widgets/src/core/url-policy.ts b/packages/custom-widgets/src/core/url-policy.ts index 9fd2cc7dc..1cd98a519 100644 --- a/packages/custom-widgets/src/core/url-policy.ts +++ b/packages/custom-widgets/src/core/url-policy.ts @@ -42,6 +42,14 @@ export function getCustomWidgetHttpUrlIssue(value: string): CustomWidgetHttpUrlI const rawHostname = withoutTrailingRootDot(hostAndPort.hostname); const parsedHostname = withoutTrailingRootDot(url.hostname); if (!rawHostname || !hasCanonicalParsedHostname(parsedHostname)) return "invalid"; + if ( + rawHostname.startsWith("[") && + parsedHostname.startsWith("[") && + !rawHostname.includes(".") && + rawHostname.toLowerCase() !== parsedHostname + ) { + return "invalid"; + } if (looksNumeric(rawHostname) && !isStrictIpv4(rawHostname)) return "invalid"; if (isStrictIpv4(parsedHostname) && rawHostname !== parsedHostname) return "invalid"; diff --git a/packages/custom-widgets/src/server/request-dispatcher-lifecycle.ts b/packages/custom-widgets/src/server/request-dispatcher-lifecycle.ts index 41cda5a02..b551c324a 100644 --- a/packages/custom-widgets/src/server/request-dispatcher-lifecycle.ts +++ b/packages/custom-widgets/src/server/request-dispatcher-lifecycle.ts @@ -1,6 +1,6 @@ import type { createPinnedAgent } from "./network-policy"; -const DISPATCHER_CLOSE_GRACE_MS = 1_000; +export const DISPATCHER_CLOSE_GRACE_MS = 1_000; type CustomWidgetDispatcher = ReturnType; @@ -31,7 +31,10 @@ function detachDispatcherDestroy(dispatcher: CustomWidgetDispatcher): void { } function abortable(operation: Promise, signal: AbortSignal): Promise { - if (signal.aborted) return Promise.reject(signal.reason ?? createAbortError()); + if (signal.aborted) { + void operation.catch(() => undefined); + return Promise.reject(signal.reason ?? createAbortError()); + } return new Promise((resolve, reject) => { const onAbort = () => { diff --git a/packages/custom-widgets/src/test/request-deadline.spec.ts b/packages/custom-widgets/src/test/request-deadline.spec.ts index 7d36693a7..d350705c0 100644 --- a/packages/custom-widgets/src/test/request-deadline.spec.ts +++ b/packages/custom-widgets/src/test/request-deadline.spec.ts @@ -15,6 +15,7 @@ vi.mock("../server/network-policy", async (importOriginal) => { }); import { executeCustomWidgetRequest, MAX_REQUEST_DURATION_MS } from "../server/request-executor"; +import { closeDispatcher, DISPATCHER_CLOSE_GRACE_MS } from "../server/request-dispatcher-lifecycle"; import { resolveAndValidateHost } from "../server/network-policy"; const query = { @@ -83,7 +84,7 @@ describe("custom widget request deadline", () => { logError, }); - await vi.advanceTimersByTimeAsync(1_000); + await vi.advanceTimersByTimeAsync(DISPATCHER_CLOSE_GRACE_MS); await expect(request).resolves.toMatchObject({ ok: true, status: 200, data: {} }); expect(close).toHaveBeenCalledOnce(); @@ -133,6 +134,28 @@ describe("custom widget request deadline", () => { ); }); + it("consumes a late close rejection when cleanup starts after the deadline", async () => { + const abortReason = new Error("request deadline elapsed"); + const closeFailure = new Error("late close failure"); + const controller = new AbortController(); + controller.abort(abortReason); + let rejectClose: ((reason: Error) => void) | undefined; + const close = vi.fn( + () => + new Promise((_resolve, reject) => { + rejectClose = reject; + }), + ); + const destroy = vi.fn(async () => undefined); + + await expect(closeDispatcher({ close, destroy } as never, controller.signal)).rejects.toBe(abortReason); + rejectClose?.(closeFailure); + await Promise.resolve(); + + expect(close).toHaveBeenCalledOnce(); + expect(destroy).toHaveBeenCalledOnce(); + }); + it("redacts apiKeyQuery credentials from transport errors and structured log events", async () => { const apiKey = "api-key-query-secret"; const request = vi.fn(async ({ path }: { path: string }) => { diff --git a/packages/custom-widgets/src/test/schema.spec.ts b/packages/custom-widgets/src/test/schema.spec.ts index 270113fdd..eeb02a262 100644 --- a/packages/custom-widgets/src/test/schema.spec.ts +++ b/packages/custom-widgets/src/test/schema.spec.ts @@ -128,6 +128,8 @@ describe("lean Custom Widget schema", () => { "https://example.com:65536", "https://[fe80::1%25eth0]", "https://[::ffff:192.168.001.001]", + "https://[0:0:0:0:0:0:0:1]", + "https://[2001:0db8::1]", ])("rejects ambiguous persisted URL spelling %s for sources and icons", (url) => { expect( customWidgetDefinitionSchema.safeParse({ diff --git a/packages/definitions/src/test/docs.spec.ts b/packages/definitions/src/test/docs.spec.ts index 251f58bb8..86d19cc74 100644 --- a/packages/definitions/src/test/docs.spec.ts +++ b/packages/definitions/src/test/docs.spec.ts @@ -36,9 +36,13 @@ describe("createDocumentationLink parameter validation", () => { meta.content = "https://docs.example.com/base/"; document.head.append(meta); - expect(createDocumentationLink("/docs/getting-started")).toBe("https://docs.example.com/base/docs/getting-started"); - - meta.remove(); + try { + expect(createDocumentationLink("/docs/getting-started")).toBe( + "https://docs.example.com/base/docs/getting-started", + ); + } finally { + meta.remove(); + } }); test("should work with only path parameter", () => { diff --git a/packages/widgets/src/custom-api/custom-jsx-display-refresh.spec.tsx b/packages/widgets/src/custom-api/custom-jsx-display-refresh.spec.tsx new file mode 100644 index 000000000..4c450105f --- /dev/null +++ b/packages/widgets/src/custom-api/custom-jsx-display-refresh.spec.tsx @@ -0,0 +1,139 @@ +import { act, createElement } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { ComponentPropsWithoutRef, ReactNode } from "react"; +import type { Root } from "react-dom/client"; +import type * as MantineCoreExports from "@mantine/core"; +import type * as TablerIconsExports from "@tabler/icons-react"; + +import CustomJsxDisplay from "./custom-jsx-display"; + +vi.mock("@homarr/api/client", () => ({ + clientApi: { + useUtils: () => ({ + widget: { + customApi: { + getData: { + invalidate: vi.fn(), + }, + }, + }, + }), + }, + fetchApi: { + customWidget: { + previewAction: { mutate: vi.fn() }, + previewQuery: { query: vi.fn() }, + previewRefresh: { mutate: vi.fn() }, + }, + widget: { + customApi: { + executeAction: { mutate: vi.fn() }, + queryRequest: { query: vi.fn() }, + refreshQueries: { mutate: vi.fn() }, + }, + }, + }, +})); + +vi.mock("@homarr/modals", () => ({ + useConfirmModal: () => ({ openConfirmModal: vi.fn() }), +})); + +vi.mock("@homarr/notifications", () => ({ + showErrorNotification: vi.fn(), + showSuccessNotification: vi.fn(), +})); + +vi.mock("@homarr/translation/client", () => ({ + useScopedI18n: () => (key: string) => (key === "refresh" ? "Refresh" : key), +})); + +vi.mock("@tanstack/react-query", () => ({ + useQueryClient: () => ({ invalidateQueries: vi.fn() }), +})); + +vi.mock("@mantine/core", async (importOriginal) => { + const original = await importOriginal(); + const renderElement = createElement; + const MockContainer = ({ children }: { children?: ReactNode }) => renderElement("div", null, children); + const ActionIcon = ({ + children, + loading: _loading, + color: _color, + variant: _variant, + size: _size, + ...props + }: ComponentPropsWithoutRef<"button"> & { + children?: ReactNode; + loading?: boolean; + color?: string; + variant?: string; + size?: string; + }) => renderElement("button", props, children); + + return { + ...original, + ActionIcon, + Alert: MockContainer, + Box: MockContainer, + Stack: MockContainer, + Text: MockContainer, + }; +}); + +vi.mock("@tabler/icons-react", async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + IconRefresh: () => createElement("span", { "aria-hidden": true }), + }; +}); + +describe("CustomJsxDisplay preview refresh", () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + Object.defineProperty(globalThis, "IS_REACT_ACT_ENVIRONMENT", { value: true, writable: true }); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + }); + + const renderPreview = async (canInvalidateQueries?: boolean) => { + await act(async () => { + root.render( + ', + data: {}, + previewSessionId: "preview-session", + ...(canInvalidateQueries === undefined ? {} : { canInvalidateQueries }), + }} + />, + ); + }); + return container.querySelector('button[aria-label="Refresh"]'); + }; + + it("enables refresh when a preview session omits the invalidation capability", async () => { + const button = await renderPreview(); + + expect(button).not.toBeNull(); + expect(button?.disabled).toBe(false); + }); + + it("honors an explicitly disabled invalidation capability", async () => { + const button = await renderPreview(false); + + expect(button).not.toBeNull(); + expect(button?.disabled).toBe(true); + }); +}); diff --git a/packages/widgets/src/custom-api/custom-jsx-display.tsx b/packages/widgets/src/custom-api/custom-jsx-display.tsx index 1bc8b879d..19c6676d8 100644 --- a/packages/widgets/src/custom-api/custom-jsx-display.tsx +++ b/packages/widgets/src/custom-api/custom-jsx-display.tsx @@ -69,7 +69,7 @@ export default function CustomJsxDisplay({ data }: { data: Record { "http://09.0.0.1", "http://099.0.0.1", "http://[:::1]", + "http://[:1:2:3:4:5:6:7:8]", + "http://[1:2:3:4:5:6:7:8:]", + "http://[192.168.1.1::]", "http://[::ffff:192.168.001.001]", "http://[::ffff:192.168.1.01]", "http://[fe80::1%25eth0]", @@ -70,6 +73,11 @@ describe("PocketBase URL compatibility", () => { expect(new PocketBaseUrl("https://例え.テスト.").hostname).toBe("xn--r8jz45g.xn--zckzah."); }); + test("normalizes IPv6 hostnames like the native parser", () => { + expect(new PocketBaseUrl("http://[1:2:3:4:5:6:7::]").hostname).toBe("[1:2:3:4:5:6:7:0]"); + expect(new PocketBaseUrl("http://[::ffff:192.168.1.1]").hostname).toBe("[::ffff:c0a8:101]"); + }); + test.each([ "https://example.com", "https://example.com.", diff --git a/packages/workshop/src/pocketbase-url.ts b/packages/workshop/src/pocketbase-url.ts index 2a152ebde..a6161c41c 100644 --- a/packages/workshop/src/pocketbase-url.ts +++ b/packages/workshop/src/pocketbase-url.ts @@ -99,8 +99,9 @@ function parseAuthority(authority: string): ParsedAuthority { const closeIndex = hostAndPort.indexOf("]"); if (closeIndex < 0) throw new TypeError("Invalid URL"); const address = hostAndPort.slice(1, closeIndex); - if (!isValidIpv6(address)) throw new TypeError("Invalid URL"); - hostname = `[${address.toLowerCase()}]`; + const normalizedAddress = normalizeIpv6(address); + if (normalizedAddress === null) throw new TypeError("Invalid URL"); + hostname = `[${normalizedAddress}]`; const suffix = hostAndPort.slice(closeIndex + 1); if (suffix) { if (!suffix.startsWith(":")) throw new TypeError("Invalid URL"); @@ -167,24 +168,66 @@ function hasForbiddenUrlCharacter(value: string): boolean { }); } -function isValidIpv6(value: string): boolean { - if (value.includes("%")) return false; +function normalizeIpv6(value: string): string | null { + if (value.includes("%")) return null; const address = value; - if (!address.includes(":") || address.includes(":::") || /[^0-9A-Fa-f:.]/u.test(address)) return false; - if (address.indexOf("::") !== address.lastIndexOf("::")) return false; - - const groups = address.split(":").filter(Boolean); - let groupCount = groups.length; - for (const [index, group] of groups.entries()) { - if (group.includes(".")) { - if (index !== groups.length - 1 || !isStrictIpv4(group)) return false; - groupCount += 1; - } else if (!/^[0-9A-Fa-f]{1,4}$/u.test(group)) { - return false; - } + if (!address.includes(":") || address.includes(":::") || /[^0-9A-Fa-f:.]/u.test(address)) return null; + if (address.indexOf("::") !== address.lastIndexOf("::")) return null; + if ((address.startsWith(":") && !address.startsWith("::")) || (address.endsWith(":") && !address.endsWith("::"))) { + return null; } - return address.includes("::") ? groupCount < 8 : groupCount === 8; + const hasCompression = address.includes("::"); + const [left = "", right = ""] = hasCompression ? address.split("::") : [address, ""]; + if (hasCompression && left.includes(".")) return null; + const leftGroups = parseIpv6Groups(left); + const rightGroups = parseIpv6Groups(right); + if (leftGroups === null || rightGroups === null) return null; + + const explicitGroupCount = leftGroups.length + rightGroups.length; + if ((!hasCompression && explicitGroupCount !== 8) || (hasCompression && explicitGroupCount >= 8)) return null; + + const groups = hasCompression + ? [...leftGroups, ...Array.from({ length: 8 - explicitGroupCount }, () => 0), ...rightGroups] + : leftGroups; + let bestStart = -1; + let bestLength = 1; + for (let index = 0; index < groups.length;) { + if (groups[index] !== 0) { + index += 1; + continue; + } + let end = index + 1; + while (end < groups.length && groups[end] === 0) end += 1; + if (end - index > bestLength) { + bestStart = index; + bestLength = end - index; + } + index = end; + } + + const hexadecimalGroups = groups.map((group) => group.toString(16)); + if (bestStart < 0) return hexadecimalGroups.join(":"); + const before = hexadecimalGroups.slice(0, bestStart).join(":"); + const after = hexadecimalGroups.slice(bestStart + bestLength).join(":"); + return `${before}::${after}`; +} + +function parseIpv6Groups(value: string): number[] | null { + if (!value) return []; + const groups = value.split(":"); + const parsed: number[] = []; + for (const [index, group] of groups.entries()) { + if (group.includes(".")) { + if (index !== groups.length - 1 || !isStrictIpv4(group)) return null; + const parts = group.split(".").map(Number); + parsed.push(((parts[0] ?? 0) << 8) | (parts[1] ?? 0), ((parts[2] ?? 0) << 8) | (parts[3] ?? 0)); + } else { + if (!/^[0-9A-Fa-f]{1,4}$/u.test(group)) return null; + parsed.push(Number.parseInt(group, 16)); + } + } + return parsed; } function isStrictIpv4(value: string): boolean { diff --git a/packages/workshop/src/schema.spec.ts b/packages/workshop/src/schema.spec.ts index 8c3f83a9d..95628a2f5 100644 --- a/packages/workshop/src/schema.spec.ts +++ b/packages/workshop/src/schema.spec.ts @@ -37,6 +37,7 @@ describe("Workshop URL configuration", () => { ["/", "/"], ["/docs", "/docs/"], ["/docs/", "/docs/"], + ["/docs..preview", "/docs..preview/"], ])("normalizes DOCS_BASE_URL %s", (input, expected) => { expect(normalizeDocsBaseUrl(input)).toBe(expected); });