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

refactor: enforce P1.1 registry path ownership

Browse Source
This commit is contained in:
marcopan
2026-08-11 14:29:00 +02:00
parent 7356d6794b
commit 86af45acb4
6 changed files with 101 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/workspaces/git-repository.ts
+50 -17
View File
@@ -130,34 +130,52 @@ export class GitWorkspaceRepository {
} }
async workspacePaths(): Promise<string[]> { async workspacePaths(): Promise<string[]> {
const output = await this.git(["ls-tree", "-r", "--name-only", "HEAD", "--", "workspaces"]); const output = await this.git(["ls-tree", "-d", "--name-only", "HEAD"]);
const paths = output.trim() === "" ? [] : output.trim().split("\n"); const directories = output.trim() === "" ? [] : output.trim().split("\n");
for (const path of paths) { const paths: string[] = [];
if (!/^workspaces\/[a-z][a-z0-9-]{2,62}\.yaml$/.test(path)) { for (const id of directories) {
if (id === "workspace-docs") continue;
if (!/^[a-z][a-z0-9-]{2,62}$/.test(id)) {
throw new WorkspaceRegistryError("workspace_invalid", "Workspace repository contains an invalid path"); throw new WorkspaceRegistryError("workspace_invalid", "Workspace repository contains an invalid path");
} }
const path = `${id}/workspace.yaml`;
const type = (await this.git(["cat-file", "-t", `HEAD:${path}`], {},
"Workspace descriptor is invalid")).trim();
if (type !== "blob") {
throw new WorkspaceRegistryError("workspace_invalid", "Workspace descriptor is invalid");
}
paths.push(path);
} }
return paths; return paths.sort();
} }
async readWorkspace(path: string): Promise<string> { async readCatalog(revision = "HEAD"): Promise<string> {
if (!/^workspaces\/[a-z][a-z0-9-]{2,62}\.yaml$/.test(path)) { return await this.git(["show", `${revision}:thoth-workspaces.yaml`], {}, "Workspace catalog is invalid");
throw new WorkspaceRegistryError("workspace_invalid", "Workspace repository path is invalid");
}
return await this.git(["show", `HEAD:${path}`]);
} }
async blob(path: string): Promise<string> { async catalogBlob(revision = "HEAD"): Promise<string> {
if (!/^workspaces\/[a-z][a-z0-9-]{2,62}\.yaml$/.test(path)) { return (await this.git(["rev-parse", `${revision}:thoth-workspaces.yaml`], {},
"Workspace catalog is invalid")).trim();
}
async readWorkspace(path: string, revision = "HEAD"): Promise<string> {
if (!/^(?!workspace-docs\/)[a-z][a-z0-9-]{2,62}\/workspace\.yaml$/.test(path)) {
throw new WorkspaceRegistryError("workspace_invalid", "Workspace repository path is invalid"); throw new WorkspaceRegistryError("workspace_invalid", "Workspace repository path is invalid");
} }
return (await this.git(["rev-parse", `HEAD:${path}`])).trim(); return await this.git(["show", `${revision}:${path}`]);
}
async blob(path: string, revision = "HEAD"): Promise<string> {
if (!/^(?!workspace-docs\/)[a-z][a-z0-9-]{2,62}\/workspace\.yaml$/.test(path)) {
throw new WorkspaceRegistryError("workspace_invalid", "Workspace repository path is invalid");
}
return (await this.git(["rev-parse", `${revision}:${path}`])).trim();
} }
/** Assert that a canonical Evidence root is a Git tree at an exact commit. */ /** Assert that a canonical Evidence root is a Git tree at an exact commit. */
async assertTreeAtRevision(revision: string, repoRelativePath: string): Promise<void> { async assertTreeAtRevision(revision: string, repoRelativePath: string): Promise<void> {
if (!/^[0-9a-f]{40}$/.test(revision) if (!/^[0-9a-f]{40}$/.test(revision)
|| !/^workspace-content\/[a-z][a-z0-9-]{2,62}\/evidence$/.test(repoRelativePath)) { || !/^[a-z][a-z0-9-]{2,62}\/evidence$/.test(repoRelativePath)) {
throw new WorkspaceRegistryError("workspace_invalid", "Workspace Evidence revision is invalid"); throw new WorkspaceRegistryError("workspace_invalid", "Workspace Evidence revision is invalid");
} }
const type = (await this.git( const type = (await this.git(
@@ -170,7 +188,7 @@ export class GitWorkspaceRepository {
} }
} }
/** Write only a validated registry artifact below the checked-out repository. */ /** Write only a validated API-owned artifact below the checked-out repository. */
async writeRegistryFile(path: string, source: string): Promise<void> { async writeRegistryFile(path: string, source: string): Promise<void> {
this.assertRegistryArtifactPath(path); this.assertRegistryArtifactPath(path);
const target = join(this.repoPath, path); const target = join(this.repoPath, path);
@@ -178,6 +196,21 @@ export class GitWorkspaceRepository {
await writeFile(target, source, { encoding: "utf8", mode: 0o600 }); await writeFile(target, source, { encoding: "utf8", mode: 0o600 });
} }
/** Create a descriptor only when no filesystem entry exists at its exact path. */
async createRegistryFile(path: string, source: string): Promise<void> {
this.assertRegistryArtifactPath(path);
if (!/^(?!workspace-docs\/)[a-z][a-z0-9-]{2,62}\/workspace\.yaml$/.test(path)) {
throw new WorkspaceRegistryError("workspace_invalid", "Workspace descriptor path is invalid");
}
const target = join(this.repoPath, path);
await mkdir(dirname(target), { recursive: true, mode: 0o700 });
try {
await writeFile(target, source, { encoding: "utf8", mode: 0o600, flag: "wx" });
} catch {
throw new WorkspaceRegistryError("workspace_curator_owned", "Workspace descriptor is curator-owned");
}
}
async removeRegistryFile(path: string): Promise<void> { async removeRegistryFile(path: string): Promise<void> {
this.assertRegistryArtifactPath(path); this.assertRegistryArtifactPath(path);
await rm(join(this.repoPath, path), { force: true }); await rm(join(this.repoPath, path), { force: true });
@@ -214,7 +247,7 @@ export class GitWorkspaceRepository {
} }
private isRegistryArtifactPath(path: string): boolean { private isRegistryArtifactPath(path: string): boolean {
return /^workspaces\/[a-z][a-z0-9-]{2,62}\.yaml$/.test(path) return /^(?!workspace-docs\/)[a-z][a-z0-9-]{2,62}\/workspace\.yaml$/.test(path)
|| /^workspace-docs\/[a-z][a-z0-9-]{2,62}\/(?:contract\.env\.example|README\.md)$/.test(path); || /^workspace-docs\/[a-z][a-z0-9-]{2,62}\/(?:contract\.env\.example|README\.md)$/.test(path);
} }
@@ -258,7 +291,7 @@ export class GitWorkspaceRepository {
private async restoreFailedPublication(): Promise<void> { private async restoreFailedPublication(): Promise<void> {
try { try {
await this.git(["reset", "--hard", `refs/remotes/origin/${this.config.branch}`]); await this.git(["reset", "--hard", `refs/remotes/origin/${this.config.branch}`]);
await this.git(["clean", "-fd", "--", "workspaces", "workspace-docs"]); await this.git(["clean", "-fd", "--", "workspace-docs"]);
} catch { } catch {
// Keep the original sanitized publish failure. A future refresh will surface any recovery // Keep the original sanitized publish failure. A future refresh will surface any recovery
// problem without leaking the Git failure details through the API. // problem without leaking the Git failure details through the API.
backend/src/workspaces/schema.ts
+1 -1
View File
@@ -341,7 +341,7 @@ function workspaceInvariants(workspace: any, context: z.RefinementCtx): void {
unique(workspace.llm_policy.allowed, context, ["llm_policy", "allowed"]); unique(workspace.llm_policy.allowed, context, ["llm_policy", "allowed"]);
if (workspace.evidence?.source.type === "filesystem") { if (workspace.evidence?.source.type === "filesystem") {
const expected = `workspace-content/${workspace.workspace.id}/evidence`; const expected = `${workspace.workspace.id}/evidence`;
if (workspace.evidence.source.uri !== expected) { if (workspace.evidence.source.uri !== expected) {
context.addIssue({ context.addIssue({
code: "custom", code: "custom",
backend/src/workspaces/types.ts
+1 -1
View File
@@ -12,7 +12,7 @@ export interface WorkspaceRegistryConfig {
export type WorkspaceErrorCode = export type WorkspaceErrorCode =
| "workspace_invalid" | "binding_missing" | "workspace_not_activatable" | "workspace_invalid" | "binding_missing" | "workspace_not_activatable"
| "workspace_stale" | "workspace_conflict" | "git_unavailable" | "workspace_stale" | "workspace_conflict" | "workspace_curator_owned" | "git_unavailable"
| "git_auth_failed" | "git_non_fast_forward" | "git_push_rejected" | "git_auth_failed" | "git_non_fast_forward" | "git_push_rejected"
| "connector_unavailable" | "semantic_index_incompatible"; | "connector_unavailable" | "semantic_index_incompatible";
backend/test/workspaces-git-repository.test.ts
+47 -29
View File
@@ -1,5 +1,5 @@
import { execFile } from "node:child_process"; import { execFile } from "node:child_process";
import { existsSync, mkdtempSync, mkdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os"; import { tmpdir } from "node:os";
import { join } from "node:path"; import { join } from "node:path";
import { promisify } from "node:util"; import { promisify } from "node:util";
@@ -55,20 +55,22 @@ async function temporaryRemote(): Promise<{ root: string; remote: string; source
await git(source, ["init", "--initial-branch=main"]); await git(source, ["init", "--initial-branch=main"]);
await git(source, ["config", "user.name", "Workspace Registry Test"]); await git(source, ["config", "user.name", "Workspace Registry Test"]);
await git(source, ["config", "user.email", "workspace-registry@example.invalid"]); await git(source, ["config", "user.email", "workspace-registry@example.invalid"]);
mkdirSync(join(source, "workspaces")); writeFileSync(join(source, "thoth-workspaces.yaml"), "schema_version: 1\nworkspaces: [{id: psd-clinical, name: Policlinico San Donato}, {id: research, name: Policlinico San Donato}]\n");
writeFileSync(join(source, "workspaces", "psd-clinical.yaml"), validYaml); mkdirSync(join(source, "psd-clinical"), { recursive: true });
writeFileSync(join(source, "workspaces", "research.yaml"), validYaml mkdirSync(join(source, "research"), { recursive: true });
writeFileSync(join(source, "psd-clinical", "workspace.yaml"), validYaml);
writeFileSync(join(source, "research", "workspace.yaml"), validYaml
.replace("id: psd-clinical", "id: research")); .replace("id: psd-clinical", "id: research"));
mkdirSync(join(source, "workspace-content", "research", "evidence"), { recursive: true }); mkdirSync(join(source, "research", "evidence"), { recursive: true });
mkdirSync(join(source, "workspace-content", "other", "evidence"), { recursive: true }); mkdirSync(join(source, "other", "evidence"), { recursive: true });
mkdirSync(join(source, "workspace-content", "blob"), { recursive: true }); mkdirSync(join(source, "blob"), { recursive: true });
mkdirSync(join(source, "workspace-content", "link"), { recursive: true }); mkdirSync(join(source, "link"), { recursive: true });
writeFileSync(join(source, "workspace-content", "research", "evidence", "guide.md"), "guide v1\n"); writeFileSync(join(source, "research", "evidence", "guide.md"), "guide v1\n");
writeFileSync(join(source, "workspace-content", "other", "evidence", "other.md"), "other\n"); writeFileSync(join(source, "other", "evidence", "other.md"), "other\n");
writeFileSync(join(source, "workspace-content", "blob", "evidence"), "not a tree\n"); writeFileSync(join(source, "blob", "evidence"), "not a tree\n");
symlinkSync("../research/evidence", join(source, "workspace-content", "link", "evidence")); symlinkSync("../research/evidence", join(source, "link", "evidence"));
symlinkSync("guide.md", join(source, "workspace-content", "research", "evidence", "nested-link")); symlinkSync("guide.md", join(source, "research", "evidence", "nested-link"));
await git(source, ["add", "workspaces", "workspace-content"]); await git(source, ["add", "-A"]);
await git(source, ["commit", "-m", "Initial workspace"]); await git(source, ["commit", "-m", "Initial workspace"]);
await git(source, ["remote", "add", "origin", remote]); await git(source, ["remote", "add", "origin", remote]);
await git(source, ["push", "origin", "main"]); await git(source, ["push", "origin", "main"]);
@@ -112,19 +114,19 @@ test("accepts only a tree at the declared Evidence root for the requested revisi
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
"workspace-content/research/evidence", "research/evidence",
)).resolves.toBeUndefined(); )).resolves.toBeUndefined();
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
"workspace-content/missing/evidence", "missing/evidence",
)).rejects.toMatchObject({ code: "workspace_invalid" }); )).rejects.toMatchObject({ code: "workspace_invalid" });
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
"workspace-content/blob/evidence", "blob/evidence",
)).rejects.toMatchObject({ code: "workspace_invalid" }); )).rejects.toMatchObject({ code: "workspace_invalid" });
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
"workspace-content/link/evidence", "link/evidence",
)).rejects.toMatchObject({ code: "workspace_invalid" }); )).rejects.toMatchObject({ code: "workspace_invalid" });
}); });
@@ -136,7 +138,7 @@ test("redacts Git failures while checking an Evidence tree", async () => {
const error = await repository.assertTreeAtRevision( const error = await repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
"workspace-content/research/evidence", "research/evidence",
).catch((failure: unknown) => failure); ).catch((failure: unknown) => failure);
expect(error).toMatchObject({ code: "git_unavailable", message: "Workspace Git operation failed" }); expect(error).toMatchObject({ code: "git_unavailable", message: "Workspace Git operation failed" });
expect((error as Error).message).not.toContain(fixture.root); expect((error as Error).message).not.toContain(fixture.root);
@@ -150,7 +152,7 @@ test("classifies repository corruption as unavailable rather than invalid Eviden
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
"workspace-content/research/evidence", "research/evidence",
)).rejects.toMatchObject({ code: "git_unavailable", message: "Workspace Git operation failed" }); )).rejects.toMatchObject({ code: "git_unavailable", message: "Workspace Git operation failed" });
}); });
@@ -158,28 +160,28 @@ test("binds Evidence tree validation to old and new content-only commits", async
const fixture = await temporaryRemote(); const fixture = await temporaryRemote();
const repository = new GitWorkspaceRepository(config(join(fixture.root, "registry"), fixture.remote)); const repository = new GitWorkspaceRepository(config(join(fixture.root, "registry"), fixture.remote));
await repository.bootstrap(); await repository.bootstrap();
writeFileSync(join(fixture.source, "workspace-content", "research", "evidence", "guide.md"), "guide v2\n"); writeFileSync(join(fixture.source, "research", "evidence", "guide.md"), "guide v2\n");
await git(fixture.source, ["add", "workspace-content/research/evidence/guide.md"]); await git(fixture.source, ["add", "research/evidence/guide.md"]);
await git(fixture.source, ["commit", "-m", "Update Evidence content"]); await git(fixture.source, ["commit", "-m", "Update Evidence content"]);
await git(fixture.source, ["push", "origin", "main"]); await git(fixture.source, ["push", "origin", "main"]);
const { stdout } = await runFile("git", ["rev-parse", "HEAD"], { cwd: fixture.source }); const { stdout } = await runFile("git", ["rev-parse", "HEAD"], { cwd: fixture.source });
const newCommit = stdout.trim(); const newCommit = stdout.trim();
await repository.pull(); await repository.pull();
const oldTree = (await runFile("git", ["rev-parse", `${fixture.initialCommit}:workspace-content/research/evidence`], { const oldTree = (await runFile("git", ["rev-parse", `${fixture.initialCommit}:research/evidence`], {
cwd: fixture.source, cwd: fixture.source,
})).stdout.trim(); })).stdout.trim();
const newTree = (await runFile("git", ["rev-parse", `${newCommit}:workspace-content/research/evidence`], { const newTree = (await runFile("git", ["rev-parse", `${newCommit}:research/evidence`], {
cwd: fixture.source, cwd: fixture.source,
})).stdout.trim(); })).stdout.trim();
expect(newTree).not.toBe(oldTree); expect(newTree).not.toBe(oldTree);
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
"workspace-content/research/evidence", "research/evidence",
)).resolves.toBeUndefined(); )).resolves.toBeUndefined();
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
newCommit, newCommit,
"workspace-content/research/evidence", "research/evidence",
)).resolves.toBeUndefined(); )).resolves.toBeUndefined();
}); });
@@ -191,7 +193,7 @@ test("defers nested Evidence symlink containment to P6", async () => {
// Task 2 validates only the declared root object. Recursive containment remains a P6 boundary. // Task 2 validates only the declared root object. Recursive containment remains a P6 boundary.
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
"workspace-content/research/evidence", "research/evidence",
)).resolves.toBeUndefined(); )).resolves.toBeUndefined();
}); });
@@ -203,11 +205,11 @@ test("rejects malformed revisions and shell-like paths without executing them",
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
"HEAD", "HEAD",
"workspace-content/research/evidence", "research/evidence",
)).rejects.toMatchObject({ code: "workspace_invalid" }); )).rejects.toMatchObject({ code: "workspace_invalid" });
await expect(repository.assertTreeAtRevision( await expect(repository.assertTreeAtRevision(
fixture.initialCommit, fixture.initialCommit,
`workspace-content/research/evidence;touch ${marker}`, `research/evidence;touch ${marker}`,
)).rejects.toMatchObject({ code: "workspace_invalid" }); )).rejects.toMatchObject({ code: "workspace_invalid" });
expect(existsSync(marker)).toBe(false); expect(existsSync(marker)).toBe(false);
}); });
@@ -298,3 +300,19 @@ test("parallel contenders recover a stale lock file without overlapping critical
expect(results.filter((result) => result.status === "rejected")).toHaveLength(1); expect(results.filter((result) => result.status === "rejected")).toHaveLength(1);
expect(maximum).toBe(1); expect(maximum).toBe(1);
}); });
test("creates a descriptor only when the exact curator path is absent", async () => {
const fixture = await temporaryRemote();
const repository = new GitWorkspaceRepository(config(join(fixture.root, "registry"), fixture.remote));
await repository.bootstrap();
const descriptorPath = "new-workspace/workspace.yaml";
const descriptor = "curator descriptor\n";
await expect(repository.createRegistryFile(descriptorPath, descriptor)).resolves.toBeUndefined();
expect(readFileSync(join(fixture.root, "registry", "repo", descriptorPath), "utf8")).toBe(descriptor);
await expect(repository.createRegistryFile(descriptorPath, "overwrite\n"))
.rejects.toMatchObject({ code: "workspace_curator_owned" });
expect(readFileSync(join(fixture.root, "registry", "repo", descriptorPath), "utf8")).toBe(descriptor);
await expect(repository.createRegistryFile("workspace-docs/workspace.yaml", descriptor))
.rejects.toMatchObject({ code: "workspace_invalid" });
});
deploy/workspaces/example.yaml
+1 -1
View File
@@ -27,7 +27,7 @@ semantic_index:
evidence: evidence:
source: source:
type: filesystem type: filesystem
uri: workspace-content/example/evidence uri: example/evidence
patterns: patterns:
- "**/*.md" - "**/*.md"
max_bytes: 10485760 max_bytes: 10485760
deploy/workspaces/psd.yaml.example
+1 -1
View File
@@ -27,7 +27,7 @@ semantic_index:
evidence: evidence:
source: source:
type: filesystem type: filesystem
uri: workspace-content/example-workspace/evidence uri: example-workspace/evidence
patterns: patterns:
- "**/*.md" - "**/*.md"
max_bytes: 10485760 max_bytes: 10485760