Compare commits

...

1 Commits

Author SHA1 Message Date
Flea Flicker 5f01df819e fix(GRO-2299): redact googleMapsApiKey from PATCH /api/admin/settings response
CI / Test (pull_request) Successful in 24s
CI / Lint & Typecheck (pull_request) Successful in 27s
CI / Build & Push Docker Images (pull_request) Successful in 1m18s
The PATCH handler returned the full businessSettings row via .returning(),
echoing the encrypted googleMapsApiKey ciphertext back to the caller. Wrap the
return in the existing redactSettings() helper (after a !updated guard) so
redaction is applied symmetrically with the GET projection (GRO-2294).

- src/routes/settings.ts: guard + redactSettings(updated) on PATCH return
- src/__tests__/settings.test.ts: assert PATCH omits googleMapsApiKey
  (existing-row and auto-create-then-update branches)
- UAT_PLAYBOOK.md §13 TC-API-13.2: assert PATCH response omits the secret

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-09 06:50:20 +00:00
3 changed files with 57 additions and 2 deletions
+1 -1
View File
@@ -333,7 +333,7 @@ This means:
| # | Scenario | Steps | Expected |
|---|----------|-------|----------|
| TC-API-13.1 | Get business settings | GET /api/admin/settings | 200 OK, business settings returned. Response body **must NOT include `googleMapsApiKey`** — the encrypted secret is redacted from the projection (GRO-2294, defense-in-depth); non-secret fields (`businessName`, colors, `routeOptimizationProvider`, etc.) are still present |
| TC-API-13.2 | Update business settings | PATCH /api/admin/settings with updated values | 200 OK, settings updated |
| TC-API-13.2 | Update business settings | PATCH /api/admin/settings with updated values | 200 OK, settings updated. Response body **must NOT include `googleMapsApiKey`** — the encrypted secret is redacted from the PATCH response symmetrically with the GET projection (GRO-2299, defense-in-depth); non-secret updated fields are still returned |
| TC-API-13.3 | Upload logo | POST /api/admin/settings/logo/upload with file | 200 OK, logo uploaded and stored |
| TC-API-13.4 | View logo | GET /api/admin/settings/logo | 200 OK, logo image returned |
| TC-API-13.5 | Delete logo | DELETE /api/admin/settings/logo | 200 OK, logo removed |
+54
View File
@@ -7,6 +7,7 @@ import { Hono } from "hono";
let selectRows: Record<string, unknown>[] = [];
let insertReturning: Record<string, unknown>[] = [];
let updateReturning: Record<string, unknown>[] = [];
function makeChainable(data: unknown[]): unknown {
const arr = [...data];
@@ -33,6 +34,9 @@ vi.mock("@groombook/db", () => {
insert: () => ({
values: () => ({ returning: () => insertReturning }),
}),
update: () => ({
set: () => ({ where: () => ({ returning: () => updateReturning }) }),
}),
}),
businessSettings,
eq: vi.fn(),
@@ -51,6 +55,17 @@ const { settingsRouter } = await import("../routes/settings.js");
const app = new Hono();
app.route("/settings", settingsRouter);
// PATCH /settings is guarded by requireSuperUser(), which reads the staff record
// from context. Inject a super-user staff row so the handler runs.
const patchApp = new Hono<{
Variables: { staff: { id: string; isSuperUser: boolean } };
}>();
patchApp.use("*", async (c, next) => {
c.set("staff", { id: "staff-1", isSuperUser: true });
await next();
});
patchApp.route("/settings", settingsRouter);
const FULL_ROW = {
id: "settings-uuid-1",
businessName: "GroomBook",
@@ -89,3 +104,42 @@ describe("GET /settings — googleMapsApiKey redaction (GRO-2294)", () => {
expect(body.id).toBe("settings-uuid-new");
});
});
describe("PATCH /settings — googleMapsApiKey redaction (GRO-2299)", () => {
beforeEach(() => {
selectRows = [];
insertReturning = [];
updateReturning = [];
});
function patchRequest(body: Record<string, unknown>) {
return patchApp.request("/settings", {
method: "PATCH",
headers: { "content-type": "application/json" },
body: JSON.stringify(body),
});
}
it("omits googleMapsApiKey from the PATCH response", async () => {
selectRows = [{ ...FULL_ROW }];
updateReturning = [{ ...FULL_ROW, businessName: "Updated Name" }];
const res = await patchRequest({ businessName: "Updated Name" });
expect(res.status).toBe(200);
const body = (await res.json()) as Record<string, unknown>;
expect(body).not.toHaveProperty("googleMapsApiKey");
// Non-secret updated fields are still returned.
expect(body.businessName).toBe("Updated Name");
expect(body.routeOptimizationProvider).toBe("google");
});
it("omits googleMapsApiKey on the auto-create-then-update branch", async () => {
selectRows = [];
insertReturning = [{ ...FULL_ROW, id: "settings-uuid-new" }];
updateReturning = [{ ...FULL_ROW, id: "settings-uuid-new" }];
const res = await patchRequest({ primaryColor: "#123456" });
expect(res.status).toBe(200);
const body = (await res.json()) as Record<string, unknown>;
expect(body).not.toHaveProperty("googleMapsApiKey");
expect(body.id).toBe("settings-uuid-new");
});
});
+2 -1
View File
@@ -65,7 +65,8 @@ settingsRouter.patch(
.where(eq(businessSettings.id, settingsId))
.returning();
return c.json(updated);
if (!updated) throw new Error("Failed to update settings");
return c.json(redactSettings(updated));
}
);