mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-17 19:20:39 +09:00
[codex] harden authenticated routes and issue editor reliability (#3741)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - The control plane depends on authenticated routes enforcing company boundaries and role permissions correctly > - This branch also touches the issue detail and markdown editing flows operators use while handling advisory and triage work > - Partial issue cache seeds and fragile rich-editor parsing could leave important issue content missing or blank at the moment an operator needed it > - Blocked issues becoming actionable again should wake their assignee automatically instead of silently staying idle > - This pull request rebases the advisory follow-up branch onto current `master`, hardens authenticated route authorization, and carries the issue-detail/editor reliability fixes forward with regression tests > - The benefit is tighter authz on sensitive routes plus more reliable issue/advisory editing and wakeup behavior on top of the latest base ## What Changed - Hardened authenticated route authorization across agent, activity, approval, access, project, plugin, health, execution-workspace, portability, and related server paths, with new cross-tenant and runtime-authz regression coverage. - Switched issue detail queries from `initialData` to placeholder-based hydration so list/quicklook seeds still refetch full issue bodies. - Normalized advisory-style HTML images before mounting the markdown editor and strengthened fallback behavior when the rich editor silently fails or rejects the content. - Woke assigned agents when blocked issues move back to `todo`, with route coverage for reopen and unblock transitions. - Rebasing note: this branch now sits cleanly on top of the latest `master` tip used for the PR base. ## Verification - `pnpm exec vitest run ui/src/lib/issueDetailQuery.test.tsx ui/src/components/MarkdownEditor.test.tsx server/src/__tests__/issue-comment-reopen-routes.test.ts server/src/__tests__/activity-routes.test.ts server/src/__tests__/agent-cross-tenant-authz-routes.test.ts` - Confirmed `pnpm-lock.yaml` is not part of the PR diff. - Rebased the branch onto current `public-gh/master` before publishing. ## Risks - Broad authz tightening may expose existing flows that were relying on permissive board or agent access and now need explicit grants. - Markdown editor fallback changes could affect focus or rendering in edge-case content that mixes HTML-like advisory markup with normal markdown. - This verification was intentionally scoped to touched regressions and did not run the full repository suite. ## Model Used - OpenAI Codex, GPT-5-based coding agent in the Codex CLI environment with tool use for terminal, git, and GitHub operations. The exact runtime model identifier is not exposed inside this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, it is behavior-only and does not need before/after screenshots - [x] I have updated relevant documentation to reflect my changes, or no documentation changes were needed for these internal fixes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
50cd76d8a3
commit
32a9165ddf
39 changed files with 3014 additions and 153 deletions
|
|
@ -59,6 +59,11 @@ const assetSvc = {
|
|||
create: vi.fn(),
|
||||
};
|
||||
|
||||
const secretSvc = {
|
||||
normalizeAdapterConfigForPersistence: vi.fn(async (_companyId: string, config: Record<string, unknown>) => config),
|
||||
resolveAdapterConfigForRuntime: vi.fn(async (_companyId: string, config: Record<string, unknown>) => ({ config, secretKeys: new Set<string>() })),
|
||||
};
|
||||
|
||||
const agentInstructionsSvc = {
|
||||
exportFiles: vi.fn(),
|
||||
materializeManagedBundle: vi.fn(),
|
||||
|
|
@ -96,6 +101,10 @@ vi.mock("../services/assets.js", () => ({
|
|||
assetService: () => assetSvc,
|
||||
}));
|
||||
|
||||
vi.mock("../services/secrets.js", () => ({
|
||||
secretService: () => secretSvc,
|
||||
}));
|
||||
|
||||
vi.mock("../services/agent-instructions.js", () => ({
|
||||
agentInstructionsService: () => agentInstructionsSvc,
|
||||
}));
|
||||
|
|
@ -117,6 +126,11 @@ describe("company portability", () => {
|
|||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
secretSvc.normalizeAdapterConfigForPersistence.mockImplementation(async (_companyId, config) => config);
|
||||
secretSvc.resolveAdapterConfigForRuntime.mockImplementation(async (_companyId, config) => ({
|
||||
config,
|
||||
secretKeys: new Set<string>(),
|
||||
}));
|
||||
companySvc.getById.mockResolvedValue({
|
||||
id: "company-1",
|
||||
name: "Paperclip",
|
||||
|
|
@ -127,6 +141,11 @@ describe("company portability", () => {
|
|||
logoUrl: null,
|
||||
requireBoardApprovalForNewAgents: true,
|
||||
});
|
||||
companySvc.create.mockResolvedValue({
|
||||
id: "company-imported",
|
||||
name: "Imported Paperclip",
|
||||
requireBoardApprovalForNewAgents: true,
|
||||
});
|
||||
agentSvc.list.mockResolvedValue([
|
||||
{
|
||||
id: "agent-1",
|
||||
|
|
@ -1509,7 +1528,7 @@ describe("company portability", () => {
|
|||
}),
|
||||
]);
|
||||
|
||||
await portability.importBundle({
|
||||
const result = await portability.importBundle({
|
||||
source: { type: "inline", rootPath: "paperclip-demo", files },
|
||||
include: { company: true, agents: true, projects: true, issues: true, skills: false },
|
||||
target: { mode: "new_company", newCompanyName: "Imported Paperclip" },
|
||||
|
|
@ -1520,12 +1539,15 @@ describe("company portability", () => {
|
|||
expect(routineSvc.create).toHaveBeenCalledWith("company-imported", expect.objectContaining({
|
||||
projectId: "project-created",
|
||||
title: "Monday Review",
|
||||
assigneeAgentId: "agent-created",
|
||||
assigneeAgentId: null,
|
||||
priority: "high",
|
||||
status: "paused",
|
||||
concurrencyPolicy: "always_enqueue",
|
||||
catchUpPolicy: "enqueue_missed_with_cap",
|
||||
}), expect.any(Object));
|
||||
expect(result.warnings).toContain(
|
||||
"Task monday-review assignee claudecoder is pending_approval; imported work was left unassigned.",
|
||||
);
|
||||
expect(routineSvc.createTrigger).toHaveBeenCalledTimes(2);
|
||||
expect(routineSvc.createTrigger).toHaveBeenCalledWith("routine-created", expect.objectContaining({
|
||||
kind: "schedule",
|
||||
|
|
@ -2418,4 +2440,178 @@ describe("company portability", () => {
|
|||
expect(nestedMaterializedFiles?.["AGENTS.md"]).not.toMatch(/^---\n/);
|
||||
expect(nestedMaterializedFiles?.["AGENTS.md"]).not.toContain('name: "ClaudeCoder"');
|
||||
});
|
||||
|
||||
it("rejects dangerous adapter types on agent-safe imports", async () => {
|
||||
const portability = companyPortabilityService({} as any);
|
||||
const exported = await portability.exportBundle("company-1", {
|
||||
include: {
|
||||
company: true,
|
||||
agents: true,
|
||||
projects: false,
|
||||
issues: false,
|
||||
},
|
||||
});
|
||||
|
||||
agentSvc.list.mockResolvedValue([]);
|
||||
|
||||
await expect(portability.importBundle({
|
||||
source: {
|
||||
type: "inline",
|
||||
rootPath: exported.rootPath,
|
||||
files: exported.files,
|
||||
},
|
||||
include: {
|
||||
company: false,
|
||||
agents: true,
|
||||
projects: false,
|
||||
issues: false,
|
||||
},
|
||||
target: {
|
||||
mode: "existing_company",
|
||||
companyId: "company-1",
|
||||
},
|
||||
agents: ["claudecoder"],
|
||||
collisionStrategy: "rename",
|
||||
adapterOverrides: {
|
||||
claudecoder: {
|
||||
adapterType: "process",
|
||||
adapterConfig: {
|
||||
command: "/bin/sh",
|
||||
args: ["-c", "id"],
|
||||
},
|
||||
},
|
||||
},
|
||||
}, "user-1", {
|
||||
mode: "agent_safe",
|
||||
sourceCompanyId: "company-1",
|
||||
})).rejects.toThrow('Adapter type "process" is not allowed in safe imports');
|
||||
|
||||
expect(agentSvc.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("imports new agents through approval and adapter-config normalization", async () => {
|
||||
const portability = companyPortabilityService({} as any);
|
||||
const exported = await portability.exportBundle("company-1", {
|
||||
include: {
|
||||
company: true,
|
||||
agents: true,
|
||||
projects: false,
|
||||
issues: false,
|
||||
},
|
||||
});
|
||||
|
||||
agentSvc.list.mockResolvedValue([]);
|
||||
secretSvc.normalizeAdapterConfigForPersistence.mockResolvedValueOnce({
|
||||
normalized: true,
|
||||
env: {
|
||||
OPENAI_API_KEY: {
|
||||
type: "secret_ref",
|
||||
secretId: "secret-1",
|
||||
version: "latest",
|
||||
},
|
||||
},
|
||||
});
|
||||
agentSvc.create.mockImplementation(async (_companyId: string, input: Record<string, unknown>) => ({
|
||||
id: "agent-created",
|
||||
name: String(input.name),
|
||||
adapterType: input.adapterType,
|
||||
adapterConfig: input.adapterConfig,
|
||||
status: input.status,
|
||||
}));
|
||||
|
||||
await portability.importBundle({
|
||||
source: {
|
||||
type: "inline",
|
||||
rootPath: exported.rootPath,
|
||||
files: exported.files,
|
||||
},
|
||||
include: {
|
||||
company: true,
|
||||
agents: true,
|
||||
projects: false,
|
||||
issues: false,
|
||||
},
|
||||
target: {
|
||||
mode: "new_company",
|
||||
newCompanyName: "Imported Paperclip",
|
||||
},
|
||||
agents: ["claudecoder"],
|
||||
collisionStrategy: "rename",
|
||||
}, "user-1");
|
||||
|
||||
expect(secretSvc.normalizeAdapterConfigForPersistence).toHaveBeenCalledWith(
|
||||
"company-imported",
|
||||
expect.any(Object),
|
||||
{ strictMode: false },
|
||||
);
|
||||
expect(agentSvc.create).toHaveBeenCalledWith("company-imported", expect.objectContaining({
|
||||
adapterType: "claude_local",
|
||||
adapterConfig: expect.objectContaining({
|
||||
normalized: true,
|
||||
}),
|
||||
status: "pending_approval",
|
||||
}));
|
||||
});
|
||||
|
||||
it("normalizes adapter config on replace imports before updating existing agents", async () => {
|
||||
const portability = companyPortabilityService({} as any);
|
||||
const exported = await portability.exportBundle("company-1", {
|
||||
include: {
|
||||
company: true,
|
||||
agents: true,
|
||||
projects: false,
|
||||
issues: false,
|
||||
},
|
||||
});
|
||||
|
||||
secretSvc.normalizeAdapterConfigForPersistence.mockResolvedValueOnce({
|
||||
normalized: "updated",
|
||||
});
|
||||
agentSvc.update.mockImplementation(async (id: string, patch: Record<string, unknown>) => ({
|
||||
id,
|
||||
name: "ClaudeCoder",
|
||||
adapterType: patch.adapterType,
|
||||
adapterConfig: patch.adapterConfig,
|
||||
}));
|
||||
|
||||
await portability.importBundle({
|
||||
source: {
|
||||
type: "inline",
|
||||
rootPath: exported.rootPath,
|
||||
files: exported.files,
|
||||
},
|
||||
include: {
|
||||
company: false,
|
||||
agents: true,
|
||||
projects: false,
|
||||
issues: false,
|
||||
},
|
||||
target: {
|
||||
mode: "existing_company",
|
||||
companyId: "company-1",
|
||||
},
|
||||
agents: ["claudecoder"],
|
||||
collisionStrategy: "replace",
|
||||
adapterOverrides: {
|
||||
claudecoder: {
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {
|
||||
model: "gpt-5.4",
|
||||
},
|
||||
},
|
||||
},
|
||||
}, "user-1");
|
||||
|
||||
expect(secretSvc.normalizeAdapterConfigForPersistence).toHaveBeenCalledWith(
|
||||
"company-1",
|
||||
expect.any(Object),
|
||||
{ strictMode: false },
|
||||
);
|
||||
expect(agentSvc.update).toHaveBeenCalledWith("agent-1", expect.objectContaining({
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {
|
||||
normalized: "updated",
|
||||
},
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue