From 99c097e503d520b5b10c6f4466c23917b8e4691e Mon Sep 17 00:00:00 2001 From: Kenneth Date: Sun, 22 Mar 2026 22:45:17 +0000 Subject: [PATCH] fix(backend): reject unknown fields in source config (#88) Add "+": "reject" to all arktype schemas so undeclared keys return 400. Sources without a configSchema now reject the config field entirely at the HTTP layer. Co-authored-by: Ona --- .../src/session/user-session-manager.ts | 23 +++-- apps/aelis-backend/src/sources/http.test.ts | 94 +++++++++++++++++-- apps/aelis-backend/src/sources/http.ts | 19 +++- apps/aelis-backend/src/tfl/provider.ts | 1 + apps/aelis-backend/src/weather/provider.ts | 1 + 5 files changed, 118 insertions(+), 20 deletions(-) diff --git a/apps/aelis-backend/src/session/user-session-manager.ts b/apps/aelis-backend/src/session/user-session-manager.ts index cf9500b..1dbaca5 100644 --- a/apps/aelis-backend/src/session/user-session-manager.ts +++ b/apps/aelis-backend/src/session/user-session-manager.ts @@ -5,10 +5,10 @@ import merge from "lodash.merge" import type { Database } from "../db/index.ts" import type { FeedEnhancer } from "../enhancement/enhance-feed.ts" -import { InvalidSourceConfigError, SourceNotFoundError } from "../sources/errors.ts" -import { sources } from "../sources/user-sources.ts" import type { FeedSourceProvider } from "./feed-source-provider.ts" +import { InvalidSourceConfigError, SourceNotFoundError } from "../sources/errors.ts" +import { sources } from "../sources/user-sources.ts" import { UserSession } from "./user-session.ts" export interface UserSessionManagerConfig { @@ -125,16 +125,14 @@ export class UserSessionManager { // read stale config. Use SELECT FOR UPDATE or atomic jsonb merge if // this becomes a problem. let mergedConfig: Record | undefined - if (update.config !== undefined) { + if (update.config !== undefined && provider.configSchema) { const existing = await sources(this.db, userId).find(sourceId) const existingConfig = (existing?.config ?? {}) as Record mergedConfig = merge({}, existingConfig, update.config) - if (provider.configSchema) { - const validated = provider.configSchema(mergedConfig) - if (validated instanceof type.errors) { - throw new InvalidSourceConfigError(sourceId, validated.summary) - } + const validated = provider.configSchema(mergedConfig) + if (validated instanceof type.errors) { + throw new InvalidSourceConfigError(sourceId, validated.summary) } } @@ -169,23 +167,24 @@ export class UserSessionManager { async upsertSourceConfig( userId: string, sourceId: string, - data: { enabled: boolean; config: unknown }, + data: { enabled: boolean; config?: unknown }, ): Promise { const provider = this.providers.get(sourceId) if (!provider) { throw new SourceNotFoundError(sourceId, userId) } - if (provider.configSchema) { + if (provider.configSchema && data.config !== undefined) { const validated = provider.configSchema(data.config) if (validated instanceof type.errors) { throw new InvalidSourceConfigError(sourceId, validated.summary) } } + const config = data.config ?? {} await sources(this.db, userId).upsertConfig(sourceId, { enabled: data.enabled, - config: data.config, + config, }) const session = this.sessions.get(userId) @@ -193,7 +192,7 @@ export class UserSessionManager { if (!data.enabled) { session.removeSource(sourceId) } else { - const source = await provider.feedSourceForUser(userId, data.config) + const source = await provider.feedSourceForUser(userId, config) if (session.hasSource(sourceId)) { session.replaceSource(sourceId, source) } else { diff --git a/apps/aelis-backend/src/sources/http.test.ts b/apps/aelis-backend/src/sources/http.test.ts index 2b70b49..3bd92c9 100644 --- a/apps/aelis-backend/src/sources/http.test.ts +++ b/apps/aelis-backend/src/sources/http.test.ts @@ -287,6 +287,31 @@ describe("PATCH /api/sources/:sourceId", () => { expect(body.error).toContain("Invalid JSON") }) + test("returns 400 when request body contains unknown fields", async () => { + activeStore = createInMemoryStore() + activeStore.seed(MOCK_USER_ID, "aelis.weather") + const { app } = createApp([createStubProvider("aelis.weather", weatherConfig)], MOCK_USER_ID) + + const res = await patch(app, "aelis.weather", { + enabled: true, + unknownField: "hello", + }) + + expect(res.status).toBe(400) + }) + + test("returns 400 when weather config contains unknown fields", async () => { + activeStore = createInMemoryStore() + activeStore.seed(MOCK_USER_ID, "aelis.weather") + const { app } = createApp([createStubProvider("aelis.weather", weatherConfig)], MOCK_USER_ID) + + const res = await patch(app, "aelis.weather", { + config: { units: "metric", unknownField: "hello" }, + }) + + expect(res.status).toBe(400) + }) + test("returns 400 when weather config fails validation", async () => { activeStore = createInMemoryStore() activeStore.seed(MOCK_USER_ID, "aelis.weather") @@ -410,7 +435,7 @@ describe("PATCH /api/sources/:sourceId", () => { removeSpy.mockRestore() }) - test("accepts location source with arbitrary config (no schema)", async () => { + test("returns 400 when config is provided for source without schema", async () => { activeStore = createInMemoryStore() activeStore.seed(MOCK_USER_ID, "aelis.location") const { app } = createApp([createStubProvider("aelis.location")], MOCK_USER_ID) @@ -419,7 +444,19 @@ describe("PATCH /api/sources/:sourceId", () => { config: { something: "value" }, }) - expect(res.status).toBe(204) + expect(res.status).toBe(400) + }) + + test("returns 400 when empty config is provided for source without schema", async () => { + activeStore = createInMemoryStore() + activeStore.seed(MOCK_USER_ID, "aelis.location") + const { app } = createApp([createStubProvider("aelis.location")], MOCK_USER_ID) + + const res = await patch(app, "aelis.location", { + config: {}, + }) + + expect(res.status).toBe(400) }) test("updates enabled on location source", async () => { @@ -493,6 +530,31 @@ describe("PUT /api/sources/:sourceId", () => { expect(res.status).toBe(400) }) + test("returns 400 when request body contains unknown fields", async () => { + activeStore = createInMemoryStore() + const { app } = createApp([createStubProvider("aelis.weather", weatherConfig)], MOCK_USER_ID) + + const res = await put(app, "aelis.weather", { + enabled: true, + config: { units: "metric" }, + unknownField: "hello", + }) + + expect(res.status).toBe(400) + }) + + test("returns 400 when weather config contains unknown fields", async () => { + activeStore = createInMemoryStore() + const { app } = createApp([createStubProvider("aelis.weather", weatherConfig)], MOCK_USER_ID) + + const res = await put(app, "aelis.weather", { + enabled: true, + config: { units: "metric", unknownField: "hello" }, + }) + + expect(res.status).toBe(400) + }) + test("returns 400 when config fails schema validation", async () => { activeStore = createInMemoryStore() const { app } = createApp([createStubProvider("aelis.weather", weatherConfig)], MOCK_USER_ID) @@ -611,7 +673,7 @@ describe("PUT /api/sources/:sourceId", () => { expect(session.hasSource("aelis.weather")).toBe(true) }) - test("accepts location source with arbitrary config (no schema)", async () => { + test("returns 400 when config is provided for source without schema", async () => { activeStore = createInMemoryStore() const { app } = createApp([createStubProvider("aelis.location")], MOCK_USER_ID) @@ -620,9 +682,29 @@ describe("PUT /api/sources/:sourceId", () => { config: { something: "value" }, }) + expect(res.status).toBe(400) + }) + + test("returns 400 when empty config is provided for source without schema", async () => { + activeStore = createInMemoryStore() + const { app } = createApp([createStubProvider("aelis.location")], MOCK_USER_ID) + + const res = await put(app, "aelis.location", { + enabled: true, + config: {}, + }) + + expect(res.status).toBe(400) + }) + + test("returns 204 without config field for source without schema", async () => { + activeStore = createInMemoryStore() + const { app } = createApp([createStubProvider("aelis.location")], MOCK_USER_ID) + + const res = await put(app, "aelis.location", { + enabled: true, + }) + expect(res.status).toBe(204) - const row = activeStore.rows.get(`${MOCK_USER_ID}:aelis.location`) - expect(row).toBeDefined() - expect(row!.config).toEqual({ something: "value" }) }) }) diff --git a/apps/aelis-backend/src/sources/http.ts b/apps/aelis-backend/src/sources/http.ts index 570a628..23d23f3 100644 --- a/apps/aelis-backend/src/sources/http.ts +++ b/apps/aelis-backend/src/sources/http.ts @@ -20,15 +20,22 @@ interface SourcesHttpHandlersDeps { } const UpdateSourceConfigRequestBody = type({ + "+": "reject", "enabled?": "boolean", "config?": "unknown", }) const ReplaceSourceConfigRequestBody = type({ + "+": "reject", enabled: "boolean", config: "unknown", }) +const ReplaceSourceConfigNoConfigRequestBody = type({ + "+": "reject", + enabled: "boolean", +}) + export function registerSourcesHttpHandlers( app: Hono, { sessionManager, authSessionMiddleware }: SourcesHttpHandlersDeps, @@ -90,6 +97,10 @@ async function handleUpdateSource(c: Context) { return c.json({ error: parsed.summary }, 400) } + if (!provider.configSchema && "config" in parsed) { + return c.json({ error: `Source "${sourceId}" does not accept config` }, 400) + } + const { enabled, config: newConfig } = parsed const user = c.get("user")! @@ -131,12 +142,16 @@ async function handleReplaceSource(c: Context) { return c.json({ error: "Invalid JSON" }, 400) } - const parsed = ReplaceSourceConfigRequestBody(body) + const schema = provider.configSchema + ? ReplaceSourceConfigRequestBody + : ReplaceSourceConfigNoConfigRequestBody + const parsed = schema(body) if (parsed instanceof type.errors) { return c.json({ error: parsed.summary }, 400) } - const { enabled, config } = parsed + const { enabled } = parsed + const config = "config" in parsed ? parsed.config : undefined const user = c.get("user")! try { diff --git a/apps/aelis-backend/src/tfl/provider.ts b/apps/aelis-backend/src/tfl/provider.ts index 9d937f3..e57fd67 100644 --- a/apps/aelis-backend/src/tfl/provider.ts +++ b/apps/aelis-backend/src/tfl/provider.ts @@ -8,6 +8,7 @@ export type TflSourceProviderOptions = | { apiKey?: never; client: ITflApi } export const tflConfig = type({ + "+": "reject", "lines?": "string[]", }) diff --git a/apps/aelis-backend/src/weather/provider.ts b/apps/aelis-backend/src/weather/provider.ts index e1dcb33..1ce11a3 100644 --- a/apps/aelis-backend/src/weather/provider.ts +++ b/apps/aelis-backend/src/weather/provider.ts @@ -9,6 +9,7 @@ export interface WeatherSourceProviderOptions { } export const weatherConfig = type({ + "+": "reject", "units?": "'metric' | 'imperial'", "hourlyLimit?": "number", "dailyLimit?": "number",