From 1622f5ec3a0c6efbbdec7afa576777437ea71894 Mon Sep 17 00:00:00 2001 From: pq198363-ops <246611021+pq198363-ops@users.noreply.github.com> Date: Sat, 4 Jul 2026 10:07:12 +0800 Subject: [PATCH] security: validate agent and service identifiers --- README.md | 14 +++++++ src/identifiers.test.ts | 87 +++++++++++++++++++++++++++++++++++++++++ src/identifiers.ts | 22 +++++++++++ src/routes/services.ts | 41 ++++++++++++++----- src/routes/usage.ts | 57 +++++++++++++++++++++------ 5 files changed, 200 insertions(+), 21 deletions(-) create mode 100644 src/identifiers.test.ts create mode 100644 src/identifiers.ts diff --git a/README.md b/README.md index 337ee4f..143b000 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,20 @@ agentpay-backend/ stroops, `priceStroops`, `billedStroops`, `/api/v1/billing/*`, and why `POST /api/v1/settle` drains backend counters without moving funds. +## Identifier Rules + +`agent` and `serviceId` values are used in in-memory keys, path parameters, +events, and exports, so they are restricted to unambiguous ASCII identifiers. +Use only letters, numbers, `.`, `_`, and `-`. + +| Field | Max length | Examples | +| ----------- | ---------- | ----------------------- | +| `agent` | 256 chars | `agent_1`, `team.alpha` | +| `serviceId` | 128 chars | `svc-1`, `embed.v2` | + +Values containing the internal `::` separator, whitespace, commas, newlines, or +control characters are rejected with `400 invalid_request`. + ## Quickstart Start a local backend on `http://localhost:3001` with the checked-in diff --git a/src/identifiers.test.ts b/src/identifiers.test.ts new file mode 100644 index 0000000..1e89bb1 --- /dev/null +++ b/src/identifiers.test.ts @@ -0,0 +1,87 @@ +import assert from "node:assert"; +import { describe, it } from "node:test"; +import request from "supertest"; +import { app } from "./index.js"; + +void describe("agent and service identifier validation", () => { + void it("rejects unsafe single usage identifiers", async () => { + const badAgent = await request(app) + .post("/api/v1/usage") + .send({ agent: "bad::agent", serviceId: "safe-service", requests: 1 }); + + const badService = await request(app) + .post("/api/v1/usage") + .send({ agent: "safe-agent", serviceId: "bad\nservice", requests: 1 }); + + assert.strictEqual(badAgent.status, 400); + assert.strictEqual(badAgent.body.error, "invalid_request"); + assert.strictEqual(badService.status, 400); + assert.strictEqual(badService.body.error, "invalid_request"); + }); + + void it("marks unsafe bulk usage and service entries invalid", async () => { + const usage = await request(app) + .post("/api/v1/usage/bulk") + .send({ + items: [ + { agent: "bad::agent", serviceId: "safe-service", requests: 1 }, + { agent: "safe-agent", serviceId: "bad service", requests: 1 }, + ], + }); + + const services = await request(app) + .post("/api/v1/services/bulk") + .send({ + items: [ + { serviceId: "bad::service", priceStroops: 1 }, + { serviceId: "safe-service", priceStroops: 1 }, + ], + }); + + assert.strictEqual(usage.status, 201); + assert.deepStrictEqual( + usage.body.results.map((item: { ok: boolean; error?: string }) => ({ + ok: item.ok, + error: item.error, + })), + [ + { ok: false, error: "invalid_item" }, + { ok: false, error: "invalid_item" }, + ] + ); + + assert.strictEqual(services.status, 201); + assert.strictEqual(services.body.results[0].ok, false); + assert.strictEqual(services.body.results[0].error, "invalid_item"); + assert.strictEqual(services.body.results[1].ok, true); + }); + + void it("rejects unsafe settlement and path identifiers", async () => { + const settle = await request(app) + .post("/api/v1/settle") + .send({ agent: "bad::agent", serviceId: "safe-service" }); + + const usageRead = await request(app).get("/api/v1/usage/bad::agent/safe-service"); + const agentTotal = await request(app).get("/api/v1/agents/bad::agent/total"); + const serviceRead = await request(app).get("/api/v1/services/bad::service"); + + assert.strictEqual(settle.status, 400); + assert.strictEqual(settle.body.error, "invalid_request"); + assert.strictEqual(usageRead.status, 400); + assert.strictEqual(usageRead.body.error, "invalid_request"); + assert.strictEqual(agentTotal.status, 400); + assert.strictEqual(agentTotal.body.error, "invalid_request"); + assert.strictEqual(serviceRead.status, 400); + assert.strictEqual(serviceRead.body.error, "invalid_request"); + }); + + void it("accepts safe identifier characters", async () => { + const created = await request(app) + .post("/api/v1/usage") + .send({ agent: "agent_1.2-3", serviceId: "service_1.2-3", requests: 1 }); + + assert.strictEqual(created.status, 201); + assert.strictEqual(created.body.agent, "agent_1.2-3"); + assert.strictEqual(created.body.serviceId, "service_1.2-3"); + }); +}); diff --git a/src/identifiers.ts b/src/identifiers.ts new file mode 100644 index 0000000..aaee911 --- /dev/null +++ b/src/identifiers.ts @@ -0,0 +1,22 @@ +export const AGENT_ID_MAX_LENGTH = 256; +export const SERVICE_ID_MAX_LENGTH = 128; + +const SAFE_IDENTIFIER_RE = /^[A-Za-z0-9._-]+$/; + +function isValidIdentifier(value: unknown, maxLength: number): value is string { + return ( + typeof value === "string" && + value.length > 0 && + value.length <= maxLength && + !value.includes("::") && + SAFE_IDENTIFIER_RE.test(value) + ); +} + +export function isValidAgentId(value: unknown): value is string { + return isValidIdentifier(value, AGENT_ID_MAX_LENGTH); +} + +export function isValidServiceId(value: unknown): value is string { + return isValidIdentifier(value, SERVICE_ID_MAX_LENGTH); +} diff --git a/src/routes/services.ts b/src/routes/services.ts index c606e5c..ce64f3d 100644 --- a/src/routes/services.ts +++ b/src/routes/services.ts @@ -1,5 +1,6 @@ import { createHash } from "node:crypto"; import { Router, type Request, type Response } from "express"; +import { isValidServiceId } from "../identifiers.js"; import { servicesDisabled, servicesMetadata, @@ -16,6 +17,25 @@ type ServiceReadShape = { owner?: string; }; +function invalidServiceIdMessage(): string { + return "serviceId must be 1-128 chars using only letters, numbers, dot, underscore, or hyphen"; +} + +function rejectInvalidServicePath( + req: Request, + res: Response, + serviceId: unknown +): boolean { + if (isValidServiceId(serviceId)) return false; + + res.status(400).json({ + error: "invalid_request", + message: invalidServiceIdMessage(), + requestId: getRequestId(req), + }); + return true; +} + /** * Builds the public read shape for service detail and list endpoints. */ @@ -56,9 +76,7 @@ export function createServicesRouter(): Router { (it: { serviceId?: unknown; priceStroops?: unknown }, i: number) => { const { serviceId, priceStroops } = it ?? {}; if ( - typeof serviceId !== "string" || - serviceId.length === 0 || - serviceId.length > 128 || + !isValidServiceId(serviceId) || typeof priceStroops !== "number" || !Number.isInteger(priceStroops) || priceStroops < 0 @@ -80,14 +98,10 @@ export function createServicesRouter(): Router { router.post("/api/v1/services", (req: Request, res: Response) => { const { serviceId, priceStroops } = req.body ?? {}; const requestId = getRequestId(req); - if ( - typeof serviceId !== "string" || - serviceId.length === 0 || - serviceId.length > 128 - ) { + if (!isValidServiceId(serviceId)) { res.status(400).json({ error: "invalid_request", - message: "serviceId must be a non-empty string up to 128 chars", + message: invalidServiceIdMessage(), requestId, }); return; @@ -111,6 +125,7 @@ export function createServicesRouter(): Router { router.get("/api/v1/services/:serviceId/usage", (req: Request, res: Response) => { const { serviceId } = req.params; + if (rejectInvalidServicePath(req, res, serviceId)) return; const suffix = `::${serviceId}`; let total = 0; let agents = 0; @@ -127,6 +142,7 @@ export function createServicesRouter(): Router { "/api/v1/services/:serviceId/agents/top", (req: Request, res: Response) => { const { serviceId } = req.params; + if (rejectInvalidServicePath(req, res, serviceId)) return; const limit = Math.min( 100, Math.max(1, Number((req.query.limit as string) ?? 10)) @@ -145,6 +161,7 @@ export function createServicesRouter(): Router { router.get("/api/v1/services/:serviceId/agents", (req: Request, res: Response) => { const { serviceId } = req.params; + if (rejectInvalidServicePath(req, res, serviceId)) return; const suffix = `::${serviceId}`; const items: { agent: string; total: number }[] = []; for (const [key, total] of usageStore.entries()) { @@ -158,6 +175,7 @@ export function createServicesRouter(): Router { /** Reads one service with its disabled state and optional metadata. */ router.get("/api/v1/services/:serviceId", (req: Request, res: Response) => { const { serviceId } = req.params; + if (rejectInvalidServicePath(req, res, serviceId)) return; const meta = servicesStore.get(serviceId); if (!meta) { res.status(404).json({ @@ -173,6 +191,7 @@ export function createServicesRouter(): Router { router.put("/api/v1/services/:serviceId/metadata", (req: Request, res: Response) => { const { serviceId } = req.params; const requestId = getRequestId(req); + if (rejectInvalidServicePath(req, res, serviceId)) return; if (!servicesStore.has(serviceId)) { res.status(404).json({ error: "not_found", @@ -204,6 +223,7 @@ export function createServicesRouter(): Router { router.get("/api/v1/services/:serviceId/metadata", (req: Request, res: Response) => { const { serviceId } = req.params; + if (rejectInvalidServicePath(req, res, serviceId)) return; const meta = servicesMetadata.get(serviceId); if (!meta) { res.status(404).json({ @@ -221,6 +241,7 @@ export function createServicesRouter(): Router { (req: Request, res: Response) => { const { serviceId } = req.params; const requestId = getRequestId(req); + if (rejectInvalidServicePath(req, res, serviceId)) return; if (!servicesStore.has(serviceId)) { res.status(404).json({ error: "not_found", @@ -247,6 +268,7 @@ export function createServicesRouter(): Router { router.patch("/api/v1/services/:serviceId/price", (req: Request, res: Response) => { const { serviceId } = req.params; const requestId = getRequestId(req); + if (rejectInvalidServicePath(req, res, serviceId)) return; const meta = servicesStore.get(serviceId); if (!meta) { res.status(404).json({ @@ -276,6 +298,7 @@ export function createServicesRouter(): Router { router.delete("/api/v1/services/:serviceId", (req: Request, res: Response) => { const { serviceId } = req.params; + if (rejectInvalidServicePath(req, res, serviceId)) return; if (!servicesStore.has(serviceId)) { res.status(404).json({ error: "not_found", diff --git a/src/routes/usage.ts b/src/routes/usage.ts index cd28a50..e6f53ef 100644 --- a/src/routes/usage.ts +++ b/src/routes/usage.ts @@ -1,4 +1,5 @@ import { Router, type Request, type Response } from "express"; +import { isValidAgentId, isValidServiceId } from "../identifiers.js"; import { recordEvent } from "../events.js"; import { servicesDisabled, @@ -21,6 +22,28 @@ type BillingTotalBreakdown = { unpricedRequests: number; }; +function invalidIdentifierMessage(kind: "agent" | "serviceId"): string { + const max = kind === "agent" ? 256 : 128; + return `${kind} must be 1-${max} chars using only letters, numbers, dot, underscore, or hyphen`; +} + +function rejectInvalidPathIdentifier( + req: Request, + res: Response, + kind: "agent" | "serviceId", + value: unknown +): boolean { + const valid = kind === "agent" ? isValidAgentId(value) : isValidServiceId(value); + if (valid) return false; + + res.status(400).json({ + error: "invalid_request", + message: invalidIdentifierMessage(kind), + requestId: getRequestId(req), + }); + return true; +} + /** * Builds usage, billing, settlement, and agent rollup routes. */ @@ -31,22 +54,18 @@ export function createUsageRouter(): Router { const { agent, serviceId, requests } = req.body ?? {}; const requestId = getRequestId(req); - if (typeof agent !== "string" || agent.length === 0 || agent.length > 256) { + if (!isValidAgentId(agent)) { res.status(400).json({ error: "invalid_request", - message: "agent must be a non-empty string up to 256 chars", + message: invalidIdentifierMessage("agent"), requestId, }); return; } - if ( - typeof serviceId !== "string" || - serviceId.length === 0 || - serviceId.length > 128 - ) { + if (!isValidServiceId(serviceId)) { res.status(400).json({ error: "invalid_request", - message: "serviceId must be a non-empty string up to 128 chars", + message: invalidIdentifierMessage("serviceId"), requestId, }); return; @@ -93,8 +112,8 @@ export function createUsageRouter(): Router { for (let i = 0; i < items.length; i++) { const { agent, serviceId, requests } = items[i] ?? {}; if ( - typeof agent !== "string" || - typeof serviceId !== "string" || + !isValidAgentId(agent) || + !isValidServiceId(serviceId) || typeof requests !== "number" || !Number.isInteger(requests) || requests <= 0 @@ -122,6 +141,12 @@ export function createUsageRouter(): Router { router.get("/api/v1/usage/:agent/:serviceId", (req: Request, res: Response) => { const { agent, serviceId } = req.params; + if ( + rejectInvalidPathIdentifier(req, res, "agent", agent) || + rejectInvalidPathIdentifier(req, res, "serviceId", serviceId) + ) { + return; + } const total = usageStore.get(usageKey(agent, serviceId)) ?? 0; res.json({ agent, serviceId, total }); }); @@ -180,6 +205,12 @@ export function createUsageRouter(): Router { router.get("/api/v1/billing/:agent/:serviceId", (req: Request, res: Response) => { const { agent, serviceId } = req.params; + if ( + rejectInvalidPathIdentifier(req, res, "agent", agent) || + rejectInvalidPathIdentifier(req, res, "serviceId", serviceId) + ) { + return; + } const requests = usageStore.get(usageKey(agent, serviceId)) ?? 0; const price = servicesStore.get(serviceId)?.priceStroops ?? 0; res.json({ @@ -194,10 +225,10 @@ export function createUsageRouter(): Router { router.post("/api/v1/settle", (req: Request, res: Response) => { const { agent, serviceId } = req.body ?? {}; const requestId = getRequestId(req); - if (typeof agent !== "string" || typeof serviceId !== "string") { + if (!isValidAgentId(agent) || !isValidServiceId(serviceId)) { res.status(400).json({ error: "invalid_request", - message: "agent and serviceId are required strings", + message: "agent and serviceId must be safe identifiers", requestId, }); return; @@ -224,6 +255,7 @@ export function createUsageRouter(): Router { router.get("/api/v1/agents/:agent/total", (req: Request, res: Response) => { const { agent } = req.params; + if (rejectInvalidPathIdentifier(req, res, "agent", agent)) return; const prefix = `${agent}::`; let total = 0; for (const [key, n] of usageStore.entries()) { @@ -234,6 +266,7 @@ export function createUsageRouter(): Router { router.get("/api/v1/agents/:agent/usage", (req: Request, res: Response) => { const { agent } = req.params; + if (rejectInvalidPathIdentifier(req, res, "agent", agent)) return; const prefix = `${agent}::`; const items: { serviceId: string; total: number }[] = []; for (const [key, total] of usageStore.entries()) {