Skip to content

Commit 5f9a72d

Browse files
committed
fix(settings): address PR review feedback for settings page
Record last_backup_at on export, document the provider SSRF carve-out, add 401 auth tests for new routes, and polish settings UX/docs.
1 parent 9092bc6 commit 5f9a72d

9 files changed

Lines changed: 125 additions & 4 deletions

File tree

AGENTS.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,16 @@ Test helper: `authedRequest(url, init)` in `src/db/test-utils.ts` signs a sessio
231231
- See the App Security Baseline design spec
232232
(docs/superpowers/specs/2026-06-19-app-security-baseline-design.md §7)
233233
for the full policy.
234+
- **Carve-out — admin-configured provider endpoints.** Chat-provider
235+
connections in `src/lib/settings/provider-connection.ts` (Hermes,
236+
OpenCode Go) intentionally do **not** use `validateFetchUrl`. The base
237+
URLs are operator-only settings saved through the authenticated settings
238+
route, and the defaults point at local services (e.g.
239+
`http://localhost:8642/v1`) that the private-range guard would block by
240+
design. These are trusted, admin-supplied destinations — not arbitrary
241+
user input — so the SSRF guard does not apply. Any new fetch of a URL
242+
that originates from an unauthenticated or non-admin source MUST still go
243+
through `validateFetchUrl`.
234244

235245
## Frontend Guidelines
236246

src/app/api/__tests__/settings.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import { GET, PATCH } from "@/app/api/settings/route";
1010
import { POST as POST_TEST } from "@/app/api/settings/test-connection/route";
1111
import { GET as GET_SYSTEM_INFO } from "@/app/api/settings/system-info/route";
1212
import { GET as GET_EXPORT } from "@/app/api/export/route";
13+
import { GET as GET_OPENROUTER_MODELS } from "@/app/api/settings/openrouter/models/route";
14+
import { GET as GET_PROVIDER_MODELS } from "@/app/api/settings/provider-models/route";
1315

1416
describe("/api/settings", () => {
1517
beforeEach(() => {
@@ -140,4 +142,75 @@ describe("/api/settings", () => {
140142
const body = await res.text();
141143
expect(body.startsWith("[")).toBe(true);
142144
});
145+
146+
it("records last_backup_at on a successful export", async () => {
147+
const res = await GET_EXPORT(
148+
await authedGet("http://localhost/api/export?format=json")
149+
);
150+
expect(res.status).toBe(200);
151+
152+
const db = getDb();
153+
const lastBackupAt = settings.get(db, "last_backup_at");
154+
expect(lastBackupAt).not.toBeNull();
155+
expect(() => new Date(lastBackupAt as string).toISOString()).not.toThrow();
156+
});
157+
158+
describe("unauthenticated access", () => {
159+
it("rejects GET /api/settings with 401", async () => {
160+
const res = await GET(new Request("http://localhost/api/settings"));
161+
expect(res.status).toBe(401);
162+
});
163+
164+
it("rejects PATCH /api/settings with 401", async () => {
165+
const res = await PATCH(
166+
new Request("http://localhost/api/settings", {
167+
method: "PATCH",
168+
headers: { "Content-Type": "application/json" },
169+
body: JSON.stringify({ ai_model: "openai/gpt-4" }),
170+
})
171+
);
172+
expect(res.status).toBe(401);
173+
});
174+
175+
it("rejects POST /api/settings/test-connection with 401", async () => {
176+
const res = await POST_TEST(
177+
new Request("http://localhost/api/settings/test-connection", {
178+
method: "POST",
179+
headers: { "Content-Type": "application/json" },
180+
body: JSON.stringify({ provider: "hermes" }),
181+
})
182+
);
183+
expect(res.status).toBe(401);
184+
});
185+
186+
it("rejects GET /api/settings/system-info with 401", async () => {
187+
const res = await GET_SYSTEM_INFO(
188+
new Request("http://localhost/api/settings/system-info")
189+
);
190+
expect(res.status).toBe(401);
191+
});
192+
193+
it("rejects GET /api/settings/openrouter/models with 401", async () => {
194+
const res = await GET_OPENROUTER_MODELS(
195+
new Request("http://localhost/api/settings/openrouter/models")
196+
);
197+
expect(res.status).toBe(401);
198+
});
199+
200+
it("rejects GET /api/settings/provider-models with 401", async () => {
201+
const res = await GET_PROVIDER_MODELS(
202+
new Request(
203+
"http://localhost/api/settings/provider-models?provider=opencode-go"
204+
)
205+
);
206+
expect(res.status).toBe(401);
207+
});
208+
209+
it("rejects GET /api/export with 401", async () => {
210+
const res = await GET_EXPORT(
211+
new Request("http://localhost/api/export?format=json")
212+
);
213+
expect(res.status).toBe(401);
214+
});
215+
});
143216
});

src/app/api/export/route.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getDb, contentItems } from "@/db/index";
1+
import { getDb, contentItems, settings } from "@/db/index";
22
import { errorResponse, logServerError } from "@/lib/api";
33
import { requireAuthenticated } from "@/lib/auth/guard";
44
import { log } from "@/lib/logger";
@@ -42,7 +42,13 @@ export async function GET(request: Request) {
4242
}
4343

4444
const items = listAllItems();
45-
const exportedAt = new Date().toISOString().slice(0, 10);
45+
const now = new Date();
46+
const exportedAt = now.toISOString().slice(0, 10);
47+
48+
// A successful export is the closest thing to a backup the app
49+
// performs today, so record it as the last backup timestamp. The
50+
// System info card surfaces this value.
51+
settings.set(getDb(), "last_backup_at", now.toISOString());
4652

4753
log("info", "content exported", {
4854
event: "export.content",

src/app/api/settings/route.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import { toPublicSettings } from "@/lib/settings/public";
1313

1414
const secretPatchValue = z.union([z.string(), z.null()]);
1515

16+
// Keep in sync with `SettingsDraft` in `src/app/settings/types.ts` and
17+
// `DRAFT_KEYS` in `src/app/settings/dirty.ts` when adding or removing a
18+
// setting. `.strict()` rejects any key not listed here.
1619
const patchSchema = z
1720
.object({
1821
openrouter_api_key: secretPatchValue.optional(),
@@ -70,6 +73,10 @@ export async function PATCH(request: Request) {
7073
return errorResponse("VALIDATION_ERROR", "No settings to update", 400);
7174
}
7275

76+
// Defense-in-depth against schema drift: `patchSchema` is `.strict()`
77+
// and only lists writable keys, so a parsed body can't currently carry
78+
// an unknown or read-only key. These guards fail closed if someone later
79+
// adds a read-only key (e.g. `last_backup_at`) to the schema by mistake.
7380
for (const [key] of entries) {
7481
if (!isSettingsKey(key)) {
7582
return errorResponse("VALIDATION_ERROR", "Invalid input", 400, {

src/app/api/settings/system-info/route.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ import { log } from "@/lib/logger";
77
function formatBytes(bytes: number): string {
88
if (bytes < 1024) return `${bytes} B`;
99
if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB`;
10-
return `${(bytes / (1024 * 1024)).toFixed(1)} MB`;
10+
if (bytes < 1024 * 1024 * 1024)
11+
return `${(bytes / (1024 * 1024)).toFixed(1)} MB`;
12+
return `${(bytes / (1024 * 1024 * 1024)).toFixed(1)} GB`;
1113
}
1214

1315
export async function GET(request: Request) {

src/app/settings/dirty.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type { SettingsDraft, SettingsSnapshot } from "./types";
22

3+
// Keep in sync with `SettingsDraft` in `./types.ts` and `patchSchema` in
4+
// `src/app/api/settings/route.ts` when adding or removing a setting.
35
const DRAFT_KEYS: Array<keyof SettingsDraft> = [
46
"openrouter_api_key",
57
"ai_model",

src/app/settings/settings-page.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"use client";
22

3-
import { useMemo, useState } from "react";
3+
import { useEffect, useMemo, useState } from "react";
44

55
import { Button } from "@/components/ui/button";
66
import { AiFeaturesSection } from "./ai-features-section";
@@ -34,6 +34,13 @@ export function SettingsPage() {
3434
return isSettingsDirty(saved, draft, clearedSecrets);
3535
}, [saved, draft, clearedSecrets]);
3636

37+
// Auto-dismiss the "Settings saved." confirmation so it doesn't linger.
38+
useEffect(() => {
39+
if (!saveMessage) return;
40+
const timer = setTimeout(() => setSaveMessage(null), 4000);
41+
return () => clearTimeout(timer);
42+
}, [saveMessage]);
43+
3744
async function handleSave() {
3845
if (!saved || !draft) return;
3946
setSaving(true);

src/app/settings/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import type { PublicSettings } from "@/lib/settings/public";
22

3+
// The writable settings shape is mirrored in three places that must stay in
4+
// sync when a setting is added or removed:
5+
// 1. `SettingsDraft` below (the client draft shape),
6+
// 2. `DRAFT_KEYS` / `SECRET_KEYS` in `./dirty.ts` (dirty-tracking + patch),
7+
// 3. `patchSchema` in `src/app/api/settings/route.ts` (server validation).
38
export type SettingsDraft = {
49
openrouter_api_key: string;
510
ai_model: string;

src/lib/settings/provider-connection.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
11
import type Database from "better-sqlite3";
22
import { getSettingValue } from "@/lib/settings/public";
33

4+
// NOTE: SSRF policy exemption. The fetches below intentionally do NOT run
5+
// through `validateFetchUrl` (src/lib/ssrf.ts). The target base URLs are
6+
// admin-only chat-provider endpoints saved via the authenticated settings
7+
// route, and the defaults point at local services (e.g. Hermes at
8+
// `http://localhost:8642/v1`) that `validateFetchUrl` would block by design.
9+
// These are trusted, operator-configured destinations — not arbitrary
10+
// user-supplied URLs — so the private-range guard does not apply. See
11+
// AGENTS.md (SSRF Protection) for the carve-out.
12+
413
export type ChatProvider = "hermes" | "opencode-go";
514

615
export type ProviderConnectionResult =

0 commit comments

Comments
 (0)