security(audit): log owner-bypass reads in GET /pets/:id/profile-summary (GRO-2062)
Adds a defense-in-depth audit row to impersonationAuditLogs when the staff-side owner-bypass path fires. Mirrors the failure-isolation pattern in src/middleware/portalAudit.ts: insert failures are logged and swallowed so a working read can never turn into a 500. - New writeOwnerBypassAudit helper called only when isOwner === true. - No DB migration; petId + actorStaffId go inside metadata jsonb. - resolveImpersonationClientId stays pure (no audit side effects). - Positive + negative tests + a cross-tenant regression test. - UAT_PLAYBOOK.md §3.19d: TC-API-3.19d documents the audit assertion. Parent tracking: GRO-2062 (Paperclip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,7 @@ import {
|
||||
eq,
|
||||
exists,
|
||||
getDb,
|
||||
impersonationAuditLogs,
|
||||
impersonationSessions,
|
||||
or,
|
||||
pets,
|
||||
@@ -156,6 +157,40 @@ async function resolveImpersonationClientId(
|
||||
return session.clientId;
|
||||
}
|
||||
|
||||
/**
|
||||
* Defense-in-depth audit write for the staff-side owner-bypass path in
|
||||
* GET /pets/:id/profile-summary. Mirrors the failure-isolation pattern in
|
||||
* src/middleware/portalAudit.ts: errors are logged but never thrown, so a
|
||||
* misbehaving audit insert cannot turn a working read into a 500.
|
||||
*
|
||||
* Called only when the owner-bypass actually fires (i.e. the requester is a
|
||||
* groomer-role staff row with no appointment linkage, but supplies a valid
|
||||
* X-Impersonation-Session-Id whose clientId matches the pet's owner). The
|
||||
* `petId` and `actorStaffId` are written inside `metadata` because the
|
||||
* impersonation_audit_logs schema has no first-class columns for them and
|
||||
* adding a migration is out of scope.
|
||||
*/
|
||||
async function writeOwnerBypassAudit(
|
||||
db: ReturnType<typeof getDb>,
|
||||
args: {
|
||||
sessionId: string;
|
||||
petId: string;
|
||||
actorStaffId: string;
|
||||
pageVisited: string;
|
||||
}
|
||||
): Promise<void> {
|
||||
try {
|
||||
await db.insert(impersonationAuditLogs).values({
|
||||
sessionId: args.sessionId,
|
||||
action: "read_profile_summary",
|
||||
pageVisited: args.pageVisited,
|
||||
metadata: { petId: args.petId, actorStaffId: args.actorStaffId },
|
||||
});
|
||||
} catch (err) {
|
||||
console.error("[pets] failed to write owner-bypass audit log:", err);
|
||||
}
|
||||
}
|
||||
|
||||
petsRouter.get("/:id/profile-summary", async (c) => {
|
||||
const db = getDb();
|
||||
const petId = c.req.param("id");
|
||||
@@ -188,8 +223,22 @@ petsRouter.get("/:id/profile-summary", async (c) => {
|
||||
// `groomer` staff row with no appointment linkage.
|
||||
let isOwner = false;
|
||||
if (isGroomer) {
|
||||
const headerSessionId = c.req.header("X-Impersonation-Session-Id");
|
||||
const ownerClientId = await resolveImpersonationClientId(db, c);
|
||||
isOwner = !!ownerClientId && ownerClientId === pet.clientId;
|
||||
if (isOwner && headerSessionId) {
|
||||
// GRO-2063: defense-in-depth audit row. Only fires when the bypass
|
||||
// is actually granted; never on the normal groomer-linkage path,
|
||||
// 403/404/401, or when the header is absent. Failure is swallowed
|
||||
// (try/catch inside writeOwnerBypassAudit) so this can never turn a
|
||||
// working read into a 500.
|
||||
await writeOwnerBypassAudit(db, {
|
||||
sessionId: headerSessionId,
|
||||
petId: pet.id,
|
||||
actorStaffId: staffRow.id,
|
||||
pageVisited: c.req.path,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Groomer RBAC: check appointment linkage to this pet's client
|
||||
|
||||
Reference in New Issue
Block a user