mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
fix(server): validate instance env settings access
This commit is contained in:
1 parent
7a60924aa5
commit
ee7a3a58a5
4 files changed
+70
-7
No files matched your search
@@ -134,6 +134,24 @@ describe("environment instance routes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("allows non-admin board members with company access to read the shared environment catalog", async () => {
|
||||
mockEnvironmentService.list.mockResolvedValue([createEnvironment()]);
|
||||
const app = createApp({
|
||||
type: "board",
|
||||
userId: "user-1",
|
||||
source: "session",
|
||||
companyIds: ["company-1"],
|
||||
memberships: [{ companyId: "company-1", membershipRole: "member", status: "active" }],
|
||||
isInstanceAdmin: false,
|
||||
});
|
||||
|
||||
const res = await request(app).get("/api/companies/company-1/environments");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toHaveLength(1);
|
||||
expect(mockEnvironmentService.list).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("rejects company agents from enumerating the shared environment catalog", async () => {
|
||||
const app = createApp({
|
||||
type: "agent",
|
||||
@@ -146,7 +164,7 @@ describe("environment instance routes", () => {
|
||||
const res = await request(app).get("/api/companies/company-1/environments");
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toContain("board operators");
|
||||
expect(res.body.error).toContain("Board access required");
|
||||
});
|
||||
|
||||
it("rejects non-admin signed-in board members from mutating instance environments", async () => {
|
||||
|
||||
@@ -15,6 +15,9 @@ const mockHeartbeatService = vi.hoisted(() => ({
|
||||
buildIssueGraphLivenessAutoRecoveryPreview: vi.fn(),
|
||||
reconcileIssueGraphLiveness: vi.fn(),
|
||||
}));
|
||||
const mockEnvironmentService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
}));
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn());
|
||||
|
||||
function registerModuleMocks() {
|
||||
@@ -23,6 +26,9 @@ function registerModuleMocks() {
|
||||
instanceSettingsService: () => mockInstanceSettingsService,
|
||||
logActivity: mockLogActivity,
|
||||
}));
|
||||
vi.doMock("../services/environments.js", () => ({
|
||||
environmentService: () => mockEnvironmentService,
|
||||
}));
|
||||
}
|
||||
|
||||
async function createApp(actor: any) {
|
||||
@@ -59,6 +65,7 @@ describe("instance settings routes", () => {
|
||||
mockInstanceSettingsService.listCompanyIds.mockReset();
|
||||
mockHeartbeatService.buildIssueGraphLivenessAutoRecoveryPreview.mockReset();
|
||||
mockHeartbeatService.reconcileIssueGraphLiveness.mockReset();
|
||||
mockEnvironmentService.getById.mockReset();
|
||||
mockLogActivity.mockReset();
|
||||
mockInstanceSettingsService.get.mockResolvedValue({
|
||||
id: "instance-settings-1",
|
||||
@@ -162,6 +169,12 @@ describe("instance settings routes", () => {
|
||||
skippedOutsideLookback: 0,
|
||||
escalationIssueIds: ["issue-2"],
|
||||
});
|
||||
mockEnvironmentService.getById.mockResolvedValue({
|
||||
id: "env-1",
|
||||
driver: "local",
|
||||
status: "active",
|
||||
config: {},
|
||||
});
|
||||
});
|
||||
|
||||
it("allows local board users to read and update experimental settings", async () => {
|
||||
@@ -220,6 +233,24 @@ describe("instance settings routes", () => {
|
||||
expect(mockLogActivity).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("rejects unknown defaultEnvironmentId values with 422", async () => {
|
||||
mockEnvironmentService.getById.mockResolvedValue(null);
|
||||
const app = await createApp({
|
||||
type: "board",
|
||||
userId: "local-board",
|
||||
source: "local_implicit",
|
||||
isInstanceAdmin: true,
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.patch("/api/instance/settings")
|
||||
.send({ defaultEnvironmentId: "11111111-1111-4111-8111-111111111111" });
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.body.error).toContain("Environment not found");
|
||||
expect(mockInstanceSettingsService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows local board users to update guarded dev-server auto-restart", async () => {
|
||||
const app = await createApp({
|
||||
type: "board",
|
||||
|
||||
@@ -26,7 +26,7 @@ import { probeEnvironment } from "../services/environment-probe.js";
|
||||
import { secretService } from "../services/secrets.js";
|
||||
import { listReadyPluginEnvironmentDrivers } from "../services/plugin-environment-driver.js";
|
||||
import { getConfiguredSecretProvider } from "../secrets/configured-provider.js";
|
||||
import { getActorInfo } from "./authz.js";
|
||||
import { assertBoardOrgAccess, getActorInfo } from "./authz.js";
|
||||
import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
|
||||
import { environmentService } from "../services/environments.js";
|
||||
import { executionWorkspaceService } from "../services/execution-workspaces.js";
|
||||
@@ -58,6 +58,10 @@ export function environmentRoutes(
|
||||
throw forbidden("Instance admin access required");
|
||||
}
|
||||
|
||||
function assertCanReadInstanceEnvironments(req: Request) {
|
||||
assertBoardOrgAccess(req);
|
||||
}
|
||||
|
||||
async function assertCanReadSecretsForDraftProbe(req: Request, companyId: string) {
|
||||
assertCanAccessInstanceEnvironments(req);
|
||||
return companyId;
|
||||
@@ -155,7 +159,7 @@ export function environmentRoutes(
|
||||
}
|
||||
|
||||
router.get("/companies/:companyId/environments", async (req, res) => {
|
||||
assertCanAccessInstanceEnvironments(req);
|
||||
assertCanReadInstanceEnvironments(req);
|
||||
const rows = await svc.list({
|
||||
status: req.query.status as string | undefined,
|
||||
driver: req.query.driver as string | undefined,
|
||||
@@ -164,7 +168,7 @@ export function environmentRoutes(
|
||||
});
|
||||
|
||||
router.get("/companies/:companyId/environments/capabilities", async (req, res) => {
|
||||
assertCanAccessInstanceEnvironments(req);
|
||||
assertCanReadInstanceEnvironments(req);
|
||||
const pluginDrivers = await listReadyPluginEnvironmentDrivers({
|
||||
db,
|
||||
workerManager: options.pluginWorkerManager,
|
||||
@@ -253,7 +257,7 @@ export function environmentRoutes(
|
||||
res.status(404).json({ error: "Environment not found" });
|
||||
return;
|
||||
}
|
||||
assertCanAccessInstanceEnvironments(req);
|
||||
assertCanReadInstanceEnvironments(req);
|
||||
res.json(environment);
|
||||
});
|
||||
|
||||
@@ -263,7 +267,7 @@ export function environmentRoutes(
|
||||
res.status(404).json({ error: "Environment not found" });
|
||||
return;
|
||||
}
|
||||
assertCanAccessInstanceEnvironments(req);
|
||||
assertCanReadInstanceEnvironments(req);
|
||||
const leases = await svc.listLeases(environment.id, {
|
||||
status: req.query.status as string | undefined,
|
||||
});
|
||||
@@ -276,7 +280,7 @@ export function environmentRoutes(
|
||||
res.status(404).json({ error: "Environment lease not found" });
|
||||
return;
|
||||
}
|
||||
assertCanAccessInstanceEnvironments(req);
|
||||
assertCanReadInstanceEnvironments(req);
|
||||
res.json(lease);
|
||||
});
|
||||
|
||||
|
||||
@@ -9,6 +9,8 @@ import {
|
||||
import { forbidden } from "../errors.js";
|
||||
import { validate } from "../middleware/validate.js";
|
||||
import { heartbeatService, instanceSettingsService, logActivity } from "../services/index.js";
|
||||
import { environmentService } from "../services/environments.js";
|
||||
import { assertEnvironmentSelectionForCompany } from "./environment-selection.js";
|
||||
import { assertBoardOrgAccess, getActorInfo } from "./authz.js";
|
||||
|
||||
function assertCanManageInstanceSettings(req: Request) {
|
||||
@@ -24,6 +26,7 @@ function assertCanManageInstanceSettings(req: Request) {
|
||||
export function instanceSettingsRoutes(db: Db) {
|
||||
const router = Router();
|
||||
const svc = instanceSettingsService(db);
|
||||
const environments = environmentService(db);
|
||||
const heartbeat = heartbeatService(db);
|
||||
|
||||
router.get("/instance/settings", async (req, res) => {
|
||||
@@ -36,6 +39,13 @@ export function instanceSettingsRoutes(db: Db) {
|
||||
validate(patchInstanceSettingsSchema),
|
||||
async (req, res) => {
|
||||
assertCanManageInstanceSettings(req);
|
||||
if (Object.prototype.hasOwnProperty.call(req.body, "defaultEnvironmentId")) {
|
||||
await assertEnvironmentSelectionForCompany(
|
||||
environments,
|
||||
"instance",
|
||||
typeof req.body.defaultEnvironmentId === "string" ? req.body.defaultEnvironmentId : null,
|
||||
);
|
||||
}
|
||||
const updated = await svc.update(req.body);
|
||||
const actor = getActorInfo(req);
|
||||
const companyIds = await svc.listCompanyIds();
|
||||
|
||||
Reference in new issue
Block a user