From 0feab692bdbf967fccb7c70c37ccabea75c5a9a8 Mon Sep 17 00:00:00 2001 From: Jade <68784313+jang-hs@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:40:53 +0900 Subject: [PATCH] fix(ui): submit raw number input value for auth key expiry (#609) Co-authored-by: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + app/components/number-input.tsx | 10 +- app/routes/settings/auth-keys/actions.ts | 13 ++- tests/unit/settings/auth-keys-action.test.ts | 96 ++++++++++++++++++++ 4 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 tests/unit/settings/auth-keys-action.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d054e7c..084cc89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Next - Fixed the Headplane agent falling back to an interactive Tailscale login. The agent now starts with a pre-auth-key, preserves its existing state across restarts, and auto-approves itself when Headscale requires manual approval (closes [#582](https://github.com/tale/headplane/issues/582)). +- Fixed creating pre-auth keys with an expiry of 1000 days or more. The number input submitted its locale-formatted value (`365,000`, `365 000`, `365.000`), which either failed with a 500 or silently created a key with a truncated expiry. The raw value is now submitted and the server rejects malformed expiries with a 400 (closes [#596](https://github.com/tale/headplane/issues/596)). # 0.7.0 diff --git a/app/components/number-input.tsx b/app/components/number-input.tsx index 194ae3e..562f458 100644 --- a/app/components/number-input.tsx +++ b/app/components/number-input.tsx @@ -21,8 +21,13 @@ export default function NumberInput(props: NumberInputProps) { const { label, name, description } = props; return ( + // `name` belongs on the Root, not the Input. The Input is a text field that + // holds the locale-formatted value ("365,000", "365 000", "365.000"), while + // the Root renders a hidden number input carrying the raw value. Naming the + // Input would submit the formatted string and break server-side parsing. - + diff --git a/app/routes/settings/auth-keys/actions.ts b/app/routes/settings/auth-keys/actions.ts index dbff875..57d0a48 100644 --- a/app/routes/settings/auth-keys/actions.ts +++ b/app/routes/settings/auth-keys/actions.ts @@ -88,10 +88,21 @@ export async function authKeysAction({ request, context }: Route.ActionArgs) { }); } - const day = Number(expiry.toString().split(" ")[0]); + // The form submits the raw, unformatted value of the number input, so + // anything that is not a plain integer (grouping separators from + // `Intl.NumberFormat`, unit suffixes, ...) is malformed input. Parsing it + // leniently either produces an Invalid Date (500) or, for dot-grouping + // locales, a silently truncated expiry. + const day = /^\d+$/.test(expiry.trim()) ? Number(expiry.trim()) : Number.NaN; const date = new Date(); date.setDate(date.getDate() + day); + if (day < 1 || Number.isNaN(date.getTime())) { + return data("`expiry` must be a whole number of days.", { + status: 400, + }); + } + const key = await api.preAuthKeys.create({ user, ephemeral: ephemeral === "on", diff --git a/tests/unit/settings/auth-keys-action.test.ts b/tests/unit/settings/auth-keys-action.test.ts new file mode 100644 index 0000000..66a7b4b --- /dev/null +++ b/tests/unit/settings/auth-keys-action.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, test, vi } from "vitest"; + +import { authContext, requestApiContext } from "~/server/context"; + +// Mock the log module to avoid console spam during tests +vi.mock("~/utils/log", () => ({ + default: { + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, +})); + +function mockFormData(expiry: string): FormData { + const formData = new FormData(); + formData.set("action_id", "add_preauthkey"); + formData.set("user_id", "1"); + formData.set("acl_tags", ""); + formData.set("reusable", "off"); + formData.set("ephemeral", "off"); + formData.set("expiry", expiry); + return formData; +} + +function mockRequest(formData: FormData): Request { + return { + formData: () => Promise.resolve(formData), + } as unknown as Request; +} + +// React Router provides context values through context.get(contextKey), so this +// returns the matching mock depending on the requested key. +function createMockContext(create: ReturnType) { + return { + get: (context: typeof authContext | typeof requestApiContext) => { + if (context === authContext) return { can: () => true }; + if (context === requestApiContext) { + return () => Promise.resolve({ principal: {}, api: { preAuthKeys: { create } } }); + } + return undefined; + }, + }; +} + +async function submitExpiry(expiry: string) { + const { authKeysAction } = await import("~/routes/settings/auth-keys/actions"); + const create = vi.fn().mockResolvedValue({ key: "test-key" }); + + const result = (await authKeysAction({ + request: mockRequest(mockFormData(expiry)), + context: createMockContext(create), + params: {}, + } as any)) as { data: unknown; init?: { status?: number } | null }; + + return { result, create }; +} + +function daysUntil(expiration: Date): number { + const start = new Date(); + return Math.round((expiration.getTime() - start.getTime()) / 86_400_000); +} + +describe("Pre-auth key expiry parsing", () => { + test("accepts a plain integer above the grouping threshold", async () => { + const { result, create } = await submitExpiry("365000"); + + expect(result.init?.status ?? 200).toBe(200); + expect(create).toHaveBeenCalledOnce(); + expect(daysUntil(create.mock.calls[0][0].expiration)).toBe(365_000); + }); + + // Values >= 1000 used to be submitted through Intl.NumberFormat, which groups + // digits differently per locale. Leniently parsing those either produced an + // Invalid Date (500) or, for dot-grouping locales, a silently wrong expiry. + test.for([ + ["en-US", "365,000"], + ["ru-RU", "365 000"], + ["fr-FR", "365 000"], + ["de-DE", "365.000"], + ])("rejects a locale-formatted value (%s)", async ([, expiry]) => { + const { result, create } = await submitExpiry(expiry); + + expect(result.init?.status).toBe(400); + expect(create).not.toHaveBeenCalled(); + }); + + test.for(["0", "-1", "", "90 days", "9999999999999"])( + "rejects an out-of-range or non-numeric value (%s)", + async (expiry) => { + const { result, create } = await submitExpiry(expiry); + + expect(result.init?.status).toBe(400); + expect(create).not.toHaveBeenCalled(); + }, + ); +});