docs: close a fail-open and three gaps in the Task 4 identity plan

CRITICAL: importSubjectContext never validated selfApp, and verifyJwt skips
the audience check entirely when audience is undefined. So an undefined
selfApp disabled the only cross-app binding in the system and accepted every
token from every app for every audience. Not hypothetical - the natural feed
is helpers' currentAppName(), which returns string | undefined. The mint side
already hard-fails on a missing app name; the import side did not.

A non-string tenant id was silently dropped at both ends. A numeric tenant id
is the common DB-backed case, and a callee reading a missing tenantId as
"global" is a cross-tenant exposure. Now refused, symmetric with the subject
check.

Token lifetime was unbounded: verifyJwt only checks exp when present, so a
token minted without one never expired, and a caller passing a large
ttlSeconds produced a long-lived impersonation credential the callee
honoured. exp is now required and age is bounded by maxAge independently.

targetApp was unvalidated, so passing an array minted one token valid at
several apps - exactly what the audience binding exists to prevent.

Also documents callerApp as self-asserted rather than authenticated
provenance, since the signing secret is workspace-wide.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-05 14:21:38 +05:30
co-authored by Claude Opus 5
parent 2257ee871e
commit 1393a8a3b8
@@ -669,7 +669,7 @@ git commit -m "feat(rpc): add defineService and the immutable procedure builder"
**Interfaces:**
- Consumes: `signJwt`, `verifyJwt`, `JwtError` from `@wrnexus/jwt`; `Context` type from `@wrnexus/core`
- Produces: `exportSubjectContext(ctx, target, options?): Promise<string | undefined>`, `importSubjectContext(token, selfApp, options?): Promise<SubjectContext>`, `interface SubjectContext { subjectId: string; tenantId?: string; callerApp: string }`, `rpcSecret(): string`, `RPC_IDENTITY_HEADER`
- Produces: `exportSubjectContext(ctx, target, options?): Promise<string | undefined>`, `importSubjectContext(token, selfApp, options?): Promise<SubjectContext>`, `interface ImportOptions { maxAgeSeconds?: number }`, `interface SubjectContext { subjectId: string; tenantId?: string; callerApp: string }`, `rpcSecret(): string`, `RPC_IDENTITY_HEADER`
- [ ] **Step 1: Write the failing test**
@@ -769,6 +769,44 @@ describe("subject context token", () => {
).rejects.toThrow();
});
test("an empty selfApp is refused rather than disabling the audience check", async () => {
configure();
const token = await exportSubjectContext(ctxFor({ id: "u1" }), "billing");
// verifyJwt skips the audience check when audience is undefined, so this
// would otherwise accept every token from every app.
for (const bad of [undefined, "", null]) {
await expect(importSubjectContext(token!, bad as never)).rejects.toThrow(/selfApp/);
}
});
test("a token with a huge ttl is still rejected once it exceeds maxAge", async () => {
configure();
const token = await exportSubjectContext(ctxFor({ id: "u1" }), "billing", {
ttlSeconds: 31_536_000,
});
await expect(importSubjectContext(token!, "billing", { maxAgeSeconds: -1 })).rejects.toThrow();
});
test("an array targetApp is refused, so no token is valid at two apps", async () => {
configure();
await expect(
exportSubjectContext(ctxFor({ id: "u1" }), ["billing", "reports"] as never),
).rejects.toThrow(/targetApp/);
});
test("a non-string tenant id is refused rather than silently dropped", async () => {
configure();
// Silently dropping it leaves the callee reading "no tenant" as "global".
for (const tenant of [42, {}, ""]) {
await expect(
exportSubjectContext(
{ user: { id: "u1" }, tenant: { id: tenant }, locals: {} } as unknown as Context,
"billing",
),
).rejects.toThrow(/tenant/i);
}
});
test("a missing secret is a setup error, not a silent pass", async () => {
process.env.WRNEXUS_APP_NAME = "web";
delete process.env.WRNEXUS_RPC_SECRET;
@@ -803,11 +841,17 @@ export const RPC_IDENTITY_HEADER = "x-wrnexus-rpc-identity";
const MIN_SECRET_LENGTH = 32;
const DEFAULT_TTL_SECONDS = 60;
/** Upper bound on accepted token age, whatever the token's own exp says. */
const DEFAULT_MAX_AGE_SECONDS = 300;
export interface SubjectContext {
subjectId: string;
tenantId?: string;
/** The app that minted the token. */
/**
* The app that CLAIMS to have minted the token. Self-asserted: the signing
* secret is workspace-wide, so any app can set this to any name. Useful for
* logs and tracing; NEVER an authorization input.
*/
callerApp: string;
}
@@ -815,6 +859,15 @@ export interface ExportOptions {
ttlSeconds?: number;
}
export interface ImportOptions {
/**
* Reject a token older than this regardless of its own `exp`, so a caller
* that mints with a huge ttlSeconds cannot create a long-lived
* impersonation credential the callee will honour. Defaults to 300s.
*/
maxAgeSeconds?: number;
}
/**
* The workspace-wide RPC signing secret.
*
@@ -871,9 +924,26 @@ export async function exportSubjectContext(
"WRN-RPC-SUBJECT: subject id must be a non-empty string; coerce numeric ids with String(id).",
);
}
const tenantId = ctx.tenant?.id;
// An array here would mint one token valid at SEVERAL apps, defeating the
// audience binding that stops app B replaying A's token against app C.
if (typeof targetApp !== "string" || targetApp === "") {
throw new Error("WRN-RPC-AUDIENCE: targetApp must be a non-empty string.");
}
// A numeric tenant id is the common DB-backed case. Dropping it silently
// would leave the callee reading "no tenant" as "global", which is a
// cross-tenant exposure — so refuse it the same way a bad subject is refused.
const rawTenant: unknown = ctx.tenant?.id;
if (
rawTenant !== undefined &&
rawTenant !== null &&
(typeof rawTenant !== "string" || rawTenant === "")
) {
throw new Error(
"WRN-RPC-TENANT: tenant id must be a non-empty string; coerce numeric ids with String(id).",
);
}
return signJwt(
{ sub: rawId, ...(typeof tenantId === "string" && tenantId ? { tenant: tenantId } : {}) },
{ sub: rawId, ...(rawTenant ? { tenant: rawTenant as string } : {}) },
rpcSecret(),
{
issuer: callerAppName(),
@@ -892,21 +962,41 @@ export async function exportSubjectContext(
export async function importSubjectContext(
token: string,
selfApp: string,
options: ImportOptions = {},
): Promise<SubjectContext> {
const claims = await verifyJwt<{ sub?: string; tenant?: string; iss?: string }>(
token,
rpcSecret(),
{ audience: selfApp },
);
// verifyJwt SKIPS the audience check entirely when audience is undefined, so
// an empty selfApp would disable the only cross-app binding in the system and
// accept every token from every app. currentAppName() returns
// `string | undefined`, which is exactly how that gets passed by accident.
if (typeof selfApp !== "string" || selfApp === "") {
throw new Error("WRN-RPC-AUDIENCE: selfApp must be a non-empty string.");
}
const claims = await verifyJwt<{
sub?: string;
tenant?: unknown;
iss?: string;
exp?: number;
}>(token, rpcSecret(), {
audience: selfApp,
maxAge: options.maxAgeSeconds ?? DEFAULT_MAX_AGE_SECONDS,
});
// verifyJwt only checks exp when it is present, so a token minted without
// one never expires. Require it.
if (typeof claims.exp !== "number") {
throw new Error("WRN-RPC-IDENTITY: token has no expiry.");
}
if (typeof claims.sub !== "string" || claims.sub === "") {
throw new Error("WRN-RPC-IDENTITY: token carries no usable subject.");
}
if (typeof claims.iss !== "string" || claims.iss === "") {
throw new Error("WRN-RPC-IDENTITY: token names no calling app.");
}
if (claims.tenant !== undefined && (typeof claims.tenant !== "string" || claims.tenant === "")) {
throw new Error("WRN-RPC-IDENTITY: token carries an unusable tenant.");
}
return {
subjectId: claims.sub,
tenantId: typeof claims.tenant === "string" && claims.tenant ? claims.tenant : undefined,
tenantId: claims.tenant as string | undefined,
callerApp: claims.iss,
};
}