Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
90 changes: 90 additions & 0 deletions src/tools/built-in/github.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { GitHubTools, createGitHubTools } from './github';
import { NoopSandbox } from '../../security/sandboxCore';
import type { SandboxAdapter } from '../../security/sandboxCore';
import { executeToolWithSandboxGuard } from '../../execution/sandboxGuard';

describe('GitHubTools scope enforcement (LOU-E14)', () => {
let githubTools: GitHubTools;
Expand Down Expand Up @@ -104,3 +107,90 @@ describe('GitHubTools scope enforcement (LOU-E14)', () => {
}
});
});

describe('GitHubTools sandbox seam (LOU-K2)', () => {
let githubTools: GitHubTools;

beforeEach(() => {
githubTools = createGitHubTools({
token: 'test-token',
owner: 'test-owner',
repo: 'test-repo',
});
});

it('flags requiresSandbox on an in-scope (real-fetching) tool, and leaves execute() untouched', async () => {
const descriptor = githubTools.get('github_get_pull_request');
expect(descriptor?.requiresSandbox).toBe(true);
expect(typeof descriptor?.sandboxExecute).toBe('function');

// execute() itself is still real and directly callable (unsandboxed
// callers like examples/ops-pipeline's guardedPr.ts rely on this).
const fetchSpy = vi.fn().mockResolvedValue({
ok: true,
json: async () => ({
number: 5,
title: 't',
body: 'b',
state: 'open',
html_url: 'https://github.com/test-owner/test-repo/pull/5',
head: { ref: 'feature' },
base: { ref: 'main' },
}),
});
vi.stubGlobal('fetch', fetchSpy);
try {
const result = await descriptor!.tool.execute!({ prNumber: 5 }, {} as any);
expect(fetchSpy).toHaveBeenCalledTimes(1);
expect(JSON.parse(result as string).number).toBe(5);
} finally {
vi.unstubAllGlobals();
}
});

it('does not flag requiresSandbox on an out-of-scope tool (it never reaches the network)', () => {
const descriptor = githubTools.get('github_create_branch');
expect(descriptor?.requiresSandbox).toBeUndefined();
});

it('(b) a custom SandboxAdapter actually gets invoked for the outbound GitHub API call', async () => {
const runSpy = vi.fn(async (cmd: string, args: string[]) => {
expect(cmd).toBe('node');
expect(args[0]).toBe('-e');
return {
stdout: JSON.stringify({
status: 200,
statusText: 'OK',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({
number: 7,
title: 'sandboxed',
body: 'b',
state: 'open',
html_url: 'https://github.com/test-owner/test-repo/pull/7',
head: { ref: 'feature' },
base: { ref: 'main' },
}),
}),
stderr: '',
exitCode: 0,
};
});
const customSandbox: SandboxAdapter = { name: 'custom-test-sandbox', run: runSpy, writeFile: vi.fn() };

const descriptor = githubTools.get('github_get_pull_request')!;
const result = await executeToolWithSandboxGuard('github_get_pull_request', descriptor, { prNumber: 7 }, customSandbox);

expect(runSpy).toHaveBeenCalledTimes(1);
expect(JSON.parse(result as string).number).toBe(7);
});

it('(c) fails closed when requiresSandbox is true but sandboxExecute is missing', async () => {
const descriptor = githubTools.get('github_get_pull_request')!;
const broken = { ...descriptor, sandboxExecute: undefined };

await expect(
executeToolWithSandboxGuard('github_get_pull_request', broken, { prNumber: 1 }, NoopSandbox)
).rejects.toThrow(/requiresSandbox but does not implement sandboxExecute/);
});
});
28 changes: 28 additions & 0 deletions src/tools/built-in/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ import { tool } from 'ai';
import { z } from 'zod';
import { ToolRegistry } from '../ToolRegistry';
import { ToolDescriptor } from '../../types';
import { SandboxAdapter } from '../../security/sandboxCore';
import { withSandboxedFetch } from './sandboxFetch';

// ============================================================================
// Type Definitions
Expand Down Expand Up @@ -369,7 +371,33 @@ export class GitHubTools extends ToolRegistry {
`been disabled.`
);
}) as typeof descriptor.tool.execute;
// Out-of-scope tools above throw before ever making an HTTP request,
// so there's no real outbound call for a sandbox boundary to be
// meaningful on - they're deliberately left unflagged below.
super.register(name, descriptor);
return;
}

// LOU-K2: every remaining GitHub tool's execute() makes a real outbound
// HTTP call to the GitHub API via the ambient global `fetch`. Flag it
// requiresSandbox and route sandboxExecute() through
// withSandboxedFetch(), which swaps `fetch` for a SandboxAdapter-routed
// implementation just for the duration of the original execute() call -
// so a caller going through executeToolWithSandboxGuard() (AgentExecutor,
// resume.ts) gets the fetch call genuinely routed through the
// configured SandboxAdapter, without every one of this file's ~30
// register*() methods needing its own hand-written sandboxed
// reimplementation. execute() itself is left completely unchanged -
// still a real, directly-callable implementation - for callers that
// invoke descriptor.tool.execute() directly rather than through the
// guard (e.g. examples/ops-pipeline's guardedPr.ts).
if (descriptor.tool?.execute) {
const originalExecute = descriptor.tool.execute;
descriptor.requiresSandbox = true;
descriptor.sandboxExecute = (args: unknown, sandbox: SandboxAdapter) =>
withSandboxedFetch(sandbox, async () => originalExecute(args as any, {} as any));
}

super.register(name, descriptor);
}

Expand Down
72 changes: 71 additions & 1 deletion src/tools/built-in/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import https from 'https';
import dns from 'dns';
import selfsigned from 'selfsigned';
import type { AddressInfo } from 'net';
import { makeHttpRequest } from './http';
import { makeHttpRequest, createHttpTool } from './http';
import { NoopSandbox } from '../../security/sandboxCore';
import type { SandboxAdapter } from '../../security/sandboxCore';
import { executeToolWithSandboxGuard } from '../../execution/sandboxGuard';

function listen(server: http.Server): Promise<string> {
return new Promise((resolve) => {
Expand Down Expand Up @@ -311,4 +314,71 @@ describe('makeHttpRequest', () => {
expect(secureResult).not.toBe('unexpectedly-resolved');
});
});

describe('sandbox seam (LOU-K2)', () => {
it('flags requiresSandbox and implements sandboxExecute', () => {
const descriptor = createHttpTool();
expect(descriptor.requiresSandbox).toBe(true);
expect(typeof descriptor.sandboxExecute).toBe('function');
});

it('(a) NoopSandbox: sandboxExecute() gets the exact same real response as the unsandboxed execute() path', async () => {
const localServer = http.createServer((_req, res) => {
res.writeHead(200, { 'Content-Type': 'text/plain' });
res.end('sandboxed-ok');
});
const baseUrl = await listen(localServer);
try {
const descriptor = createHttpTool();

const direct = await descriptor.tool.execute!({ url: baseUrl, method: 'GET' }, {} as any);
const viaGuard = await executeToolWithSandboxGuard('http', descriptor, { url: baseUrl, method: 'GET' }, NoopSandbox);

expect(direct).toBe('sandboxed-ok');
expect(viaGuard).toBe('sandboxed-ok');
} finally {
await new Promise<void>((resolve) => localServer.close(() => resolve()));
}
}, 15000);

it('(b) a custom SandboxAdapter actually gets invoked for the outbound request', async () => {
const runSpy = vi.fn(async (cmd: string, args: string[]) => {
expect(cmd).toBe('node');
expect(args[0]).toBe('-e');
return {
stdout: JSON.stringify({ status: 200, statusText: 'OK', headers: {}, body: 'from-custom-sandbox' }),
stderr: '',
exitCode: 0,
};
});
const customSandbox: SandboxAdapter = {
name: 'custom-test-sandbox',
run: runSpy,
writeFile: vi.fn(),
};

const descriptor = createHttpTool();
// TEST-NET-3 (RFC 5737, 203.0.113.0/24): a real, non-blocked-range IP
// literal that skips isBlockedHost()'s DNS-resolution path entirely
// (isIP() short-circuits it) - so this test never touches real DNS.
const result = await executeToolWithSandboxGuard(
'http',
descriptor,
{ url: 'https://203.0.113.10/', method: 'GET' },
customSandbox
);

expect(runSpy).toHaveBeenCalledTimes(1);
expect(result).toBe('from-custom-sandbox');
});

it('(c) fails closed when requiresSandbox is true but sandboxExecute is missing', async () => {
const descriptor = createHttpTool();
const broken = { ...descriptor, sandboxExecute: undefined };

await expect(
executeToolWithSandboxGuard('http', broken, { url: 'https://203.0.113.10/', method: 'GET' }, NoopSandbox)
).rejects.toThrow(/requiresSandbox but does not implement sandboxExecute/);
});
});
});
Loading
Loading