harden: address adversarial review of Pods CRUD
Backend: atomic create with insert-retry-on-duplicate-key (removes slug TOCTOU race + wrong 400s), resilient per-index creation so a unique-index failure can't silently drop the constraint or block seeding, idempotent race-safe seeding via $setOnInsert, type validation + size caps on all pod fields/members, removeMember no-ops without a write, /api/outcome guarded. Frontend: per-pod busy state (one mutation no longer disables every card), joined pod derived from the list (can't go stale), create keeps inputs on error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -48,13 +48,23 @@ export async function initMemory(): Promise<void> {
|
||||
const db = await getDb();
|
||||
await db.command({ ping: 1 });
|
||||
const c = await collections();
|
||||
await Promise.all([
|
||||
c.pods.createIndex({ id: 1 }, { unique: true }),
|
||||
c.observations.createIndex({ podId: 1, observedAt: -1 }),
|
||||
c.observations.createIndex({ engineerId: 1 }),
|
||||
c.collisions.createIndex({ podId: 1, detectedAt: -1 }),
|
||||
c.interventions.createIndex({ collisionId: 1 }),
|
||||
c.outcomes.createIndex({ interventionId: 1 }),
|
||||
]);
|
||||
// Create each index independently so one failure (e.g. the unique pods index
|
||||
// failing on pre-existing duplicate ids) doesn't abort the others or block
|
||||
// seeding. Failures are logged loudly rather than silently swallowed.
|
||||
const indexes: Array<[string, () => Promise<unknown>]> = [
|
||||
['pods.id (unique)', () => c.pods.createIndex({ id: 1 }, { unique: true })],
|
||||
['observations.podId', () => c.observations.createIndex({ podId: 1, observedAt: -1 })],
|
||||
['observations.engineerId', () => c.observations.createIndex({ engineerId: 1 })],
|
||||
['collisions.podId', () => c.collisions.createIndex({ podId: 1, detectedAt: -1 })],
|
||||
['interventions.collisionId', () => c.interventions.createIndex({ collisionId: 1 })],
|
||||
['outcomes.interventionId', () => c.outcomes.createIndex({ interventionId: 1 })],
|
||||
];
|
||||
for (const [name, make] of indexes) {
|
||||
try {
|
||||
await make();
|
||||
} catch (err) {
|
||||
console.error(`[memory] index "${name}" failed: ${(err as Error).message}`);
|
||||
}
|
||||
}
|
||||
console.log(`[memory] mongo connected -> ${db.databaseName}`);
|
||||
}
|
||||
|
||||
+78
-36
@@ -3,6 +3,12 @@ import { collections } from '../memory/db.js';
|
||||
|
||||
const NO_ID = { projection: { _id: 0 } } as const;
|
||||
|
||||
const MAX_NAME = 120;
|
||||
const MAX_REPO = 140;
|
||||
const MAX_DESCRIPTION = 2000;
|
||||
const MAX_MEMBERS = 50;
|
||||
const MAX_MEMBER_LEN = 80;
|
||||
|
||||
function slugify(name: string): string {
|
||||
return name
|
||||
.toLowerCase()
|
||||
@@ -11,22 +17,43 @@ function slugify(name: string): string {
|
||||
.replace(/^-+|-+$/g, '');
|
||||
}
|
||||
|
||||
/** Trim, drop empties, de-dupe case-insensitively while preserving order. */
|
||||
/** Validate + normalize a string field. Throws (-> 400) on type/length errors. */
|
||||
function str(value: unknown, field: string, max: number, required = false): string | undefined {
|
||||
if (value === undefined || value === null) {
|
||||
if (required) throw new Error(`${field} is required`);
|
||||
return undefined;
|
||||
}
|
||||
if (typeof value !== 'string') throw new Error(`${field} must be a string`);
|
||||
const trimmed = value.trim();
|
||||
if (required && !trimmed) throw new Error(`${field} is required`);
|
||||
if (trimmed.length > max) throw new Error(`${field} too long (max ${max})`);
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
/** Validate, trim, length-cap, de-dupe (case-insensitive), and limit count. */
|
||||
function cleanMembers(members: unknown): string[] {
|
||||
if (!Array.isArray(members)) return [];
|
||||
if (members === undefined || members === null) return [];
|
||||
if (!Array.isArray(members)) throw new Error('members must be an array');
|
||||
const seen = new Set<string>();
|
||||
const out: string[] = [];
|
||||
for (const raw of members) {
|
||||
const name = String(raw).trim();
|
||||
if (typeof raw !== 'string') throw new Error('member names must be strings');
|
||||
const name = raw.trim();
|
||||
if (name.length > MAX_MEMBER_LEN) throw new Error(`member name too long (max ${MAX_MEMBER_LEN})`);
|
||||
const key = name.toLowerCase();
|
||||
if (name && !seen.has(key)) {
|
||||
seen.add(key);
|
||||
out.push(name);
|
||||
}
|
||||
}
|
||||
if (out.length > MAX_MEMBERS) throw new Error(`too many members (max ${MAX_MEMBERS})`);
|
||||
return out;
|
||||
}
|
||||
|
||||
function isDuplicateKey(err: unknown): boolean {
|
||||
return (err as { code?: number })?.code === 11000;
|
||||
}
|
||||
|
||||
function now(): string {
|
||||
return new Date().toISOString();
|
||||
}
|
||||
@@ -41,42 +68,49 @@ export async function getPod(id: string): Promise<Pod | null> {
|
||||
return c.pods.findOne({ id }, NO_ID);
|
||||
}
|
||||
|
||||
async function uniqueSlug(base: string): Promise<string> {
|
||||
const c = await collections();
|
||||
const root = base || 'pod';
|
||||
let slug = root;
|
||||
let n = 2;
|
||||
while (await c.pods.findOne({ id: slug })) slug = `${root}-${n++}`;
|
||||
return slug;
|
||||
}
|
||||
|
||||
export async function createPod(input: PodInput): Promise<Pod> {
|
||||
const name = (input.name ?? '').trim();
|
||||
if (!name) throw new Error('name is required');
|
||||
const name = str(input.name, 'name', MAX_NAME, true)!;
|
||||
const repo = str(input.repo, 'repo', MAX_REPO) ?? '';
|
||||
const description = str(input.description, 'description', MAX_DESCRIPTION);
|
||||
const members = cleanMembers(input.members);
|
||||
const c = await collections();
|
||||
const ts = now();
|
||||
const pod: Pod = {
|
||||
id: await uniqueSlug(slugify(name)),
|
||||
name,
|
||||
repo: (input.repo ?? '').trim(),
|
||||
description: input.description?.trim() || undefined,
|
||||
members: cleanMembers(input.members),
|
||||
createdAt: ts,
|
||||
updatedAt: ts,
|
||||
};
|
||||
await c.pods.insertOne({ ...pod });
|
||||
return pod;
|
||||
const base = slugify(name) || 'pod';
|
||||
|
||||
// Atomic create: let the unique index arbitrate slug collisions. On a
|
||||
// duplicate-key error, bump the suffix and retry — no check-then-insert race.
|
||||
for (let suffix = 1; suffix <= MAX_MEMBERS + 50; suffix++) {
|
||||
const pod: Pod = {
|
||||
id: suffix === 1 ? base : `${base}-${suffix}`,
|
||||
name,
|
||||
repo,
|
||||
description: description || undefined,
|
||||
members,
|
||||
createdAt: ts,
|
||||
updatedAt: ts,
|
||||
};
|
||||
try {
|
||||
await c.pods.insertOne({ ...pod });
|
||||
return pod;
|
||||
} catch (err) {
|
||||
if (isDuplicateKey(err)) continue;
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
throw new Error('could not allocate a unique pod id');
|
||||
}
|
||||
|
||||
export async function updatePod(id: string, patch: PodInput): Promise<Pod | null> {
|
||||
const set: Partial<Pod> = { updatedAt: now() };
|
||||
if (patch.name !== undefined) {
|
||||
const name = patch.name.trim();
|
||||
const name = str(patch.name, 'name', MAX_NAME);
|
||||
if (!name) throw new Error('name cannot be empty');
|
||||
set.name = name;
|
||||
}
|
||||
if (patch.repo !== undefined) set.repo = patch.repo.trim();
|
||||
if (patch.description !== undefined) set.description = patch.description.trim() || undefined;
|
||||
if (patch.repo !== undefined) set.repo = str(patch.repo, 'repo', MAX_REPO) ?? '';
|
||||
if (patch.description !== undefined) {
|
||||
set.description = str(patch.description, 'description', MAX_DESCRIPTION) || undefined;
|
||||
}
|
||||
if (patch.members !== undefined) set.members = cleanMembers(patch.members);
|
||||
const c = await collections();
|
||||
const updated = await c.pods.findOneAndUpdate(
|
||||
@@ -103,26 +137,32 @@ async function setMembers(id: string, members: string[]): Promise<Pod | null> {
|
||||
return updated ?? null;
|
||||
}
|
||||
|
||||
export async function addMember(id: string, rawName: string): Promise<Pod | null> {
|
||||
export async function addMember(id: string, rawName: unknown): Promise<Pod | null> {
|
||||
if (typeof rawName !== 'string') throw new Error('member name must be a string');
|
||||
const name = rawName.trim();
|
||||
if (!name) throw new Error('member name is required');
|
||||
if (name.length > MAX_MEMBER_LEN) throw new Error(`member name too long (max ${MAX_MEMBER_LEN})`);
|
||||
const pod = await getPod(id);
|
||||
if (!pod) return null;
|
||||
if (pod.members.some((m) => m.toLowerCase() === name.toLowerCase())) return pod;
|
||||
if (pod.members.length >= MAX_MEMBERS) throw new Error(`too many members (max ${MAX_MEMBERS})`);
|
||||
return setMembers(id, [...pod.members, name]);
|
||||
}
|
||||
|
||||
export async function removeMember(id: string, rawName: string): Promise<Pod | null> {
|
||||
const name = rawName.trim().toLowerCase();
|
||||
const name = String(rawName).trim().toLowerCase();
|
||||
const pod = await getPod(id);
|
||||
if (!pod) return null;
|
||||
return setMembers(
|
||||
id,
|
||||
pod.members.filter((m) => m.toLowerCase() !== name),
|
||||
);
|
||||
const filtered = pod.members.filter((m) => m.toLowerCase() !== name);
|
||||
if (filtered.length === pod.members.length) return pod; // no-op: don't write / bump updatedAt
|
||||
return setMembers(id, filtered);
|
||||
}
|
||||
|
||||
/** Insert the default pods once, if the collection is empty. */
|
||||
/**
|
||||
* Seed the default pods into a fresh DB. Idempotent and race-safe: gated on an
|
||||
* empty collection, and uses per-doc upserts so concurrent startups can't
|
||||
* create duplicates. Won't resurrect a default a user later deletes.
|
||||
*/
|
||||
export async function seedDefaultPods(): Promise<void> {
|
||||
const c = await collections();
|
||||
if ((await c.pods.estimatedDocumentCount()) > 0) return;
|
||||
@@ -156,6 +196,8 @@ export async function seedDefaultPods(): Promise<void> {
|
||||
updatedAt: ts,
|
||||
},
|
||||
];
|
||||
await c.pods.insertMany(defaults.map((p) => ({ ...p })));
|
||||
await Promise.all(
|
||||
defaults.map((p) => c.pods.updateOne({ id: p.id }, { $setOnInsert: p }, { upsert: true })),
|
||||
);
|
||||
console.log('[pods] seeded default pods');
|
||||
}
|
||||
|
||||
+10
-2
@@ -51,8 +51,16 @@ app.post('/api/sync-pr', async (req, res) => {
|
||||
|
||||
// Outcome ACK -> closes the continual-learning policy loop.
|
||||
app.post('/api/outcome', async (req, res) => {
|
||||
await recordOutcome(req.body as InterventionOutcome);
|
||||
res.json({ ok: true });
|
||||
try {
|
||||
const o = (req.body ?? {}) as Partial<InterventionOutcome>;
|
||||
if (typeof o.interventionId !== 'string' || !o.interventionId) {
|
||||
return res.status(400).json({ error: 'interventionId is required' });
|
||||
}
|
||||
await recordOutcome(req.body as InterventionOutcome);
|
||||
res.json({ ok: true });
|
||||
} catch (e) {
|
||||
res.status(500).json({ error: (e as Error).message });
|
||||
}
|
||||
});
|
||||
|
||||
// Memory counts — quick way to confirm Mongo persistence is working.
|
||||
|
||||
Reference in New Issue
Block a user