mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(backend): harden principal child isolation

Browse Source
This commit is contained in:
User
2026-07-16 18:38:40 +02:00
parent 458eb13c89
commit b454fb478b
9 changed files with 167 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.superpowers/sdd/task-5-report.md
+21
View File
@@ -48,3 +48,24 @@ enforced, new sessions had no trusted principal binding, and settings were globa
- Existing dependency-injected route fakes without `sessionShow` retain a narrow test seam;
production `ThtRunner` always has that method, so deployed requests cannot bypass the
repository authorization check.
## Review follow-up
### RED
Focused regressions initially failed exactly at the three review findings: stale ambient
display names survived into both `tht` and Pi child environments; mutation/document runner
methods dropped the selected workspace; and `expandLocalHome` did not exist.
### GREEN
- Child environments now remove all four `THT_PRINCIPAL_*` keys from their cloned base
environment before applying the exact request principal. Regression tests prove an absent
display name does not inherit a stale ambient value in either child path.
- `setName`, `setGroup`, `archive`, `unarchive`, and `documents` now take and retain an
optional workspace. The rename route regression proves `session show` authorization and
the mutation use the same non-default workspace.
- Local principal paths expand `~`/`~/...`; existing local home and identity file modes are
repaired to POSIX `0700`/`0600` when applicable, with Windows left unchanged.
- Focused suite: `74 passed`; full backend suite: `213 passed` across `22` files, followed by
TypeScript typecheck, production build, and diff check.
backend/src/auth/principal.ts
+24 -2
View File
@@ -1,4 +1,4 @@
import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
import { chmodSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
import { homedir } from "node:os";
import { join } from "node:path";
import { randomUUID } from "node:crypto";
@@ -10,6 +10,25 @@ export interface PrincipalContext {
isAdmin: boolean;
}
const principalEnvKeys = [
"THT_PRINCIPAL_ISSUER", "THT_PRINCIPAL_SUBJECT", "THT_PRINCIPAL_DISPLAY_NAME", "THT_PRINCIPAL_IS_ADMIN",
] as const;
export function clearPrincipalEnvironment(env: NodeJS.ProcessEnv): void {
for (const key of principalEnvKeys) delete env[key];
}
export function expandLocalHome(path: string, home = homedir()): string {
if (path === "~") return home;
if (path.startsWith("~/")) return join(home, path.slice(2));
return path;
}
function harden(path: string, mode: number): void {
if (process.platform === "win32") return;
try { chmodSync(path, mode); } catch { /* best-effort parity with harness local storage */ }
}
const invalid = (value: string) => value.length === 0 || value.length > 512 || /[\u0000-\u001f\u007f]/.test(value);
function required(value: unknown): string | undefined {
@@ -34,12 +53,14 @@ export function upstreamPrincipal(headers: Record<string, unknown>): PrincipalCo
}
export function localPrincipal(): PrincipalContext {
const home = process.env.THT_HOME ?? join(homedir(), ".thothii");
const home = expandLocalHome(process.env.THT_HOME ?? join(homedir(), ".thothii"));
const identityPath = join(home, "identity.json");
mkdirSync(home, { recursive: true, mode: 0o700 });
harden(home, 0o700);
try {
const stored = JSON.parse(readFileSync(identityPath, "utf8"));
if (stored?.issuer === "local" && typeof stored.subject === "string" && /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(stored.subject)) {
harden(identityPath, 0o600);
return { issuer: "local", subject: stored.subject, isAdmin: false };
}
throw new Error("invalid local identity");
@@ -48,6 +69,7 @@ export function localPrincipal(): PrincipalContext {
const principal = { issuer: "local", subject: randomUUID() };
try {
writeFileSync(identityPath, JSON.stringify(principal) + "\n", { mode: 0o600, flag: "wx" });
harden(identityPath, 0o600);
return { ...principal, isAdmin: false };
} catch (writeError: any) {
// Another local request won the identity creation race; always converge on its UUID.
backend/src/pi/pi-process-manager.ts
+2 -1
View File
@@ -5,7 +5,7 @@ import { SessionBridge } from "../bridge/session-bridge.js";
import type { ThtRunner } from "../tht/tht-runner.js";
import { buildPiChildEnv, canonicalPiProvider } from "./provider-credentials.js";
import { secretValue } from "../config/secret-bundle.js";
import { principalEnvironment, type PrincipalContext } from "../auth/principal.js";
import { clearPrincipalEnvironment, principalEnvironment, type PrincipalContext } from "../auth/principal.js";
export interface SessionRuntime {
rpc: RpcClient;
@@ -55,6 +55,7 @@ export class PiProcessManager {
credentialFile: this.cfg.modelApiKeyFile,
additions: { THT_SESSION: sessionId, THT_AUTHOR: author },
});
clearPrincipalEnvironment(env);
if (principal) Object.assign(env, principalEnvironment(principal));
// The Thoth gate executes the deterministic `tht` CLI as a Pi tool. Give only
// this managed session process the adapter values already loaded by the core
backend/src/routes/sessions.ts
+5 -5
View File
@@ -397,7 +397,7 @@ export function sessionRoutes(
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).setName(id, (req.body as any).name);
await runnerFor(principal).setName(id, (req.body as any).name, settings.workspace);
} catch { return storageFailure(reply); }
return reply.code(204).send();
});
@@ -407,7 +407,7 @@ export function sessionRoutes(
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).setGroup(id, (req.body as any).group);
await runnerFor(principal).setGroup(id, (req.body as any).group, settings.workspace);
} catch { return storageFailure(reply); }
return reply.code(204).send();
});
@@ -417,7 +417,7 @@ export function sessionRoutes(
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).archive(id);
await runnerFor(principal).archive(id, settings.workspace);
} catch { return storageFailure(reply); }
return reply.code(204).send();
});
@@ -427,7 +427,7 @@ export function sessionRoutes(
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).unarchive(id);
await runnerFor(principal).unarchive(id, settings.workspace);
} catch { return storageFailure(reply); }
return reply.code(204).send();
});
@@ -458,7 +458,7 @@ export function sessionRoutes(
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
return await runnerFor(principal).documents(id);
return await runnerFor(principal).documents(id, settings.workspace);
} catch { return storageFailure(reply); }
});
}
backend/src/tht/tht-runner.ts
+7 -6
View File
@@ -1,7 +1,7 @@
import { spawn } from "node:child_process";
import { existsSync } from "node:fs";
import { join } from "node:path";
import { principalEnvironment, type PrincipalContext } from "../auth/principal.js";
import { clearPrincipalEnvironment, principalEnvironment, type PrincipalContext } from "../auth/principal.js";
export interface ThtConfig {
thtBin: string;
@@ -68,6 +68,7 @@ export class ThtRunner {
return new Promise((resolve) => {
const env: NodeJS.ProcessEnv = { ...process.env };
delete env.THT_DATA_ROOT;
clearPrincipalEnvironment(env);
if (this.cfg.dataRoot !== undefined) env.THT_DATA_ROOT = this.cfg.dataRoot;
if (this.principal) Object.assign(env, principalEnvironment(this.principal));
const ch = spawn(this.cfg.thtBin, this.buildArgv(args, workspace), {
@@ -159,15 +160,15 @@ export class ThtRunner {
closeSession(id: string, workspace?: string) { return this.ok(["session", "close", id], workspace); }
failSession(id: string, workspace?: string) { return this.ok(["session", "fail", id], workspace); }
reopenSession(id: string, workspace?: string) { return this.ok(["session", "reopen", id], workspace); }
setName(id: string, name: string) { return this.ok(["session", "set-name", id, "--name", name]); }
setGroup(id: string, group: string) { return this.ok(["session", "set-group", id, "--group", group]); }
archive(id: string) { return this.ok(["session", "archive", id]); }
unarchive(id: string) { return this.ok(["session", "unarchive", id]); }
setName(id: string, name: string, workspace?: string) { return this.ok(["session", "set-name", id, "--name", name], workspace); }
setGroup(id: string, group: string, workspace?: string) { return this.ok(["session", "set-group", id, "--group", group], workspace); }
archive(id: string, workspace?: string) { return this.ok(["session", "archive", id], workspace); }
unarchive(id: string, workspace?: string) { return this.ok(["session", "unarchive", id], workspace); }
async deleteSession(id: string, workspace?: string) {
const { code, stderr } = await this.run(["session", "delete", id], workspace);
if (code !== 0) throw new Error(`tht session delete exit ${code}: ${stderr.trim()}`);
}
documents(id: string) { return this.json<SessionDocument[]>(["session", "documents", id, "--json"]); }
documents(id: string, workspace?: string) { return this.json<SessionDocument[]>(["session", "documents", id, "--json"], workspace); }
preferencesGet(workspace?: string) { return this.json<Record<string, unknown>>(["session", "preferences", "get", "--json"], workspace); }
async preferencesSet(preferences: Record<string, unknown>, workspace?: string): Promise<void> {
backend/test/auth.test.ts
+23
View File
@@ -1,6 +1,10 @@
import { test, expect } from "vitest";
import Fastify from "fastify";
import { authPreHandler, getPrincipal } from "../src/auth/auth.js";
import { chmodSync, mkdtempSync, rmSync, statSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { expandLocalHome, localPrincipal } from "../src/auth/principal.js";
test("local mode resolves a stable local principal", async () => {
const app = Fastify();
@@ -46,3 +50,22 @@ test("upstream mode accepts only normalized proxy principal headers", async () =
issuer: "portal", subject: "42", displayName: "Alice", isAdmin: true,
});
});
test("local identity expands tilde homes and restores private POSIX permissions", () => {
expect(expandLocalHome("~/thoth-test", "/home/tester")).toBe("/home/tester/thoth-test");
expect(expandLocalHome("~", "/home/tester")).toBe("/home/tester");
const home = mkdtempSync(join(tmpdir(), "thoth-principal-"));
chmodSync(home, 0o755);
const previous = process.env.THT_HOME;
process.env.THT_HOME = home;
try {
localPrincipal();
if (process.platform !== "win32") {
expect(statSync(home).mode & 0o777).toBe(0o700);
expect(statSync(join(home, "identity.json")).mode & 0o777).toBe(0o600);
}
} finally {
if (previous === undefined) delete process.env.THT_HOME; else process.env.THT_HOME = previous;
rmSync(home, { recursive: true, force: true });
}
});
backend/test/pi-spawn-args.test.ts
+25
View File
@@ -32,3 +32,28 @@ test("production spawnFn launches `pi --mode rpc` with no --approve (pi 0.73 dro
expect(args).not.toContain("--approve");
mgr.teardown("s1");
});
test("Pi child replaces stale principal env and omits absent display names", async () => {
const saved = Object.fromEntries([
"THT_PRINCIPAL_ISSUER", "THT_PRINCIPAL_SUBJECT", "THT_PRINCIPAL_DISPLAY_NAME", "THT_PRINCIPAL_IS_ADMIN",
].map((key) => [key, process.env[key]]));
Object.assign(process.env, {
THT_PRINCIPAL_ISSUER: "stale", THT_PRINCIPAL_SUBJECT: "stale", THT_PRINCIPAL_DISPLAY_NAME: "stale",
THT_PRINCIPAL_IS_ADMIN: "true",
});
try {
(nodeSpawn as any).mockClear();
const mgr = new PiProcessManager(loadConfig({}));
await mgr.spawnFor("s-principal", { principal: { issuer: "portal", subject: "42", isAdmin: false } });
const env = (nodeSpawn as any).mock.calls[0][2].env;
expect(env).toMatchObject({
THT_PRINCIPAL_ISSUER: "portal", THT_PRINCIPAL_SUBJECT: "42", THT_PRINCIPAL_IS_ADMIN: "false",
});
expect(env).not.toHaveProperty("THT_PRINCIPAL_DISPLAY_NAME");
mgr.teardown("s-principal");
} finally {
for (const [key, value] of Object.entries(saved)) {
if (value === undefined) delete process.env[key]; else process.env[key] = value;
}
}
});
backend/test/routes-sessions.test.ts
+21
View File
@@ -1282,6 +1282,27 @@ test("POST /sessions/:id/rename calls setName", async () => {
expect(arg).toEqual({ id: "s1", name: "N" });
});
test("rename authorizes and mutates through the same selected workspace", async () => {
const workspaces: string[] = [];
const app = buildApp(loadConfig({ AUTH_MODE: "upstream", THT_HARNESS_DIR: "../harness" }), {
thtRunner: {
withPrincipal: () => ({
sessionShow: async (_id: string, workspace: string) => { workspaces.push(`show:${workspace}`); return { id: "s1" }; },
setName: async (_id: string, _name: string, workspace: string) => { workspaces.push(`set:${workspace}`); },
}),
} as any,
getSettings: () => ({ workspace: "tenant-a" }) as any,
});
const res = await app.inject({
method: "POST", url: "/sessions/s1/rename", payload: { name: "N" },
headers: {
"x-thoth-principal-issuer": "portal", "x-thoth-principal-subject": "42", "x-thoth-is-admin": "false",
},
});
expect(res.statusCode).toBe(204);
expect(workspaces).toEqual(["show:tenant-a", "set:tenant-a"]);
});
test("POST /sessions/:id/group calls setGroup", async () => {
let arg: any;
const app = mutApp({ setGroup: async (id: string, group: string) => { arg = { id, group }; } });
backend/test/tht-runner.test.ts
+39 -12
View File
@@ -83,6 +83,31 @@ test("run omits ambient THT_DATA_ROOT when config does not provide one", async (
}
});
test("principal-bound tht child replaces stale principal env and omits an absent display name", async () => {
const saved = Object.fromEntries([
"THT_PRINCIPAL_ISSUER", "THT_PRINCIPAL_SUBJECT", "THT_PRINCIPAL_DISPLAY_NAME", "THT_PRINCIPAL_IS_ADMIN",
].map((key) => [key, process.env[key]]));
Object.assign(process.env, {
THT_PRINCIPAL_ISSUER: "stale-issuer", THT_PRINCIPAL_SUBJECT: "stale-subject",
THT_PRINCIPAL_DISPLAY_NAME: "Stale Name", THT_PRINCIPAL_IS_ADMIN: "true",
});
try {
(spawn as any).mockClear();
const runner = new ThtRunner({ thtBin: "tht", harnessDir: "/h", configPath: "config/tht.yaml" })
.withPrincipal({ issuer: "portal", subject: "42", isAdmin: false });
await runner.run(["session", "list", "--json"]);
const env = (spawn as any).mock.calls[0][2].env;
expect(env).toMatchObject({
THT_PRINCIPAL_ISSUER: "portal", THT_PRINCIPAL_SUBJECT: "42", THT_PRINCIPAL_IS_ADMIN: "false",
});
expect(env).not.toHaveProperty("THT_PRINCIPAL_DISPLAY_NAME");
} finally {
for (const [key, value] of Object.entries(saved)) {
if (value === undefined) delete process.env[key]; else process.env[key] = value;
}
}
});
test("run with exit != 0 propagates error with stderr", async () => {
const r = new ThtRunner({ thtBin: "tht", harnessDir: "/h", configPath: "config/tht.yaml" });
r.run = async () => ({ code: 1, stdout: "", stderr: "ERRORE: boom" });
@@ -137,23 +162,25 @@ test("setName builds the right argv", async () => {
const calls: string[][] = [];
const r = new ThtRunner({ thtBin: "tht", harnessDir: "/h", configPath: "config/tht.yaml" });
r.run = async (args) => { calls.push(args); return { code: 0, stdout: "", stderr: "" }; };
await r.setName("sid", "Mio nome");
await r.setName("sid", "Mio nome", "tenant-a");
expect(calls[0]).toEqual(["session", "set-name", "sid", "--name", "Mio nome"]);
});
test("setGroup / archive / unarchive / deleteSession build argv", async () => {
const calls: string[][] = [];
test("mutation and document commands retain their requested workspace", async () => {
const calls: Array<{ args: string[]; workspace?: string }> = [];
const r = new ThtRunner({ thtBin: "tht", harnessDir: "/h", configPath: "config/tht.yaml" });
r.run = async (args) => { calls.push(args); return { code: 0, stdout: "", stderr: "" }; };
await r.setGroup("sid", "G1");
await r.archive("sid");
await r.unarchive("sid");
await r.deleteSession("sid");
r.run = async (args, workspace) => { calls.push({ args, workspace }); return { code: 0, stdout: "[]", stderr: "" }; };
await r.setGroup("sid", "G1", "tenant-a");
await r.archive("sid", "tenant-a");
await r.unarchive("sid", "tenant-a");
await r.documents("sid", "tenant-a");
await r.deleteSession("sid", "tenant-a");
expect(calls).toEqual([
["session", "set-group", "sid", "--group", "G1"],
["session", "archive", "sid"],
["session", "unarchive", "sid"],
["session", "delete", "sid"],
{ args: ["session", "set-group", "sid", "--group", "G1"], workspace: "tenant-a" },
{ args: ["session", "archive", "sid"], workspace: "tenant-a" },
{ args: ["session", "unarchive", "sid"], workspace: "tenant-a" },
{ args: ["session", "documents", "sid", "--json"], workspace: "tenant-a" },
{ args: ["session", "delete", "sid"], workspace: "tenant-a" },
]);
});