security: bound JSONB inputs + whitelist batchUpdateCustomFields keys (#48)
batchUpdateCustomFields used $executeRaw to merge a manager-supplied
record straight into Resource.dynamicFields with no key whitelist —
so a manager could pollute the JSONB namespace with arbitrary keys
(e.g. ones admin tools later interpret). Separately, several user-facing
JSONB fields (allocation/demand metadata, dynamicFields) were typed as
unbounded z.record(z.string(), z.unknown()), letting clients ship
multi-MB payloads that flow into DB writes, audit logs, and SSE frames.
- Add BoundedJsonRecord helper (shared) — 64 keys / depth 4 /
8 KB strings / 32 KB serialized total. Conservative defaults; call
sites needing more should use a strict object schema.
- Apply BoundedJsonRecord to the highest-traffic untrusted JSONB inputs:
allocation metadata (Create/CreateDemandRequirement/CreateAssignment),
resource & project dynamicFields, and the createDemand router input.
- batchUpdateCustomFields:
* Tighten input schema (key length, value bounds, max 100 keys).
* Fetch each target resource and verify all input keys are in the
union of (specific blueprint defs) ∪ (active global RESOURCE
blueprint defs) for that resource. Empty whitelist → reject all
keys (stricter than create/update, but appropriate for a bulk
escape-hatch endpoint).
* Run the existing per-key value validator afterwards.
* 404 if any requested id does not exist (was silently skipped).
- New helper getAllowedDynamicFieldKeys() in blueprint-validation.
- 7 new BoundedJsonRecord tests, 2 new batchUpdateCustomFields tests
covering the whitelist-rejection and not-found paths.
Covers EAPPS 3.2.7 (input bounds) / OWASP A03 (injection / mass assignment).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,7 @@ import {
|
||||
updateDemandRequirement,
|
||||
} from "@capakraken/application";
|
||||
import {
|
||||
BoundedJsonRecord,
|
||||
CreateDemandRequirementSchema,
|
||||
FillDemandRequirementSchema,
|
||||
FillOpenDemandByAllocationSchema,
|
||||
@@ -53,7 +54,7 @@ export const allocationDemandProcedures = {
|
||||
startDate: z.coerce.date(),
|
||||
endDate: z.coerce.date(),
|
||||
budgetCents: z.number().int().min(0).optional(),
|
||||
metadata: z.record(z.string(), z.unknown()).optional(),
|
||||
metadata: BoundedJsonRecord.optional(),
|
||||
}),
|
||||
)
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
|
||||
@@ -78,3 +78,51 @@ export async function assertBlueprintDynamicFields({
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the set of dynamic-field keys allowed for a blueprint (specific + all
|
||||
* active global blueprints for the target). Used to whitelist keys in bulk-
|
||||
* update paths (`batchUpdateCustomFields`) where value-only validation would
|
||||
* silently accept attacker-injected keys into the JSONB namespace.
|
||||
*/
|
||||
export async function getAllowedDynamicFieldKeys({
|
||||
db,
|
||||
blueprintId,
|
||||
target,
|
||||
}: {
|
||||
db: BlueprintLookup;
|
||||
blueprintId: string | undefined;
|
||||
target: BlueprintTarget;
|
||||
}): Promise<Set<string>> {
|
||||
const allowed = new Set<string>();
|
||||
|
||||
if (blueprintId) {
|
||||
const blueprint = await db.blueprint.findUnique({
|
||||
where: { id: blueprintId },
|
||||
select: { fieldDefs: true, target: true },
|
||||
});
|
||||
if (blueprint) {
|
||||
if (blueprint.target !== target) {
|
||||
throw new TRPCError({
|
||||
code: "BAD_REQUEST",
|
||||
message: `${target} entities require a ${target.toLowerCase()} blueprint`,
|
||||
});
|
||||
}
|
||||
for (const def of blueprint.fieldDefs as BlueprintFieldDefinition[]) {
|
||||
allowed.add(def.key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const globals = await db.blueprint.findMany({
|
||||
where: { target, isGlobal: true, isActive: true },
|
||||
select: { fieldDefs: true },
|
||||
});
|
||||
for (const bp of globals) {
|
||||
for (const def of bp.fieldDefs as BlueprintFieldDefinition[]) {
|
||||
allowed.add(def.key);
|
||||
}
|
||||
}
|
||||
|
||||
return allowed;
|
||||
}
|
||||
|
||||
@@ -11,7 +11,10 @@ import { z } from "zod";
|
||||
import { findUniqueOrThrow } from "../db/helpers.js";
|
||||
import { ROLE_BRIEF_SELECT } from "../db/selects.js";
|
||||
import { adminProcedure, managerProcedure, requirePermission } from "../trpc.js";
|
||||
import { assertBlueprintDynamicFields } from "./blueprint-validation.js";
|
||||
import {
|
||||
assertBlueprintDynamicFields,
|
||||
getAllowedDynamicFieldKeys,
|
||||
} from "./blueprint-validation.js";
|
||||
|
||||
export const resourceMutationProcedures = {
|
||||
create: managerProcedure
|
||||
@@ -322,12 +325,59 @@ export const resourceMutationProcedures = {
|
||||
.input(
|
||||
z.object({
|
||||
ids: z.array(z.string()).min(1).max(100),
|
||||
fields: z.record(z.string(), z.union([z.string(), z.number(), z.boolean(), z.null()])),
|
||||
fields: z
|
||||
.record(
|
||||
z.string().min(1).max(128),
|
||||
z.union([z.string().max(8_000), z.number(), z.boolean(), z.null()]),
|
||||
)
|
||||
.refine((r) => Object.keys(r).length <= 100, {
|
||||
message: "Too many custom-field keys in one batch (max 100)",
|
||||
}),
|
||||
}),
|
||||
)
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
requirePermission(ctx, PermissionKey.MANAGE_RESOURCES);
|
||||
|
||||
// Whitelist input keys against the union of (each resource's blueprint
|
||||
// field defs) ∪ (all active global RESOURCE blueprints). Rejects any key
|
||||
// that is not explicitly defined for every target resource — blocks
|
||||
// namespace pollution and privilege escalation via admin-tool
|
||||
// interpretation of attacker-placed JSONB keys.
|
||||
const resources = await ctx.db.resource.findMany({
|
||||
where: { id: { in: input.ids } },
|
||||
select: { id: true, blueprintId: true },
|
||||
});
|
||||
if (resources.length !== input.ids.length) {
|
||||
throw new TRPCError({ code: "NOT_FOUND", message: "One or more resources not found" });
|
||||
}
|
||||
|
||||
const inputKeys = Object.keys(input.fields);
|
||||
for (const resource of resources) {
|
||||
const allowed = await getAllowedDynamicFieldKeys({
|
||||
db: ctx.db,
|
||||
blueprintId: resource.blueprintId ?? undefined,
|
||||
target: BlueprintTarget.RESOURCE,
|
||||
});
|
||||
// If no blueprint at all is registered for this resource, `allowed` is
|
||||
// empty — we still enforce the whitelist to refuse any key rather than
|
||||
// silently accepting arbitrary JSONB. This is stricter than the legacy
|
||||
// create/update paths but correct for a bulk endpoint.
|
||||
const unknownKey = inputKeys.find((k) => !allowed.has(k));
|
||||
if (unknownKey !== undefined) {
|
||||
throw new TRPCError({
|
||||
code: "UNPROCESSABLE_CONTENT",
|
||||
message: `Unknown dynamic-field key "${unknownKey}" for resource ${resource.id}`,
|
||||
});
|
||||
}
|
||||
// Still validate values via the existing per-key typed validator.
|
||||
await assertBlueprintDynamicFields({
|
||||
db: ctx.db,
|
||||
blueprintId: resource.blueprintId ?? undefined,
|
||||
dynamicFields: input.fields,
|
||||
target: BlueprintTarget.RESOURCE,
|
||||
});
|
||||
}
|
||||
|
||||
await ctx.db.$transaction(async (tx) => {
|
||||
await Promise.all(
|
||||
input.ids.map(
|
||||
|
||||
Reference in New Issue
Block a user