Skip to content
Closed

Pr 6332 #6399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
4 changes: 3 additions & 1 deletion apps/daemon/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,15 @@
"pptxgenjs": "4.0.1",
"prom-client": "15.1.3",
"tar": "7.5.15",
"undici": "7.25.0"
"undici": "7.25.0",

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.

Pin both new dependency specs to exact versions. This line adds yauzl as ^3.4.0, and the same changed dependency block adds @types/yauzl as ^3.4.0; the repository guard reports both as violations because project dependencies must use exact versions or workspace:*. As written, pnpm guard exits 1, so the branch cannot satisfy the required merge validation and future installs could resolve unreviewed releases. Change both specs to 3.4.0 and refresh the lockfile with the pinned workspace pnpm version.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

"yauzl": "3.4.0"
},
"devDependencies": {
"@types/better-sqlite3": "7.6.13",
"@types/express": "5.0.6",
"@types/multer": "2.1.0",
"@types/node": "20.19.39",
"@types/yauzl": "3.4.0",
"jsdom": "29.1.1",
"typescript": "5.9.3",
"vitest": "4.1.6"
Expand Down
51 changes: 24 additions & 27 deletions apps/daemon/src/runtimes/defs/antigravity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import {
mkdirSync,
readFileSync,
writeFileSync,
renameSync,
statSync,
} from 'node:fs';
import { randomBytes } from 'node:crypto';
import { readFile as fsReadFile } from 'node:fs/promises';
import { homedir } from 'node:os';
import { dirname, join } from 'node:path';
Expand Down Expand Up @@ -49,9 +52,11 @@ export function writeAntigravityModelSelection(
label: string,
settingsPath: string = ANTIGRAVITY_SETTINGS_PATH,
): void {
let fileMode = 0o600;
let existing: Record<string, unknown> = {};
if (existsSync(settingsPath)) {
try {
fileMode = statSync(settingsPath).mode;
const parsed = JSON.parse(readFileSync(settingsPath, 'utf8')) as unknown;
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
existing = parsed as Record<string, unknown>;
Expand All @@ -63,7 +68,11 @@ export function writeAntigravityModelSelection(
}
existing.model = label;
mkdirSync(dirname(settingsPath), { recursive: true });
writeFileSync(settingsPath, `${JSON.stringify(existing, null, 2)}\n`);

// Use atomic write to prevent JSON corruption during concurrent agent spawns
const tempPath = `${settingsPath}.${randomBytes(4).toString('hex')}.tmp`;
writeFileSync(tempPath, `${JSON.stringify(existing, null, 2)}\n`, { mode: fileMode });
renameSync(tempPath, settingsPath);
}

// Per-process serialization for write-settings → spawn → agy-reads
Expand Down Expand Up @@ -215,39 +224,27 @@ export const antigravityAgentDef = {
runtimeContext.antigravitySettingsPath,
);
}
// We invoke agy via `-p -` (print mode + stdin sentinel), NOT
// `chat -`. Verified against `agy --help` on v1.0.3 — the
// `Available subcommands` list is `changelog / help / install /
// plugin / update`, and `chat` is NOT among them. `-p` is the
// documented print-mode flag (`Short alias for --print`) and
// `agy -p -` reads the prompt from stdin. The looper reviewer
// bot's environment runs a different agy build that may have
// renamed the entry point; until upstream confirms a stable
// headless subcommand (see google-antigravity/antigravity-cli#119)
// and the change actually ships in the auto-update channel that
// packaged OD users get, `-p -` is the contract that actually
// produces a print-mode reply on the installed CLI.
// We no longer use `-p -` because recent `agy` versions treat `-` as a literal
// prompt string instead of reading from stdin (see issue #5495).
// Instead, we use `promptViaFile: true` so the daemon securely prepares a
// managed temp file per-run and cleans it up after the agent exits.
if (!runtimeContext.promptFilePath) {
throw new Error('antigravity requires runtimeContext.promptFilePath when promptViaFile is true');
}

const args: string[] = [];
// Always opt into `--log-file` when the daemon supplied a path so
// it can post-exit grep for the actual upstream failure shape
// (auth missing vs quota reached vs upstream error) — without it
// the chat surfaces a generic "empty response" because print mode
// never echoes those errors on stdout. See server.ts empty-output
// guard for the consumer.
//
// Flag order is load-bearing on agy v1.0.3: `agy -p --log-file
// /tmp/x -` runs successfully but leaves /tmp/x empty, while `agy
// --log-file /tmp/x -p -` captures the diagnostic log, including
// `Propagating selected model override to backend: label="<model>"`
// and auth/quota failures.
if (runtimeContext.agentLogFilePath) {
args.push('--log-file', runtimeContext.agentLogFilePath);
}

args.push(`--add-dir=${dirname(runtimeContext.promptFilePath)}`);

args.push('-p');
args.push('-');
args.push(`Read the system instructions, conversation history, and user request from the file ${runtimeContext.promptFilePath}. Follow the instructions strictly and provide the final response to the user's latest request.`);
return args;
},
promptViaStdin: true,
promptViaStdin: false,
promptViaFile: true,
streamFormat: 'plain',
installUrl: 'https://antigravity.google/cli',
docsUrl: 'https://antigravity.google/docs/cli-overview',
Expand Down
97 changes: 78 additions & 19 deletions apps/daemon/src/services/plugin-installation.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import JSZip from 'jszip';
import stream from 'node:stream';
import util from 'node:util';
import yauzl from 'yauzl';

const pipeline = util.promisify(stream.pipeline);

export interface PluginInstallResult {
ok: boolean;
Expand Down Expand Up @@ -109,22 +113,72 @@ export function safeUploadRelativePath(input: unknown) {

export async function extractPluginZipToFolder(buffer: Buffer, stagedFolder: string, maxBytes: number) {
if (buffer.length > maxBytes) throw new Error('zip file too large');
const zip = await JSZip.loadAsync(buffer);
let totalBytes = 0;
const entries = Object.values(zip.files);
if (entries.length === 0) throw new Error('zip contains no files');
for (const entry of entries) {
if (entry.dir) continue;
const rel = safeUploadRelativePath(entry.name);
const unixMode = typeof entry.unixPermissions === 'number' ? entry.unixPermissions : 0;
if ((unixMode & 0o170000) === 0o120000) throw new Error(`zip entry is a symbolic link: ${entry.name}`);
const content = await entry.async('nodebuffer');
totalBytes += content.length;
if (totalBytes > maxBytes) throw new Error('zip extracted size exceeds 50 MiB');
const dest = path.join(stagedFolder, rel);
await fs.promises.mkdir(path.dirname(dest), { recursive: true });
await fs.promises.writeFile(dest, content);
}

const zip = await new Promise<yauzl.ZipFile>((resolve, reject) => {
yauzl.fromBuffer(buffer, { lazyEntries: true }, (err, zipfile) => {
if (err || !zipfile) reject(err ?? new Error('failed to load zip'));
else resolve(zipfile);
});
});

return new Promise<void>((resolve, reject) => {
let totalBytes = 0;
let entryCount = 0;

zip.on('error', reject);
zip.on('end', () => {
if (entryCount === 0) reject(new Error('zip contains no files'));
else resolve();
});

zip.on('entry', (entry: yauzl.Entry) => {
if (entry.fileName.endsWith('/')) {
zip.readEntry();
return;
}
entryCount++;

const unixMode = typeof entry.externalFileAttributes === 'number' ? (entry.externalFileAttributes >>> 16) : 0;
if ((unixMode & 0o170000) === 0o120000) {
reject(new Error(`zip entry is a symbolic link: ${entry.fileName}`));
return;
}

let rel: string;
try {
rel = safeUploadRelativePath(entry.fileName);
} catch (err) {
reject(err);
return;
}

zip.openReadStream(entry, async (err, readStream) => {
if (err || !readStream) {
reject(err ?? new Error(`failed to read entry: ${entry.fileName}`));
return;
}

const dest = path.join(stagedFolder, rel);
try {
await fs.promises.mkdir(path.dirname(dest), { recursive: true });

readStream.on('data', (chunk: Buffer) => {
totalBytes += chunk.length;
if (totalBytes > maxBytes) {
readStream.destroy(new Error('zip extracted size exceeds 50 MiB'));

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.

Clean up the staging directory when streaming extraction fails. This new path writes each entry into stagedFolder before the aggregate-size check can reject the archive; when readStream.destroy(...) makes pipeline reject, extractPluginZipToFolder rejects before finishUploadedPluginInstall is called, and stageUploadedPluginZip has no catch/finally cleanup. A highly compressed upload that expands past the limit therefore leaves up to the configured limit (and possibly the crossing chunk) under the system temp directory on every request, allowing repeated rejected uploads to consume disk. Wrap extraction and installation in stageUploadedPluginZip with the same try/catch cleanup used by stageUploadedPluginFolder (while preserving finishUploadedPluginInstall's cleanup), and add a ZIP-bomb/oversized-expanded fixture test that asserts rejection and removal of the staging directory.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

}
});

await pipeline(readStream, fs.createWriteStream(dest));
zip.readEntry();
} catch (pipeErr) {
reject(pipeErr);
}
});
});

zip.readEntry();
});
}

export function createPluginInstallationHelpers(deps: PluginInstallationHelpersDeps) {
Expand Down Expand Up @@ -163,8 +217,13 @@ export function createPluginInstallationHelpers(deps: PluginInstallationHelpersD

async function stageUploadedPluginZip(buffer: Buffer, source: string) {
const stagedFolder = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'od-plugin-zip-'));
await extractPluginZipToFolder(buffer, stagedFolder, deps.PLUGIN_UPLOAD_MAX_BYTES);
return finishUploadedPluginInstall(stagedFolder, source);
try {
await extractPluginZipToFolder(buffer, stagedFolder, deps.PLUGIN_UPLOAD_MAX_BYTES);
return await finishUploadedPluginInstall(stagedFolder, source);
} catch (err) {
await fs.promises.rm(stagedFolder, { recursive: true, force: true }).catch(() => undefined);
throw err;
}
}

async function stageUploadedPluginFolder(files: Array<{ buffer: Buffer; originalname: string }>, rawPaths: unknown) {
Expand Down
34 changes: 34 additions & 0 deletions apps/daemon/tests/plugin-installation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { describe, it, expect } from 'vitest';
import fs from 'node:fs';
import os from 'node:os';
import JSZip from 'jszip';
import { createPluginInstallationHelpers } from '../src/services/plugin-installation.js';

describe('plugin-installation zip extraction', () => {
it('cleans up staging directory on extraction failure', async () => {
// Generate a zip bomb: tiny compressed size, large decompressed size
const zip = new JSZip();
zip.file('bomb.txt', Buffer.alloc(10000, 'A'), { compression: 'DEFLATE' });
const buffer = await zip.generateAsync({
type: 'nodebuffer',
compression: 'DEFLATE',
compressionOptions: { level: 9 }
});

const deps = {
db: {} as any,
PLUGIN_UPLOAD_MAX_BYTES: buffer.length, // Buffer length is tiny (e.g. 200 bytes). Decompressed is 10000 bytes.
PLUGIN_REGISTRY_ROOTS: [],
PLUGIN_LOCKFILE_PATH: '',
installFromLocalFolder: async function* () { yield { kind: 'success' }; }
};
const helpers = createPluginInstallationHelpers(deps);

const initialTmpCount = fs.readdirSync(os.tmpdir()).filter(n => n.startsWith('od-plugin-zip-')).length;

await expect(helpers.stageUploadedPluginZip(buffer, 'test')).rejects.toThrow('zip extracted size exceeds 50 MiB');

const finalTmpCount = fs.readdirSync(os.tmpdir()).filter(n => n.startsWith('od-plugin-zip-')).length;
expect(finalTmpCount).toBe(initialTmpCount);
});
});
42 changes: 33 additions & 9 deletions apps/daemon/tests/runtimes/agent-args.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { existsSync, readFileSync } from 'node:fs';
import { existsSync, readFileSync, statSync } from 'node:fs';
import { test } from 'vitest';
import {
AGENT_DEFS, aider, antigravity, assert, claude, codex, copilot, cursorAgent, deepseek, devin, detectAgents, grokBuild, join, kilo, kimi, kiro, mkdtempSync, opencode, pi, qoder, qwen, rmSync, spawnEnvForAgent, tmpdir, vibe, writeFileSync, chmodSync,
Expand Down Expand Up @@ -534,18 +534,28 @@ test('qwen args check promptViaStdin, base args, model args and exclude `-` sent
// the daemon would render the resulting empty reply as a "successful"
// agent response — exactly the failure mode the auth/quota guard at
// server.ts ~12090 is meant to catch but for the wrong reason.
test('antigravity pipes prompt via stdin via -p flag (print mode)', () => {
test('antigravity delivers prompt via managed temp file instead of stdin', () => {
assert.equal(antigravity.bin, 'agy');
assert.equal(antigravity.streamFormat, 'plain');
assert.equal(antigravity.promptViaStdin, true);
assert.equal(antigravity.promptViaStdin, false);
assert.equal(antigravity.promptViaFile, true);

const args = antigravity.buildArgs('write hello world', [], [], {}, {});
assert.deepEqual(args, ['-p', '-']);
assert.throws(() => {
antigravity.buildArgs('hello', [], [], {}, {});
}, /requires runtimeContext\.promptFilePath/);

const expectedFileText = 'Read the system instructions, conversation history, and user request from the file /tmp/managed-prompt.md. Follow the instructions strictly and provide the final response to the user\'s latest request.';

const args = antigravity.buildArgs('write hello world', [], [], {}, {
promptFilePath: '/tmp/managed-prompt.md'
});
assert.deepEqual(args, ['--add-dir=/tmp', '-p', expectedFileText]);

const argsWithLog = antigravity.buildArgs('write hello world', [], [], {}, {
agentLogFilePath: '/tmp/od-agy-test.log',
promptFilePath: '/tmp/managed-prompt.md'
});
assert.deepEqual(argsWithLog, ['--log-file', '/tmp/od-agy-test.log', '-p', '-']);
assert.deepEqual(argsWithLog, ['--log-file', '/tmp/od-agy-test.log', '--add-dir=/tmp', '-p', expectedFileText]);

// No `--model` flag exists upstream, so buildArgs argv must stay the
// same regardless of which label the user picks.
Expand All @@ -558,9 +568,10 @@ test('antigravity pipes prompt via stdin via -p flag (print mode)', () => {
}, {
agentLogFilePath: '/tmp/od-agy-test.log',
antigravitySettingsPath: join(settingsDir, 'settings.json'),
promptFilePath: '/tmp/managed-prompt.md'
});
assert.equal(withModel.includes('--model'), false);
assert.deepEqual(withModel, ['--log-file', '/tmp/od-agy-test.log', '-p', '-']);
assert.deepEqual(withModel, ['--log-file', '/tmp/od-agy-test.log', '--add-dir=/tmp', '-p', expectedFileText]);
} finally {
rmSync(settingsDir, { recursive: true, force: true });
}
Expand All @@ -575,14 +586,16 @@ test('antigravity pipes prompt via stdin via -p flag (print mode)', () => {
// same regression.
const followUp = antigravity.buildArgs('next message', [], [], {}, {
hasPriorAssistantTurn: true,
promptFilePath: '/tmp/managed-prompt.md'
});
assert.deepEqual(followUp, ['-p', '-']);
assert.deepEqual(followUp, ['--add-dir=/tmp', '-p', expectedFileText]);
assert.equal(followUp.includes('-c'), false);

const firstTurn = antigravity.buildArgs('first', [], [], {}, {
hasPriorAssistantTurn: false,
promptFilePath: '/tmp/managed-prompt.md'
});
assert.deepEqual(firstTurn, ['-p', '-']);
assert.deepEqual(firstTurn, ['--add-dir=/tmp', '-p', expectedFileText]);
assert.equal(antigravity.resumesSessionViaCli, undefined);

assert.equal(antigravity.maxPromptArgBytes, undefined);
Expand Down Expand Up @@ -669,6 +682,17 @@ test('antigravity persists model selection to agy settings.json', () => {
writeAntigravityModelSelection('Gemini 3.5 Flash (Low)', corruptPath);
const recovered = JSON.parse(readFileSync(corruptPath, 'utf8'));
assert.equal(recovered.model, 'Gemini 3.5 Flash (Low)');
// 5. Atomic replacement must preserve the destination's existing mode
// so sensitive policies in settings.json do not become world-readable.
const modePath = join(dir, 'mode-settings.json');
writeFileSync(modePath, JSON.stringify({ model: 'old' }));
chmodSync(modePath, 0o600);
const expectedMode = statSync(modePath).mode & 0o777;

writeAntigravityModelSelection('Gemini 3.5 Pro', modePath);

const postStat = statSync(modePath);
assert.equal(postStat.mode & 0o777, expectedMode);
} finally {
rmSync(dir, { recursive: true, force: true });
}
Expand Down
4 changes: 2 additions & 2 deletions nix/pnpm-deps.nix
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
# 1. Temporarily set the consuming `hash = lib.fakeHash;`
# 2. Run the relevant nix build/flake check
# 3. Copy the expected hash printed by Nix into the matching field below
daemonHash = "sha256-kWSVza0qjeIvD5wVVYP/47fZhe/RBZ0DD6omHB4aIK8=";
webHash = "sha256-dHbQvec3qpC+6RgbXUBdI1PpilMdAafpknk46ah5E2U=";
daemonHash = "sha256-VVQRlqaBZByNG9Nsw77Fj0z0BlTH9fjW0wUa1hu9r6I=";
webHash = "sha256-+0ZbXyBH8EQx06EMg58iUT8wQ61DHJhueVo+/iFWdxc=";
}
Loading
Loading