mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-22 15:15:18 +00:00
chore(core): P3 self-review R2 — vm-realm opts revive + error-msg sanitize + 12 tests
R2 of pre-push adversarial self-review on PR #5034. 6 diverse-lens finders (60 agents, ~2.5M tokens, 24 min) over the R1-fix-applied code, with 2 independent skeptics defaulting to refuted=true. 12 confirmed survivors after adversarial verify; decisions below. Security (FIX): - agent() wrapper in workflow-sandbox.ts now JSON-revives agentOpts inside the vm runInContext block BEFORE passing them to the host dispatch. Closes a Proxy/inherited-getter escape that P3 introduced along with the user-supplied schema object: a script could have wrapped agentOpts.schema in a Proxy whose getter ran host-side code during SyntheticOutputTool construction / AJV compile. Same mechanism as args / parallel-result revival. - runOverridePath now sanitizes opts.agentType through sanitizeForErrorMessage() (control chars → space) before interpolation into the "agent type 'X' not found" error message. Prevents a model-authored agentType containing CRLF / NUL from fragmenting a single-line error across log records / OTLP fields. Reuse-altitude (FIX): - Added JSDoc block to WorkflowWorktreeIsolation interface documenting each field's role for cleanup. Test gaps (FIX, 12 new tests): - agentType control-char sanitization regression - dispose() runs in finally when subagent.execute throws - isolation:'worktree' provision error branches (5): nested parent / git unavailable / not a git repo / parent dirty / createUserWorktree returns failure - isolation:'worktree' cleanup branches (3): removeUserWorktree fails / branchPreserved race / removeUserWorktree throws — each preserves the worktree (or branch) with the right user-facing suffix - combinations (2): model + isolation:'worktree' threads model AND provisions worktree; schema + isolation:'worktree' returns structured payload verbatim (preserved suffix only on string return) Test infrastructure: vi.mock'd GitWorktreeService at the module level (partial mock; preserves the existing exports the unrelated worktreeCleanup.ts depends on) with a per-test beforeEach reset. Declined R2 findings (kept the R1 line): - [major] Schema parameter upfront validation: same scope-creep decline as R1. Upstream doesn't do it; AJV's downstream error is descriptive enough. - [major] Worktree provision extracted to shared util with AgentTool: agreed in principle but out of P3 scope. A separate refactor PR should land that with AgentTool maintainers in the loop. 178/178 tests pass (workflow + adjacent suites). typecheck + lint clean across packages/core, packages/cli, integration-tests, sdk, webui.
This commit is contained in:
LaZzyMan
parent
e1c5ec79c3
commit
62624a9949
3 changed files with 433 additions and 2 deletions
|
|
@ -41,6 +41,57 @@ const { created, nextTerminateMode } = vi.hoisted(() => ({
|
|||
nextTerminateMode: { value: 'GOAL' as string },
|
||||
}));
|
||||
|
||||
// P3 R2 self-review (P3-T6 gap, batch): tests below for
|
||||
// agent({isolation:'worktree'}) need to drive GitWorktreeService's
|
||||
// provision/cleanup branches deterministically. Module-level mock with
|
||||
// per-test stub overrides via vi.mocked(...).mockImplementation.
|
||||
//
|
||||
// Default behaviour = "everything succeeds and the worktree is clean
|
||||
// post-spawn" so the cleanup branch removes the worktree. Tests that
|
||||
// need a specific error path override the relevant method.
|
||||
const worktreeStubs = vi.hoisted(() => {
|
||||
const makeStub = () => ({
|
||||
checkGitAvailable: vi.fn(async () => ({ available: true })),
|
||||
isGitRepository: vi.fn(async () => true),
|
||||
getRepoTopLevel: vi.fn(async () => '/fake/repo'),
|
||||
getCurrentBranch: vi.fn(async () => 'main'),
|
||||
hasWorktreeChanges: vi.fn(async () => false),
|
||||
hasUnmergedWorktreeCommits: vi.fn(async () => false),
|
||||
createUserWorktree: vi.fn(
|
||||
async (
|
||||
slug: string,
|
||||
_base?: string,
|
||||
_options?: { symlinkDirectories?: readonly string[] },
|
||||
) => ({
|
||||
success: true,
|
||||
worktree: {
|
||||
path: `/fake/repo/.qwen/worktrees/${slug}`,
|
||||
branch: `worktree-${slug}`,
|
||||
},
|
||||
}),
|
||||
),
|
||||
removeUserWorktree: vi.fn(async () => ({ success: true })),
|
||||
});
|
||||
return { makeStub, instances: [] as Array<ReturnType<typeof makeStub>> };
|
||||
});
|
||||
|
||||
vi.mock('../../services/gitWorktreeService.js', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<
|
||||
typeof import('../../services/gitWorktreeService.js')
|
||||
>();
|
||||
return {
|
||||
...actual,
|
||||
generateAgentWorktreeSlug: () => 'agent-deadbe1',
|
||||
writeWorktreeSessionMarker: vi.fn(async () => {}),
|
||||
GitWorktreeService: vi.fn().mockImplementation(() => {
|
||||
const stub = worktreeStubs.makeStub();
|
||||
worktreeStubs.instances.push(stub);
|
||||
return stub;
|
||||
}),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('./agent-headless.js', () => ({
|
||||
AgentHeadless: {
|
||||
create: async (
|
||||
|
|
@ -732,6 +783,21 @@ describe('WorkflowOrchestrator P2 — parallel() / pipeline() / caps', () => {
|
|||
// stub matching just enough surface (findSubagentByName +
|
||||
// createAgentHeadless) for the path under test.
|
||||
describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', () => {
|
||||
// Reset GitWorktreeService stub state between tests so an override set
|
||||
// by one test does not bleed into the next (mockImplementation is
|
||||
// persistent; the per-test overrides below rely on a clean baseline).
|
||||
beforeEach(async () => {
|
||||
const { GitWorktreeService } = await import(
|
||||
'../../services/gitWorktreeService.js'
|
||||
);
|
||||
worktreeStubs.instances.length = 0;
|
||||
vi.mocked(GitWorktreeService).mockImplementation(() => {
|
||||
const stub = worktreeStubs.makeStub();
|
||||
worktreeStubs.instances.push(stub);
|
||||
return stub as unknown as InstanceType<typeof GitWorktreeService>;
|
||||
});
|
||||
});
|
||||
|
||||
type StubSubagentCall = {
|
||||
config: { name?: string; model?: string; disallowedTools?: string[] };
|
||||
runtimeContextSame: boolean;
|
||||
|
|
@ -785,6 +851,12 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', (
|
|||
const cfg = {
|
||||
createToolRegistry: async () => fakeRegistry,
|
||||
getToolRegistry: () => fakeRegistry,
|
||||
// P3 R2 self-review: isolation:'worktree' provisioning reads
|
||||
// these methods. Provide deterministic returns so the tests can
|
||||
// drive GitWorktreeService stubs without re-deriving cwd.
|
||||
getTargetDir: () => '/fake/repo',
|
||||
getSessionId: () => 'sess_fake_test_id',
|
||||
getWorktreeSymlinkDirectories: () => [],
|
||||
getSubagentManager: () => ({
|
||||
findSubagentByName: opts.findSubagentByName ?? (async () => null),
|
||||
createAgentHeadless: async (
|
||||
|
|
@ -1305,4 +1377,306 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', (
|
|||
);
|
||||
expect(helper.disposed).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
// R2 self-review (sec-2): sanitize control characters in user-controlled
|
||||
// strings before interpolating into error messages so a model-authored
|
||||
// agentType cannot fragment a single-line error across log records.
|
||||
it('agentType not found: control chars in name are scrubbed from the error message', async () => {
|
||||
const { config } = fakeConfigWithMgr({
|
||||
findSubagentByName: async () => null,
|
||||
onCreate: async () => ({ finalText: '', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
// newline + nul + del — all control codes < 0x20 or == 0x7f.
|
||||
const evil = 'Explore\n\rEvil\x00\x7f';
|
||||
await expect(dispatch('hi', { agentType: evil })).rejects.toThrow();
|
||||
// The thrown message must NOT contain raw newlines / NULs.
|
||||
try {
|
||||
await dispatch('hi', { agentType: evil });
|
||||
} catch (err) {
|
||||
const msg = (err as Error).message;
|
||||
// eslint-disable-next-line no-control-regex
|
||||
expect(msg).not.toMatch(/[\n\r\u0000\u007f]/);
|
||||
expect(msg).toContain('not found');
|
||||
}
|
||||
});
|
||||
|
||||
// R2 self-review (test-5): dispose() MUST still run even when
|
||||
// subagent.execute throws synchronously from inside the in-flight
|
||||
// event-emitter callback (schema-mode failure path). The R1 dispose
|
||||
// tests covered terminate-mode-non-GOAL; this covers the thrown-from-
|
||||
// execute branch.
|
||||
it('override path: dispose() still runs in finally when execute throws', async () => {
|
||||
const helper = fakeConfigWithMgr({
|
||||
onCreate: async () => ({
|
||||
finalText: '',
|
||||
terminateMode: 'GOAL', // not reached
|
||||
runWithEmitter: (_emitter) => {
|
||||
throw new Error('simulated subagent failure');
|
||||
},
|
||||
}),
|
||||
});
|
||||
const dispatch = createProductionDispatch(helper.config);
|
||||
await expect(
|
||||
dispatch('extract', { schema: { type: 'object' } }),
|
||||
).rejects.toThrow(/simulated subagent failure/);
|
||||
expect(helper.disposed).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
// ─── isolation:'worktree' provision error branches ──────────────
|
||||
// R2 self-review (test-1, [critical]): each provisionWorkflowWorktree
|
||||
// error branch had no unit test. Coverage came only from the real-
|
||||
// LLM E2E S7 happy path. Adversarial review noted the parent-dirty
|
||||
// refuse, nested-worktree refuse, and git-not-available paths can
|
||||
// change behavior with no regression signal. Each test below stubs
|
||||
// the GitWorktreeService method that controls the branch.
|
||||
|
||||
it("isolation:'worktree' refuses when parent cwd is already inside a worktree", async () => {
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'unused', terminateMode: 'GOAL' }),
|
||||
});
|
||||
// Override getTargetDir to look nested-worktree-ish.
|
||||
(config as unknown as { getTargetDir: () => string }).getTargetDir = () =>
|
||||
'/some/repo/.qwen/worktrees/agent-existing/inner';
|
||||
const dispatch = createProductionDispatch(config);
|
||||
await expect(dispatch('hi', { isolation: 'worktree' })).rejects.toThrow(
|
||||
/already inside a worktree/,
|
||||
);
|
||||
});
|
||||
|
||||
it("isolation:'worktree' refuses when git is not available", async () => {
|
||||
worktreeStubs.instances.length = 0; // reset
|
||||
const { GitWorktreeService } = await import(
|
||||
'../../services/gitWorktreeService.js'
|
||||
);
|
||||
vi.mocked(GitWorktreeService).mockImplementation(
|
||||
() =>
|
||||
({
|
||||
...worktreeStubs.makeStub(),
|
||||
checkGitAvailable: vi.fn(async () => ({
|
||||
available: false,
|
||||
error: 'git binary missing',
|
||||
})),
|
||||
}) as unknown as InstanceType<typeof GitWorktreeService>,
|
||||
);
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'unused', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
await expect(dispatch('hi', { isolation: 'worktree' })).rejects.toThrow(
|
||||
/git binary missing/,
|
||||
);
|
||||
});
|
||||
|
||||
it("isolation:'worktree' refuses when cwd is not a git repository", async () => {
|
||||
const { GitWorktreeService } = await import(
|
||||
'../../services/gitWorktreeService.js'
|
||||
);
|
||||
vi.mocked(GitWorktreeService).mockImplementation(
|
||||
() =>
|
||||
({
|
||||
...worktreeStubs.makeStub(),
|
||||
isGitRepository: vi.fn(async () => false),
|
||||
}) as unknown as InstanceType<typeof GitWorktreeService>,
|
||||
);
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'unused', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
await expect(dispatch('hi', { isolation: 'worktree' })).rejects.toThrow(
|
||||
/not a git repository/,
|
||||
);
|
||||
});
|
||||
|
||||
it("isolation:'worktree' refuses when parent working tree is dirty", async () => {
|
||||
const { GitWorktreeService } = await import(
|
||||
'../../services/gitWorktreeService.js'
|
||||
);
|
||||
vi.mocked(GitWorktreeService).mockImplementation(
|
||||
() =>
|
||||
({
|
||||
...worktreeStubs.makeStub(),
|
||||
hasWorktreeChanges: vi.fn(async () => true),
|
||||
}) as unknown as InstanceType<typeof GitWorktreeService>,
|
||||
);
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'unused', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
await expect(dispatch('hi', { isolation: 'worktree' })).rejects.toThrow(
|
||||
/uncommitted changes/,
|
||||
);
|
||||
});
|
||||
|
||||
it("isolation:'worktree' surfaces createUserWorktree failure", async () => {
|
||||
const { GitWorktreeService } = await import(
|
||||
'../../services/gitWorktreeService.js'
|
||||
);
|
||||
vi.mocked(GitWorktreeService).mockImplementation(
|
||||
() =>
|
||||
({
|
||||
...worktreeStubs.makeStub(),
|
||||
createUserWorktree: vi.fn(async () => ({
|
||||
success: false,
|
||||
error: 'simulated worktree create failure',
|
||||
})),
|
||||
}) as unknown as InstanceType<typeof GitWorktreeService>,
|
||||
);
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'unused', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
await expect(dispatch('hi', { isolation: 'worktree' })).rejects.toThrow(
|
||||
/simulated worktree create failure/,
|
||||
);
|
||||
});
|
||||
|
||||
// ─── isolation:'worktree' cleanup error branches ────────────────
|
||||
// R2 self-review (test-2, [major]): cleanupWorkflowWorktree has 3
|
||||
// notable error/branch transitions: removeUserWorktree returns
|
||||
// {success:false}, returns {success:true, branchPreserved:true}, or
|
||||
// throws synchronously. Each path produces a different preserved
|
||||
// suffix and was previously untested.
|
||||
|
||||
it("isolation:'worktree' cleanup: removeUserWorktree failure preserves path+branch", async () => {
|
||||
const { GitWorktreeService } = await import(
|
||||
'../../services/gitWorktreeService.js'
|
||||
);
|
||||
vi.mocked(GitWorktreeService).mockImplementation(
|
||||
() =>
|
||||
({
|
||||
...worktreeStubs.makeStub(),
|
||||
// Subagent left the worktree clean.
|
||||
hasWorktreeChanges: vi
|
||||
.fn(async () => false)
|
||||
// ... but the subsequent cleanup-side hasWorktreeChanges
|
||||
// (called from inside cleanupWorkflowWorktree) also reports
|
||||
// clean. Cleanup proceeds to removeUserWorktree which fails.
|
||||
.mockImplementation(async () => false),
|
||||
removeUserWorktree: vi.fn(async () => ({
|
||||
success: false,
|
||||
error: 'simulated remove failure',
|
||||
})),
|
||||
}) as unknown as InstanceType<typeof GitWorktreeService>,
|
||||
);
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'done', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
const result = await dispatch('hi', { isolation: 'worktree' });
|
||||
// Suffix should appear because removeUserWorktree failed → preserve.
|
||||
expect(String(result)).toMatch(
|
||||
/\[worktree preserved:.*\(branch worktree-agent-deadbe1\)\]/,
|
||||
);
|
||||
});
|
||||
|
||||
it("isolation:'worktree' cleanup: branchPreserved race yields branch-only suffix", async () => {
|
||||
const { GitWorktreeService } = await import(
|
||||
'../../services/gitWorktreeService.js'
|
||||
);
|
||||
vi.mocked(GitWorktreeService).mockImplementation(
|
||||
() =>
|
||||
({
|
||||
...worktreeStubs.makeStub(),
|
||||
removeUserWorktree: vi.fn(async () => ({
|
||||
success: true,
|
||||
branchPreserved: true, // race: commits landed between checks and delete
|
||||
})),
|
||||
}) as unknown as InstanceType<typeof GitWorktreeService>,
|
||||
);
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'done', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
const result = await dispatch('hi', { isolation: 'worktree' });
|
||||
// Directory-removed-but-branch-preserved suffix matches AgentTool
|
||||
// verbatim and includes the recover hint.
|
||||
expect(String(result)).toMatch(/worktree directory removed/);
|
||||
expect(String(result)).toMatch(/git worktree add/);
|
||||
});
|
||||
|
||||
it("isolation:'worktree' cleanup: thrown removeUserWorktree preserves path+branch", async () => {
|
||||
const { GitWorktreeService } = await import(
|
||||
'../../services/gitWorktreeService.js'
|
||||
);
|
||||
vi.mocked(GitWorktreeService).mockImplementation(
|
||||
() =>
|
||||
({
|
||||
...worktreeStubs.makeStub(),
|
||||
removeUserWorktree: vi.fn(async () => {
|
||||
throw new Error('simulated git crash during remove');
|
||||
}),
|
||||
}) as unknown as InstanceType<typeof GitWorktreeService>,
|
||||
);
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'done', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
const result = await dispatch('hi', { isolation: 'worktree' });
|
||||
expect(String(result)).toMatch(
|
||||
/\[worktree preserved:.*\(branch worktree-agent-deadbe1\)\]/,
|
||||
);
|
||||
});
|
||||
|
||||
// ─── isolation:'worktree' option combinations ───────────────────
|
||||
// R2 self-review (test-3/4): single-option tests don't catch
|
||||
// interactions. These verify model+worktree and schema+worktree
|
||||
// each compose correctly.
|
||||
|
||||
it("model + isolation:'worktree': model threaded through AND worktree provisioned", async () => {
|
||||
const { config, calls } = fakeConfigWithMgr({
|
||||
onCreate: async () => ({ finalText: 'done', terminateMode: 'GOAL' }),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
const result = await dispatch('hi', {
|
||||
model: 'qwen3-max',
|
||||
isolation: 'worktree',
|
||||
});
|
||||
expect(calls[0].config.model).toBe('qwen3-max');
|
||||
// Default-clean stub auto-removes; no suffix expected.
|
||||
expect(String(result)).not.toMatch(/worktree preserved/);
|
||||
});
|
||||
|
||||
it("schema + isolation:'worktree': structured payload returned, worktree info logged", async () => {
|
||||
const { config } = fakeConfigWithMgr({
|
||||
onCreate: async (_call, _ee) => ({
|
||||
finalText: '',
|
||||
terminateMode: 'CANCELLED',
|
||||
runWithEmitter: (emitter) => {
|
||||
emitter.emit('tool_call', {
|
||||
subagentId: 'sub',
|
||||
round: 1,
|
||||
callId: 'c1',
|
||||
name: 'structured_output',
|
||||
args: { ok: true, in_worktree: true },
|
||||
description: '',
|
||||
isOutputMarkdown: false,
|
||||
timestamp: 1,
|
||||
});
|
||||
emitter.emit('tool_result', {
|
||||
subagentId: 'sub',
|
||||
round: 1,
|
||||
callId: 'c1',
|
||||
name: 'structured_output',
|
||||
success: true,
|
||||
responseParts: [],
|
||||
resultDisplay: '',
|
||||
durationMs: 1,
|
||||
timestamp: 1,
|
||||
});
|
||||
},
|
||||
}),
|
||||
});
|
||||
const dispatch = createProductionDispatch(config);
|
||||
const result = await dispatch('extract from worktree', {
|
||||
schema: { type: 'object' },
|
||||
isolation: 'worktree',
|
||||
});
|
||||
// Schema-mode returns the structured object verbatim, not the
|
||||
// string-with-suffix shape. The preserved-suffix mechanism intentionally
|
||||
// does NOT mutate the structured payload — operator-visible info goes
|
||||
// to debugLogger instead. See runOverridePath's "schema-mode... payload
|
||||
// is returned verbatim" comment.
|
||||
expect(result).toEqual({ ok: true, in_worktree: true });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -210,6 +210,19 @@ function generateRunId(): string {
|
|||
return `wf_${randomBytes(8).toString('hex')}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize a user-controlled string for safe interpolation into an error
|
||||
* message. Control characters (CR / LF / NUL / etc.) are replaced with a
|
||||
* single space so a model-authored agentType cannot fragment a single-line
|
||||
* error across log records / OTLP fields / display payloads. P3 R2
|
||||
* self-review surfaced this; the corresponding throw site is in
|
||||
* `runOverridePath` for `opts.agentType`.
|
||||
*/
|
||||
function sanitizeForErrorMessage(value: string): string {
|
||||
// eslint-disable-next-line no-control-regex
|
||||
return value.replace(/[\u0000-\u001f\u007f]+/g, ' ');
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the production agent-dispatch function.
|
||||
*
|
||||
|
|
@ -352,8 +365,15 @@ async function runOverridePath(
|
|||
// "agent({agentType}): agent type '{name}' not found". Match for
|
||||
// user-visible parity so scripts authored against either runtime see
|
||||
// the same error text.
|
||||
//
|
||||
// SECURITY (P3 R2 self-review): sanitize opts.agentType before
|
||||
// interpolation. The string is model-authored; an attacker model
|
||||
// could embed CRLF / control characters that fragment the error
|
||||
// message in logs / display / OTLP traces. Replace control chars
|
||||
// with a single space so the error stays single-line.
|
||||
const safeAgentType = sanitizeForErrorMessage(opts.agentType);
|
||||
throw new Error(
|
||||
`agent({agentType}): agent type '${opts.agentType}' not found.`,
|
||||
`agent({agentType}): agent type '${safeAgentType}' not found.`,
|
||||
);
|
||||
}
|
||||
baseConfig = resolved;
|
||||
|
|
@ -556,6 +576,25 @@ interface WorktreePreservedInfo {
|
|||
branch: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* In-flight handle for one `agent({isolation:'worktree'})` provision —
|
||||
* passed from `provisionWorkflowWorktree` to `cleanupWorkflowWorktree`
|
||||
* (plus the outer-finally fallback) so cleanup can address the right
|
||||
* worktree without re-discovering it from the filesystem.
|
||||
*
|
||||
* - `slug` — the ephemeral `agent-<7hex>` name; used as the
|
||||
* `removeUserWorktree` argument AND as the input to
|
||||
* `hasUnmergedWorktreeCommits` (which resolves the slug to its
|
||||
* branch name internally).
|
||||
* - `path` — absolute worktree directory under
|
||||
* `<projectRoot>/.qwen/worktrees/`; the subagent's rebound cwd.
|
||||
* - `branch` — the branch created for this worktree
|
||||
* (`worktree-<slug>`); appears verbatim in the user-facing preserved
|
||||
* suffix.
|
||||
* - `repoRoot` — the parent project root; the cleanup helper builds a
|
||||
* fresh `GitWorktreeService` anchored here so the worktree-side
|
||||
* git invocations target the right repo.
|
||||
*/
|
||||
interface WorkflowWorktreeIsolation {
|
||||
slug: string;
|
||||
path: string;
|
||||
|
|
|
|||
|
|
@ -532,6 +532,24 @@ export function createWorkflowSandbox(opts: SandboxOptions): WorkflowSandbox {
|
|||
__b.pushPhase(agentOpts.phase);
|
||||
}
|
||||
}
|
||||
// SECURITY (P3 R2 self-review): user-script-controlled agentOpts
|
||||
// cross the vm/host boundary verbatim via vmAsync's hostFn.apply.
|
||||
// A Proxy / inherited-getter / non-plain object in agentOpts.schema
|
||||
// would let host-side code (SyntheticOutputTool constructor + AJV
|
||||
// compile) trigger user-controlled trap handlers that execute with
|
||||
// the host realm's full surface. Revive agentOpts through JSON
|
||||
// round-trip BEFORE crossing so the host only ever sees vm-realm
|
||||
// plain objects with vm-realm prototypes. Same mechanism that
|
||||
// makes args + parallel/pipeline results safe.
|
||||
var safeOpts;
|
||||
try {
|
||||
safeOpts = JSON.parse(JSON.stringify(agentOpts));
|
||||
} catch (e) {
|
||||
throw new Error(
|
||||
"agent() opts contain a non-JSON-serializable value: " +
|
||||
String(e && e.message != null ? e.message : e)
|
||||
);
|
||||
}
|
||||
// SECURITY (PR #4947 R1 wenshao, extended for P3): vmAsync's resolve
|
||||
// path is verbatim (no re-wrap of resolved values). Host-realm
|
||||
// strings cross the boundary harmlessly because primitives have no
|
||||
|
|
@ -546,7 +564,7 @@ export function createWorkflowSandbox(opts: SandboxOptions): WorkflowSandbox {
|
|||
// global revival). The fallback to null on a non-serializable
|
||||
// resolve mirrors the errors-as-data convention parallel/pipeline
|
||||
// already use for individual slot failures.
|
||||
return __b.hostAgent(prompt, agentOpts).then(function (value) {
|
||||
return __b.hostAgent(prompt, safeOpts).then(function (value) {
|
||||
if (value === null || typeof value !== 'object') {
|
||||
return value;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue