sync: update from private repo (8ed98ab)
CI / build-and-test (push) Has been cancelled

This commit is contained in:
oss-sync
2026-06-09 00:46:41 +00:00
parent ef44e1a5d9
commit d6d8e83867
20 changed files with 452 additions and 12 deletions
+10 -2
View File
@@ -530,15 +530,23 @@ function buildStorage(out: Record<string, unknown>): StorageConfig {
// sub-form which binds directly to `tools.*`, so keeping that path
// alive is the cheapest fix for the v2 read path.
if (storage.taskUploadMaxSizeMb !== undefined) {
// Audit 2026-06-08 fix: a user-authored v2 storage block
// (existing.X !== undefined) is authoritative and must override the tools.*
// default — matching the worktreeDir precedence above. The prior
// `toolsObj.X === undefined` guard was a permanent no-op because defaults
// already seeded tools.{taskUploadMaxSizeMb,trashRetentionDays}, so a v2
// `storage.*` value was silently ignored (same default-precedence trap as
// PR #368→#369). The UI path writes tools.* directly (not storage.*), so
// there existing.X is undefined and tools.X is set → no overwrite.
const toolsObj = (out.tools ?? {}) as Record<string, unknown>;
if (toolsObj.taskUploadMaxSizeMb === undefined) {
if (existing.taskUploadMaxSizeMb !== undefined || toolsObj.taskUploadMaxSizeMb === undefined) {
toolsObj.taskUploadMaxSizeMb = storage.taskUploadMaxSizeMb;
out.tools = toolsObj;
}
}
if (storage.trashRetentionDays !== undefined) {
const toolsObj = (out.tools ?? {}) as Record<string, unknown>;
if (toolsObj.trashRetentionDays === undefined) {
if (existing.trashRetentionDays !== undefined || toolsObj.trashRetentionDays === undefined) {
toolsObj.trashRetentionDays = storage.trashRetentionDays;
out.tools = toolsObj;
}
+80
View File
@@ -0,0 +1,80 @@
import { describe, it, expect, afterEach } from 'vitest';
import { mkdtempSync, rmSync, writeFileSync } from 'fs';
import { join } from 'path';
import { tmpdir } from 'os';
import { loadConfig } from './config.js';
// Audit 2026-06-08 (config-consistency, P1×2). Both encode the documented,
// expected behavior that the current normalizer/env-override plumbing violates.
function withTempConfig(yaml: string, fn: (path: string) => void): void {
const dir = mkdtempSync(join(tmpdir(), 'maestro-cfg-audit-'));
try {
const p = join(dir, 'config.yaml');
writeFileSync(p, yaml);
fn(p);
} finally {
rmSync(dir, { recursive: true, force: true });
}
}
describe('config audit regression (2026-06-08)', () => {
const envSnapshot: Record<string, string | undefined> = {};
afterEach(() => {
for (const [k, v] of Object.entries(envSnapshot)) {
if (v === undefined) delete process.env[k];
else process.env[k] = v;
}
});
// P1: defaults seed tools.trashRetentionDays=30 BEFORE the normalizer runs, so
// the reverse-backfill (`if (tools.X === undefined)`) is a no-op and a user's
// v2 `storage.trash_retention_days` is silently ignored by the actual consumer
// (server.ts reads config.tools.trashRetentionDays). Same default-precedence
// trap as PR #368→#369.
it('v2 storage.trash_retention_days actually takes effect on the consumed value', () => {
withTempConfig('config_version: 2\nstorage:\n trash_retention_days: 60\n', (p) => {
const config = loadConfig(p);
expect(config.tools.trashRetentionDays).toBe(60);
});
});
// P1: OLLAMA_BASE_URL / OLLAMA_MODEL are applied to `provider.*` BEFORE
// normalize, but for a v2 config with explicit llm.workers the backfill keeps
// the YAML workers and never reads provider — so the documented env override
// silently has no effect on the endpoint that is actually called.
// The runtime executes config.provider.workers (worker.ts:459), so the env
// override must reach THAT array. config.llm.workers is the v2 block that
// config-manager persists, so it must stay the explicit YAML value — otherwise
// saving any setting while OLLAMA_BASE_URL is set would bake the env value into
// the YAML and permanently clobber the user's endpoint (codex P2).
it('OLLAMA_BASE_URL overrides the executed (provider) worker but not the persisted llm block', () => {
envSnapshot['OLLAMA_BASE_URL'] = process.env['OLLAMA_BASE_URL'];
process.env['OLLAMA_BASE_URL'] = 'http://override-host:9999/v1';
withTempConfig(
'config_version: 2\nllm:\n workers:\n - name: w1\n endpoint: http://yaml-host:11434/v1\n model: m1\n',
(p) => {
const config = loadConfig(p);
expect(config.provider.workers[0]?.endpoint).toBe('http://override-host:9999/v1'); // runtime
expect(config.llm.workers[0]?.endpoint).toBe('http://yaml-host:11434/v1'); // persisted, untouched
},
);
});
// The override is scoped to the FIRST/default worker only — a multi-worker
// pool (gateway/title/reflection) must NOT have every endpoint clobbered when
// a Docker .env sets OLLAMA_BASE_URL (codex P1).
it('OLLAMA_BASE_URL leaves additional workers untouched on the runtime array', () => {
envSnapshot['OLLAMA_BASE_URL'] = process.env['OLLAMA_BASE_URL'];
process.env['OLLAMA_BASE_URL'] = 'http://override-host:9999/v1';
withTempConfig(
'config_version: 2\nllm:\n workers:\n' +
' - name: w1\n endpoint: http://yaml-host:11434/v1\n model: m1\n' +
' - name: w2\n endpoint: http://gateway-host:4000/v1\n model: m2\n',
(p) => {
const config = loadConfig(p);
expect(config.provider.workers[0]?.endpoint).toBe('http://override-host:9999/v1');
expect(config.provider.workers[1]?.endpoint).toBe('http://gateway-host:4000/v1');
},
);
});
});
+39
View File
@@ -251,6 +251,17 @@ export interface SafetyConfig {
* - 'off': 旧来の素 exec(後方互換・デバッグ用、非推奨)
*/
bashSandbox?: 'auto' | 'always' | 'off';
/**
* When true, the bwrap sandbox keeps host network access (the `--unshare-net`
* isolation is dropped) so sandboxed Bash / python / npm can reach the network
* (pip install, npm install, curl, etc.). SECURITY: this weakens isolation —
* sandboxed commands can then exfiltrate data and reach the internal network
* (SSRF / metadata endpoints). The other --unshare-* isolations remain.
* Only meaningful when the sandbox is active (bashSandbox auto/always with bwrap
* present); in 'off' / hardened-whitelist fallback the network is already open.
* Default: false (network isolated).
*/
bashAllowNetwork?: boolean;
}
export interface SkillsConfig {
@@ -791,6 +802,34 @@ export function loadConfig(configPath: string = 'config.yaml'): AppConfig {
}
}
// Audit 2026-06-08 fix: OLLAMA_BASE_URL / OLLAMA_MODEL must also override the
// FIRST (default) worker's endpoint/model post-normalize. The pre-normalize
// provider.baseUrl override (above) is lost when a v2 config carries an
// explicit `llm.workers` block, because normalizeConfig re-derives the worker
// arrays from it and never consults provider.baseUrl, so a fresh v2 install
// silently ignored the env override.
//
// Apply ONLY to provider.workers[0] — the array worker.ts actually executes —
// and deliberately NOT to llm.workers. Rationale:
// - Scope to workers[0]: config-manager treats only llm.workers[0] as the
// env-overridden worker; overwriting every worker would clobber a
// multi-worker pool (gateway/title/reflection) the moment a Docker .env
// sets OLLAMA_BASE_URL.
// - provider only (not llm): config-manager persists the v2 `llm` block and
// STRIPS the legacy `provider` block on save (V2_STRIPPED_TOP_LEVEL_KEYS).
// Mutating llm.workers here would bake the transient env value into the
// saved YAML on the next Settings save, permanently clobbering the user's
// explicit endpoint (codex P2). Mutating provider keeps the override
// runtime-only — the UI still flags the env override via overriddenByEnv.
if (process.env['OLLAMA_BASE_URL']) {
const ep = process.env['OLLAMA_BASE_URL'];
if (config.provider?.workers?.[0]) config.provider.workers[0].endpoint = ep;
}
if (process.env['OLLAMA_MODEL']) {
const model = process.env['OLLAMA_MODEL'];
if (config.provider?.workers?.[0]) config.provider.workers[0].model = model;
}
const errors = validateConfig(config);
for (const err of errors) {
logger.warn(`Config validation: ${err}`);
+27
View File
@@ -202,3 +202,30 @@ describe('MCP table migrations', () => {
expect(row.owner_id).toBeNull();
});
});
// Audit 2026-06-08 (P1, db schema dual-path): user_gitea_orgs is created ONLY
// in Repository.initSchema() — not in schema.sql nor migrate.ts. The task-list
// display SELECT (repository.ts:33,39) references it in a correlated subquery,
// so any DB built via the migration path alone crashes on the most common
// query. This pins the invariant from the rule:
// tables/columns must live in BOTH the fresh path and the migration path.
describe('audit regression: migration path provides display-join tables (2026-06-08)', () => {
let db: Database.Database;
beforeEach(() => {
db = new Database(':memory:');
db.pragma('foreign_keys = ON');
seedMinimalSchema(db);
});
afterEach(() => {
db.close();
});
it('runMigrations creates user_gitea_orgs (referenced by the task-list display SELECT)', () => {
runMigrations(db);
expect(() =>
db.prepare('SELECT MIN(org_name) AS n FROM user_gitea_orgs WHERE org_id = ?').get('x'),
).not.toThrow();
});
});
+16
View File
@@ -24,6 +24,22 @@ export function runMigrations(db: Database.Database): void {
db.exec("ALTER TABLE local_tasks ADD COLUMN owner_id TEXT REFERENCES users(id)");
}
// Audit 2026-06-08 fix: user_gitea_orgs was created ONLY in
// Repository.initSchema(), so a DB built via the migration path alone lacked
// it and the task-list display SELECT (correlated subquery on org_name in
// repository.ts) crashed with "no such table". Create it here too so the
// fresh path (schema.sql) and the migration path stay in sync.
db.exec(`
CREATE TABLE IF NOT EXISTS user_gitea_orgs (
user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE,
org_id TEXT NOT NULL,
org_name TEXT NOT NULL,
fetched_at TEXT NOT NULL DEFAULT (datetime('now')),
PRIMARY KEY (user_id, org_id)
);
CREATE INDEX IF NOT EXISTS idx_user_gitea_orgs_org_id ON user_gitea_orgs(org_id);
`);
// Add context tracking columns to jobs (if not exists)
// re-fetch after the owner_id ALTER above to reflect the updated schema
const jobsColsAfter = db.prepare("PRAGMA table_info('jobs')").all() as Array<{ name: string }>;
+12
View File
@@ -590,3 +590,15 @@ CREATE TABLE IF NOT EXISTS user_notification_prefs (
v1_migrated INTEGER NOT NULL DEFAULT 0,
updated_at TEXT NOT NULL DEFAULT (datetime('now'))
);
-- Audit 2026-06-08 fix: user_gitea_orgs (per-user Gitea org cache) was created
-- only in Repository.initSchema(); mirrored here (fresh path) and in migrate.ts
-- (upgrade path) so the task-list display SELECT never hits "no such table".
CREATE TABLE IF NOT EXISTS user_gitea_orgs (
user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE,
org_id TEXT NOT NULL,
org_name TEXT NOT NULL,
fetched_at TEXT NOT NULL DEFAULT (datetime('now')),
PRIMARY KEY (user_id, org_id)
);
CREATE INDEX IF NOT EXISTS idx_user_gitea_orgs_org_id ON user_gitea_orgs(org_id);
+3 -2
View File
@@ -291,7 +291,7 @@ export async function runPiece(
spawnSubTask?: (params: { title: string; instruction: string; piece?: string }) => Promise<{ jobId: string; subtaskIndex: number; workspacePath: string }>;
cancelCheck?: () => boolean;
abortController?: AbortController;
safetyConfig?: { maxIterations?: number; maxRevisits?: number; bashUnrestricted?: boolean; bashSandbox?: 'auto' | 'always' | 'off' };
safetyConfig?: { maxIterations?: number; maxRevisits?: number; bashUnrestricted?: boolean; bashSandbox?: 'auto' | 'always' | 'off'; bashAllowNetwork?: boolean };
searchFilter?: SearchFilterConfig;
customPiecesDir?: string | string[];
contextManager?: ContextManager;
@@ -690,7 +690,7 @@ function prepareMovementContext(
/** Role for the job owner. */
notesUserRole?: 'admin' | 'user';
/** Safety config — threaded so prepareMovementContext can propagate bashUnrestricted. */
safetyConfig?: { bashUnrestricted?: boolean; bashSandbox?: 'auto' | 'always' | 'off' };
safetyConfig?: { bashUnrestricted?: boolean; bashSandbox?: 'auto' | 'always' | 'off'; bashAllowNetwork?: boolean };
skillCatalog?: import('./skills.js').SkillCatalog;
/** Per-task option: when true, MCP tools are not loaded/dispatched. */
mcpDisabled?: boolean;
@@ -716,6 +716,7 @@ function prepareMovementContext(
allowedCommands: movementDef.allowed_commands,
bashUnrestricted: options?.safetyConfig?.bashUnrestricted,
bashSandbox: options?.safetyConfig?.bashSandbox,
bashAllowNetwork: options?.safetyConfig?.bashAllowNetwork,
skillCatalog: options?.skillCatalog,
allowedSshConnections: movementDef.allowed_ssh_connections,
pieceName: piece.name,
+2
View File
@@ -62,6 +62,7 @@ export interface ToolContext {
allowedCommands?: string[]; // Bash ツールで許可するコマンド名一覧 (省略時は DEFAULT_ALLOWED_COMMANDS)
bashUnrestricted?: boolean; // true: skip the command whitelist (bwrap/exec is chosen by bashSandbox, not this)
bashSandbox?: 'auto' | 'always' | 'off'; // サンドボックス機構の選択 (config.safety.bashSandbox 由来)
bashAllowNetwork?: boolean; // true: bwrap サンドボックスで host network を許可 (--unshare-net を外す。config.safety.bashAllowNetwork 由来)
skillCatalog?: import('../skills.js').SkillCatalog;
toolsConfig?: ToolsConfig;
searchFilter?: SearchFilterConfig; // AppConfig.searchFilter (トップレベル)
@@ -1007,6 +1008,7 @@ async function executeBash(input: Record<string, unknown>, ctx: ToolContext): Pr
}
const result: SandboxedBashResult = await executeSandboxedBash(
command, ctx.workspacePath, timeoutSec, BASH_MAX_BUFFER_BYTES, ctx.abortSignal, skillBinds,
ctx.bashAllowNetwork === true,
);
const out = result.isError ? result.output : capOutput(result.output, 'stdout');
logBashHistory(ctx.workspacePath, command, result.isError, Date.now() - startedAt, {
+21
View File
@@ -245,6 +245,27 @@ describe('buildBwrapArgs sandboxing', () => {
expect(i).toBeGreaterThan(-1);
expect(args).toContain('HOME');
});
it('unshares network by default (allowNetwork omitted)', () => {
const args = buildBwrapArgs('echo hi', '/work/ws');
expect(args).toContain('--unshare-net');
});
it('keeps host network when allowNetwork=true (drops only --unshare-net)', () => {
const args = buildBwrapArgs('echo hi', '/work/ws', undefined, process.env, true);
expect(args).not.toContain('--unshare-net');
// all other isolations remain
expect(args).toContain('--unshare-user');
expect(args).toContain('--unshare-ipc');
expect(args).toContain('--unshare-pid');
expect(args).toContain('--unshare-uts');
expect(args).toContain('--unshare-cgroup');
});
it('unshares network when allowNetwork=false (explicit)', () => {
const args = buildBwrapArgs('echo hi', '/work/ws', undefined, process.env, false);
expect(args).toContain('--unshare-net');
});
});
describe('checkBwrapAvailable', () => {
+10 -2
View File
@@ -36,6 +36,7 @@ export function buildBwrapArgs(
workspacePath: string,
extraReadOnlyBinds?: ExtraReadOnlyBind[],
parentEnv: NodeJS.ProcessEnv = process.env,
allowNetwork: boolean = false,
): string[] {
const args: string[] = [];
@@ -70,7 +71,13 @@ export function buildBwrapArgs(
args.push('--chdir', workspacePath);
args.push('--die-with-parent');
args.push('--unshare-user', '--unshare-ipc', '--unshare-pid', '--unshare-uts', '--unshare-cgroup', '--unshare-net');
// Network isolation is the one --unshare-* that's optional. When
// allowNetwork is true (config.safety.bashAllowNetwork) the sandbox keeps host
// network so pip/npm/curl work; all other namespaces stay unshared.
args.push('--unshare-user', '--unshare-ipc', '--unshare-pid', '--unshare-uts', '--unshare-cgroup');
if (!allowNetwork) {
args.push('--unshare-net');
}
args.push('--clearenv');
const sandboxEnv = buildSandboxEnv(parentEnv, workspacePath);
@@ -124,8 +131,9 @@ export async function executeSandboxedBash(
maxBuffer: number,
abortSignal?: AbortSignal,
extraReadOnlyBinds?: ExtraReadOnlyBind[],
allowNetwork: boolean = false,
): Promise<SandboxedBashResult> {
const args = buildBwrapArgs(command, workspacePath, extraReadOnlyBinds);
const args = buildBwrapArgs(command, workspacePath, extraReadOnlyBinds, process.env, allowNetwork);
if (abortSignal?.aborted) {
return { output: 'Cancelled before sandbox bash launch', isError: true };
+30
View File
@@ -5,6 +5,8 @@ import { describe, it, expect } from 'vitest';
import { Registry } from 'prom-client';
import { normalizeToolNameForMetric, BUILTIN_TOOL_NAMES } from './tool-name-allowlist.js';
import { createWorkerMetrics } from './worker-metrics.js';
import { TOOL_DEFS as SLIDE_DEFS } from '../engine/tools/slide.js';
import { TOOL_DEFS as MSLEARN_DEFS } from '../engine/tools/ms-learn.js';
describe('normalizeToolNameForMetric', () => {
it('passes built-in tool names through verbatim', () => {
@@ -59,3 +61,31 @@ describe('normalizeToolNameForMetric', () => {
expect(dump).not.toMatch(/tool_name="mcp__/);
});
});
// Audit 2026-06-08 (P1, metrics): the allowlist carried GHOST names
// (CreateSlide, MsLearn{Fetch,Read,Search,Summarize}) that no tool emits, while
// the REAL slide (SetTheme/AddSlide/BuildPptx/ResetSlides) and ms-learn
// (Search/Fetch/SearchCache/RefreshMicrosoftLearn[Cache]) tools were missing —
// so every real call collapsed to 'unknown' in metrics. Pin the allowlist to
// the actual TOOL_DEFS so it can't drift again.
describe('metrics allowlist ↔ real tool definitions (audit regression)', () => {
const realNames = [...Object.keys(SLIDE_DEFS), ...Object.keys(MSLEARN_DEFS)];
it('every real slide/ms-learn tool normalizes to itself (not "unknown")', () => {
for (const name of realNames) {
expect(normalizeToolNameForMetric(name)).toBe(name);
}
});
it('every real slide/ms-learn tool is present in BUILTIN_TOOL_NAMES', () => {
for (const name of realNames) {
expect(BUILTIN_TOOL_NAMES.has(name)).toBe(true);
}
});
it('carries no ghost names that no real tool emits', () => {
for (const ghost of ['CreateSlide', 'MsLearnFetch', 'MsLearnRead', 'MsLearnSearch', 'MsLearnSummarize']) {
expect(BUILTIN_TOOL_NAMES.has(ghost)).toBe(false);
}
});
});
+9 -6
View File
@@ -50,14 +50,14 @@ const BUILTIN_TOOL_NAMES_LIST: ReadonlyArray<string> = [
// review.ts
'BatchReviewTextWithLLM', 'MergeReviewedResults',
// browser.ts
'BrowseWeb',
'BrowseWeb', 'BrowseWithSession', 'InteractiveBrowse',
// knowledge.ts
'IngestDocument', 'IngestStatus', 'ListDocuments', 'ListNamespaces',
'SearchKnowledge',
// orchestration.ts
'SpawnSubTask',
// x.ts
'XPostDetail', 'XSearch', 'XTimeline', 'XUserPosts',
'XFetchCardMedia', 'XPostDetail', 'XSearch', 'XTimeline', 'XUserPosts',
// maps.ts
'GetDirections', 'ReverseGeocode', 'SearchPlaces',
// youtube.ts
@@ -75,7 +75,8 @@ const BUILTIN_TOOL_NAMES_LIST: ReadonlyArray<string> = [
// mission.ts
'MissionUpdate',
// user-folder.ts
'ListUserAssets', 'ReadUserTemplate', 'RenderUserTemplate', 'RunUserScript',
'ListUserAssets', 'ReadUserMemory', 'ReadUserTemplate', 'RenderUserTemplate',
'RunUserScript', 'UpdateUserMemory', 'WriteUserScript', 'WriteUserTemplate',
// brainstorm.ts
'Brainstorm',
// app-docs.ts
@@ -88,10 +89,12 @@ const BUILTIN_TOOL_NAMES_LIST: ReadonlyArray<string> = [
'ReadNote', 'SearchNotes', 'WriteNote',
// dashboard.ts
'UpdateDashboardWidget',
// skills.ts
'InstallSkill', 'ListSkills', 'ReadSkill',
// ms-learn.ts (Microsoft Learn search)
'MsLearnFetch', 'MsLearnRead', 'MsLearnSearch', 'MsLearnSummarize',
// slide.ts
'CreateSlide',
'FetchMicrosoftLearn', 'RefreshMicrosoftLearnCache', 'SearchMicrosoftLearn', 'SearchMicrosoftLearnCache',
// slide.ts (pptxgenjs)
'AddSlide', 'BuildPptx', 'ResetSlides', 'SetTheme',
];
export const BUILTIN_TOOL_NAMES: ReadonlySet<string> = new Set<string>([