Skip to content

fix(sandbox): restrict terminal and prod code execution to admins #1664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
2witstudios merged 1 commit into from
Jun 21, 2026
Merged

fix(sandbox): restrict terminal and prod code execution to admins #1664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions apps/web/src/app/api/pages/[pageId]/terminal/execute/__tests__/route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { NextResponse } from 'next/server';

vi.mock('@pagespace/lib/services/sandbox/can-run-code', () => ({
isCodeExecutionEnabled: vi.fn().mockReturnValue(true),
canRunCode: vi.fn().mockResolvedValue({ ok: true }),
}));

vi.mock('@/lib/auth', () => ({
Expand Down Expand Up @@ -80,7 +81,7 @@ vi.mock('@pagespace/lib/audit/audit-log', () => ({
// ─── Import SUT and mocked modules ────────────────────────────────────────

import { POST } from '../route';
import { isCodeExecutionEnabled } from '@pagespace/lib/services/sandbox/can-run-code';
import { canRunCode, isCodeExecutionEnabled } from '@pagespace/lib/services/sandbox/can-run-code';
import { authenticateRequestWithOptions, canPrincipalEditPage } from '@/lib/auth';
import { db } from '@pagespace/db/db';
import { checkCodeExecutionQuota, acquireCodeExecutionSlot, releaseCodeExecutionSlot } from '@pagespace/lib/services/sandbox/quota';
Expand All @@ -89,7 +90,6 @@ import { createSpritesSandboxClient } from '@pagespace/lib/services/sandbox/sand

// ─── Type helpers ──────────────────────────────────────────────────────────

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function asMock<T>(fn: T): ReturnType<typeof vi.fn> {
return fn as unknown as ReturnType<typeof vi.fn>;
}
Expand All @@ -105,7 +105,7 @@ const mockSessionAuth = {
userId: mockUserId,
tokenType: 'session' as const,
sessionId: 'sess_1',
role: 'user' as const,
role: 'admin' as const,
tokenVersion: 1,
adminRoleVersion: 1,
};
Expand Down Expand Up @@ -175,6 +175,7 @@ describe('POST /api/pages/[pageId]/terminal/execute', () => {
beforeEach(() => {
vi.clearAllMocks();
asMock(isCodeExecutionEnabled).mockReturnValue(true);
asMock(canRunCode).mockResolvedValue({ ok: true });
asMock(authenticateRequestWithOptions).mockResolvedValue(mockSessionAuth);
asMock(canPrincipalEditPage).mockResolvedValue(true);
asMock(checkCodeExecutionQuota).mockResolvedValue({ allowed: true });
Expand Down Expand Up @@ -203,11 +204,33 @@ describe('POST /api/pages/[pageId]/terminal/execute', () => {
});

describe('authorization', () => {
it('returns 403 when a non-admin tries to execute a terminal command', async () => {
asMock(authenticateRequestWithOptions).mockResolvedValue({
...mockSessionAuth,
role: 'user',
});

const res = await POST(makeRequest(), makeParams());

expect(res.status).toBe(403);
expect(vi.mocked(canPrincipalEditPage)).not.toHaveBeenCalled();
});

it('returns 403 when user cannot edit page', async () => {
asMock(canPrincipalEditPage).mockResolvedValue(false);
const res = await POST(makeRequest(), makeParams());
expect(res.status).toBe(403);
});

it('returns 403 when shared code-execution authorization denies the user', async () => {
setupDbMocks();
asMock(canRunCode).mockResolvedValue({ ok: false, reason: 'app_admin_required' });

const res = await POST(makeRequest(), makeParams());

expect(res.status).toBe(403);
expect(vi.mocked(acquireCodeExecutionSlot)).not.toHaveBeenCalled();
});
});

describe('request body validation', () => {
Expand Down
12 changes: 11 additions & 1 deletion apps/web/src/app/api/pages/[pageId]/terminal/execute/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { NextResponse } from 'next/server';
import { z } from 'zod/v4';
import { authenticateRequestWithOptions, isAuthError, canPrincipalEditPage } from '@/lib/auth';
import { isCodeExecutionEnabled } from '@pagespace/lib/services/sandbox/can-run-code';
import { canRunCode, isCodeExecutionEnabled } from '@pagespace/lib/services/sandbox/can-run-code';
import { getSandboxSessionSecret } from '@pagespace/lib/services/sandbox/session-manager';
import {
acquireTerminalSandbox,
Expand Down Expand Up @@ -54,6 +54,10 @@ export async function POST(
const auth = await authenticateRequestWithOptions(req, AUTH_OPTIONS);
if (isAuthError(auth)) return auth.error;
const userId = auth.userId;
if (auth.role !== 'admin') {
auditRequest(req, { eventType: 'authz.access.denied', userId, resourceType: 'terminal_session', resourceId: pageId, details: { reason: 'app_admin_required', method: 'POST' }, riskScore: 0.5 });
return NextResponse.json({ error: 'Terminal access requires administrator privileges' }, { status: 403 });
}
Comment on lines +57 to +60

@coderabbitai coderabbitai Bot Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace inline role check with centralized permission evaluation (Line 57).

auth.role !== 'admin' is a custom authorization path in an API route and should use the shared permission layer for consistency and policy correctness.

As per coding guidelines, apps/web/src/app/api/**/*.{ts,tsx} must “Use centralized permission logic from @pagespace/lib/permissions/permissions via getUserAccessLevel() and canUserEditPage() functions,” and **/*.{ts,tsx} must “Always use centralized permission functions from packages/lib/src/permissions/. Never roll your own access checks.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/app/api/pages/`[pageId]/terminal/execute/route.ts around lines
57 - 60, Replace the inline authorization check `auth.role !== 'admin'` with a
call to the centralized permission function from the shared permissions layer.
Instead of directly checking the auth role, use `getUserAccessLevel()` or
`canUserEditPage()` functions imported from
`@pagespace/lib/permissions/permissions` to determine if the user has the
required permissions to access the terminal session. This ensures consistency
with the centralized authorization policy and maintains a single source of truth
for permission evaluation across the application.

Source: Coding guidelines


// 3. Page permission: editor+ required to run code.
const canEdit = await canPrincipalEditPage(auth, pageId);
Expand Down Expand Up @@ -86,6 +90,12 @@ export async function POST(
}
const { driveId } = pageRow;

const codeAuth = await canRunCode({ userId, driveId, requestOrigin: 'user' });
if (!codeAuth.ok) {
auditRequest(req, { eventType: 'authz.access.denied', userId, resourceType: 'terminal_session', resourceId: pageId, details: { reason: codeAuth.reason, method: 'POST' }, riskScore: 0.5 });
return NextResponse.json({ error: 'Code execution is not available for this user' }, { status: 403 });
}

const [driveRow, actorRow] = await Promise.all([
db.select({ ownerId: drives.ownerId }).from(drives).where(eq(drives.id, driveId)).limit(1),
db.select({ subscriptionTier: users.subscriptionTier, email: users.email, name: users.name }).from(users).where(eq(users.id, userId)).limit(1),
Expand Down
40 changes: 39 additions & 1 deletion apps/web/src/app/api/pages/__tests__/route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ describe('POST /api/pages', () => {

expect(response.status).toBe(400);
expect(body.error).toContain('Invalid option');
expect(body.error).not.toContain('TERMINAL');
expect(pageService.createPage).not.toHaveBeenCalled();
});

Expand Down Expand Up @@ -310,6 +309,45 @@ describe('POST /api/pages', () => {
expect(response.status).toBe(400);
expect(body.error).toMatch(/not found/i);
});

it('returns 403 when a non-admin creates a TERMINAL page', async () => {
const response = await POST(createRequest({
title: 'Prod Shell',
type: 'TERMINAL',
driveId: mockDriveId,
}));
const body = await response.json();

expect(response.status).toBe(403);
expect(body.error).toMatch(/administrator/i);
expect(pageService.createPage).not.toHaveBeenCalled();
});

it('allows an admin to create a TERMINAL page', async () => {
vi.mocked(authenticateRequestWithOptions).mockResolvedValue({
...mockWebAuth(mockUserId),
role: 'admin',
});
vi.mocked(pageService.createPage).mockResolvedValue({
...successResult,
page: { ...mockPage, type: 'TERMINAL' },
});

const response = await POST(createRequest({
title: 'Prod Shell',
type: 'TERMINAL',
driveId: mockDriveId,
}));
const body = await response.json();

expect(response.status).toBe(201);
expect(body.type).toBe('TERMINAL');
expect(pageService.createPage).toHaveBeenCalledWith(
mockUserId,
expect.objectContaining({ type: 'TERMINAL' }),
expect.objectContaining({ authorizeEdit: expect.any(Function) }),
);
});
});

describe('service delegation', () => {
Expand Down
11 changes: 10 additions & 1 deletion apps/web/src/app/api/pages/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,22 @@ import { z } from 'zod/v4';
import { broadcastPageEvent, createPageEventPayload } from '@/lib/websocket';
import { loggers } from '@pagespace/lib/logging/logger-config'
import { getCreatablePageTypes } from '@pagespace/lib/content/page-types.config'
import { PageType } from '@pagespace/lib/utils/enums'
import { auditRequest } from '@pagespace/lib/audit/audit-log';
import { trackPageOperation } from '@pagespace/lib/monitoring/activity-tracker';
import { authenticateRequestWithOptions, isAuthError, checkMCPCreateScope, isMCPAuthResult, canPrincipalEditPage } from '@/lib/auth';
import { pageService, type CreatePageParams } from '@/services/api';

const AUTH_OPTIONS = { allow: ['session', 'mcp'] as const, requireCSRF: true };
const creatablePageTypes = [
...getCreatablePageTypes(),
PageType.TERMINAL,
] as unknown as [string, ...string[]];

// Zod schema for page creation request
const createPageSchema = z.object({
title: z.string().min(1, 'Title is required'),
type: z.enum(getCreatablePageTypes() as [string, ...string[]]),
type: z.enum(creatablePageTypes),
driveId: z.string().min(1, 'Drive ID is required'),
parentId: z.string().nullable().optional(),
content: z.string().optional(),
Expand Down Expand Up @@ -45,6 +50,10 @@ export async function POST(request: Request) {
}

const validatedData = parseResult.data;
if (validatedData.type === PageType.TERMINAL && auth.role !== 'admin') {
auditRequest(request, { eventType: 'authz.access.denied', userId, resourceType: 'page', resourceId: validatedData.driveId, details: { reason: 'app_admin_required', type: validatedData.type, method: 'POST' }, riskScore: 0.5 });
return NextResponse.json({ error: 'Terminal pages require administrator privileges' }, { status: 403 });
}
Comment on lines +53 to +56

@coderabbitai coderabbitai Bot Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use centralized permission helpers for the TERMINAL create gate (Line 53).

This inline auth.role !== 'admin' check bypasses the shared authorization contract for API routes and can drift from canonical policy behavior.

As per coding guidelines, apps/web/src/app/api/**/*.{ts,tsx} must “Use centralized permission logic from @pagespace/lib/permissions/permissions via getUserAccessLevel() and canUserEditPage() functions,” and **/*.{ts,tsx} must “Always use centralized permission functions from packages/lib/src/permissions/. Never roll your own access checks.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/app/api/pages/route.ts` around lines 53 - 56, Replace the inline
authorization check in the TERMINAL page type validation (the condition checking
`auth.role !== 'admin'`) with the centralized permission helper functions from
`@pagespace/lib/permissions/permissions`. Instead of directly comparing auth.role,
use either getUserAccessLevel() or canUserEditPage() to determine if the current
user has permission to create a TERMINAL page type. Keep the existing audit
request call and error response unchanged, but update the condition logic to
delegate authorization checks to the centralized permission module.

Source: Coding guidelines


// Check MCP token scope - scoped tokens can only create pages in allowed drives
const scopeError = checkMCPCreateScope(auth, validatedData.driveId);
Expand Down
9 changes: 7 additions & 2 deletions apps/web/src/components/create/QuickCreatePalette.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { uploadFileToS3 } from '@/lib/upload/orchestrator';
import { useUIStore } from '@/stores/useUIStore';
import { usePageNavigation } from '@/hooks/usePageNavigation';
import { useBreadcrumbs } from '@/hooks/useBreadcrumbs';
import { useAuth } from '@/hooks/useAuth';
import { useDisplayPreferences } from '@/hooks/useDisplayPreferences';
import { matchesKeyEvent, getEffectiveBinding } from '@/stores/useHotkeyStore';
import { isEditingActive } from '@/stores/useEditingStore';
Expand Down Expand Up @@ -131,6 +132,7 @@ export default function QuickCreatePalette() {
const closeQuickCreate = useUIStore((s) => s.closeQuickCreate);

const { navigateToPage } = usePageNavigation();
const { user } = useAuth();
const { preferences } = useDisplayPreferences();
const { data: tree } = useCachedPageTree(driveId);

Expand Down Expand Up @@ -251,7 +253,7 @@ export default function QuickCreatePalette() {
} finally {
setIsCreating(false);
}
}, [driveId, selectedType, isCreating, selectedFile, name, effectiveParentId, preferences, closeQuickCreate, navigateToPage]);
}, [driveId, selectedType, isCreating, selectedFile, name, effectiveParentId, preferences, closeQuickCreate, swrMutate, navigateToPage]);

const handleKeyDownNameEntry = useCallback(
(e: React.KeyboardEvent) => {
Expand All @@ -266,7 +268,10 @@ export default function QuickCreatePalette() {
[handleCreate]
);

const creatableTypes = getCreatablePageTypes();
const creatableTypes = useMemo(() => {
const types = getCreatablePageTypes();
return user?.role === 'admin' ? [...types, PageType.TERMINAL] : types;
}, [user?.role]);

if (!quickCreateOpen) return null;

Expand Down
16 changes: 13 additions & 3 deletions apps/web/src/components/layout/middle-content/page-views/terminal/TerminalView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const TerminalView = ({ pageId }: TerminalViewProps) => {
const socket = useSocket();
const { user } = useAuth();
const { resolvedTheme } = useTheme();
const isAdmin = user?.role === 'admin';

const {
document: documentState,
Expand Down Expand Up @@ -139,6 +140,7 @@ const TerminalView = ({ pageId }: TerminalViewProps) => {
// Handle command submission — gated on document initialization
const handleCommand = useCallback(async (command: string) => {
if (!documentState) { toast.error('Terminal is still loading'); return; }
if (!isAdmin) { toast.error('Terminal access requires administrator privileges'); return; }
if (isReadOnly) { toast.error('You do not have permission to edit this page'); return; }

setIsExecuting(true);
Expand Down Expand Up @@ -177,7 +179,7 @@ const TerminalView = ({ pageId }: TerminalViewProps) => {
saveWithDebounce(serialized);
return updated;
});
}, [isReadOnly, documentState, pageId, updateContent, saveWithDebounce]);
}, [isAdmin, isReadOnly, documentState, pageId, updateContent, saveWithDebounce]);

// Handle clearing the terminal
const handleClear = useCallback(() => {
Expand Down Expand Up @@ -238,7 +240,15 @@ const TerminalView = ({ pageId }: TerminalViewProps) => {
transition={{ duration: 0.2 }}
className="h-full flex flex-col relative"
>
{isReadOnly && (
{!isAdmin && (
<div className="bg-yellow-50 dark:bg-yellow-900/20 border-b border-yellow-200 dark:border-yellow-800 px-4 py-2">
<p className="text-sm text-yellow-800 dark:text-yellow-200 text-center">
Terminal access requires administrator privileges
</p>
</div>
)}

{isAdmin && isReadOnly && (
<div className="bg-yellow-50 dark:bg-yellow-900/20 border-b border-yellow-200 dark:border-yellow-800 px-4 py-2">
<p className="text-sm text-yellow-800 dark:text-yellow-200 text-center">
You don&apos;t have permission to edit this page
Expand All @@ -252,7 +262,7 @@ const TerminalView = ({ pageId }: TerminalViewProps) => {
onCommand={handleCommand}
onClear={handleClear}
isDark={isDark}
isReadOnly={isReadOnly || isLoading || !documentState || isExecuting}
isReadOnly={!isAdmin || isReadOnly || isLoading || !documentState || isExecuting}
/>
</div>

Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/services/api/page-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ function sanitizeEmptyContent(content: string): string {
/**
* Page types
*/
export type PageType = 'FOLDER' | 'DOCUMENT' | 'CHANNEL' | 'AI_CHAT' | 'CANVAS' | 'SHEET' | 'TASK_LIST' | 'CODE';
export type PageType = 'FOLDER' | 'DOCUMENT' | 'CHANNEL' | 'AI_CHAT' | 'CANVAS' | 'SHEET' | 'TASK_LIST' | 'CODE' | 'TERMINAL';

/**
* Message with user info for page details
Expand Down
38 changes: 38 additions & 0 deletions packages/lib/src/services/sandbox/__tests__/can-run-code.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ const agentViewOnlyPerms: PermissionLevel = {
function makeDeps(overrides: Partial<CanRunCodeDeps> = {}): CanRunCodeDeps {
return {
getUserDrivePermissions: async () => adminPerms,
getUserRole: async () => 'admin',
getAgentAccessLevel: async () => agentEditPerms,
isCodeExecutionEnabled: () => true,
getNodeEnv: () => 'test',
...overrides,
};
}
Expand Down Expand Up @@ -94,6 +96,42 @@ describe('canRunCode', () => {
expect(result).toEqual({ ok: false, reason: 'insufficient_role' });
});

it('given production and a non-admin app user, should deny even with drive admin access', async () => {
const result = await canRunCode({
userId: 'u1',
driveId: 'd1',
deps: makeDeps({
getUserRole: async () => 'user',
getNodeEnv: () => 'production',
}),
});
expect(result).toEqual({ ok: false, reason: 'app_admin_required' });
});

it('given production and an admin app user, should allow when drive authorization passes', async () => {
const result = await canRunCode({
userId: 'u1',
driveId: 'd1',
deps: makeDeps({
getUserRole: async () => 'admin',
getNodeEnv: () => 'production',
}),
});
expect(result.ok).toBe(true);
});

it('given development and a non-admin app user, should preserve the drive-role gate', async () => {
const result = await canRunCode({
userId: 'u1',
driveId: 'd1',
deps: makeDeps({
getUserRole: async () => 'user',
getNodeEnv: () => 'development',
}),
});
expect(result.ok).toBe(true);
});

it('given an agent actor with edit access, should allow', async () => {
const result = await canRunCode({
userId: 'u1',
Expand Down
Loading
Loading