mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
* refactor(mcp): centralize ParamDef→JSON Schema via shared paramDefToSchema Three duplicate inline mappers existed across the MCP surface: - src/mcp/tool-defs.ts (stdio MCP buildToolDefs) - src/commands/serve-http.ts:837 (live HTTP MCP tools/list) - src/core/minions/tools/brain-allowlist.ts:84 (subagent tool registry) Each had subtly different items propagation. The HTTP MCP variant dropped items entirely, leaving extract_facts.entity_hints broken for OAuth- authenticated remote agents even after a buildToolDefs-only patch. The subagent variant propagated one level of items but used the same shallow shape so nested arrays would silently drop. Extract a single recursive paramDefToSchema helper exported from src/mcp/tool-defs.ts and have all three mappers consume it. Closes the bug class at the architecture level instead of patching one site at a time. The helper copies type, description, enum, default, and recursively rebuilds items so array-of-arrays preserves inner shape. Key ordering (type, description, enum, default, items) matches the pre-v0.34 inline mappers so JSON.stringify output stays byte-stable for every existing operation that does not use nested arrays. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(schema): add items to extract_facts.entity_hints and handle-to-tweet candidates Two array fields shipped without the items property required by JSON Schema. Strict-mode validators (Gemini Pro structured outputs, OpenAI strict tool definitions) reject the entire schema when any type:'array' lacks items. Downstream agents on those providers couldn't use extract_facts or the x_handle_to_tweet resolver. extract_facts.entity_hints — declared items: { type: 'string' } matching the handler at src/core/operations.ts:2733 which already coerces the runtime value to string[]. handle_to_tweet outputSchema.candidates — full XTweetCandidate spec including required + additionalProperties: false. The XTweetCandidate TypeScript interface declares all five fields as required; without required in the JSON Schema, a validator would accept {} as a valid candidate. additionalProperties: false closes the OpenAI strict-mode contract. 19 community PRs (#1028 #999 #980 #979 #910 #904 #847 #832 #863 #862 #812 for entity_hints; #910 caught candidates) converged on these locations. This wave cherry-picks the deepest variant (#910 surfaced both bugs) and centralizes via the paramDefToSchema helper from the preceding commit so the live HTTP MCP tools/list path is also fixed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: DmitryBMsk (PR #910) * fix(git-remote): move --no-recurse-submodules after the subcommand verb Git CLI accepts two flag positions: git [global -c flags] <subcommand> [subcommand flags] [args] Global -c config flags belong before the verb. Subcommand-specific flags (like --no-recurse-submodules) belong after. Pre-v0.34 GIT_SSRF_FLAGS spliced both kinds before the verb, so cloneRepo invoked: git -c http.followRedirects=false ... --no-recurse-submodules clone URL DIR Real git rejects this with exit 129 ("unknown option: --no-recurse-submodules") because --no-recurse-submodules is a clone subcommand flag, not a global config flag. Every remote-source clone broke in production from v0.28 onward. The fake-git harness in test/git-remote.test.ts exits 0 regardless of argv shape, which is why CI never caught it. Split GIT_SSRF_FLAGS (3 -c config flags, spread BEFORE the verb) from GIT_SSRF_SUBCOMMAND_FLAGS (--no-recurse-submodules, spread AFTER the verb). cloneRepo and pullRepo both spread the new constant after their respective verbs. The constant names signal the position rule so future additions land in the right place. 7 community PRs converged on this location (#1023 #1020 #985 #963 #846 #842 — #800 doesn't exist). This wave cherry-picks the semantic- constant approach from #846's GIT_SSRF_SUBCOMMAND_FLAGS name (the clearest signal of the position rule). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(mcp+git+resolvers): structural array-items + subcommand-position guards Three new tests / test groups close the bug classes the wave fixes: test/mcp-tool-defs.test.ts — recursive structural guard walks every operation's inputSchema and fails with a property path if any type:'array' lacks items.type. Explicit fixture assertions for extract_facts.entity_hints.items.type and a synthetic nested-array ParamDef pinning items.items.type recursion. Without the explicit fixtures the legacyInlineMap byte-equality test is mirror-theater — mirroring both sides of the equality preserves the blind spot. test/git-remote.test.ts — split snapshot test into GIT_SSRF_FLAGS (3 global -c entries) and GIT_SSRF_SUBCOMMAND_FLAGS (--no-recurse-submodules). cloneRepo + pullRepo argv tests now assert the subcommand flag appears AFTER the verb index. Pre-v0.34 the pinned argv slice prefix included --no-recurse-submodules, which baked the bug into the test suite (codex catch). test/resolvers.test.ts — recursive walk over both inputSchema AND outputSchema for builtin resolvers (xHandleToTweetResolver, urlReachableResolver). Explicit imports rather than getDefaultRegistry(), which starts empty until commands/resolvers.ts runs — codex catch on a hollow-walk failure mode. Dedicated case pins candidates items shape including required + additionalProperties. Reference legacyInlineMap in mcp-tool-defs.test.ts mirrors the new recursive paramDefToSchema helper. No current op uses nested arrays so the byte-equality test stays green for every existing operation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(e2e): raise rerank timeouts for ZE live cold-start The first rerank call of a CI run hits ZeroEntropy's cold-start latency (observed ~5-6s on Tier 2 LLM Skills runners; subsequent calls < 500ms). Two timeouts fired simultaneously at ~5s: 1. bun:test's default 5000ms per-test timeout caused (fail). 2. gateway.rerank's DEFAULT_RERANK_TIMEOUT_MS = 5000 fired right after, reported as "Unhandled error between tests". The next rerank test (top_n=2) ran in 409ms because the API was already warm. Cold-start is the only issue. Pass explicit timeoutMs to each rerank() call and a longer per-test timeout (30s) on both ZE rerank tests. Production DEFAULT_RERANK_TIMEOUT_MS stays at 5s for the search hot path — these E2E tests bypass it locally without changing the default that protects user latency. Unrelated to the fix-wave in this PR (mcp-tool-defs + git-remote + resolver guards). Lands here to keep Tier 2 LLM Skills green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.35.2.0) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: sync for v0.35.2.0 Update CLAUDE.md Key files annotations for the v0.35.2.0 fix wave: - src/mcp/tool-defs.ts: document new exported recursive paramDefToSchema helper and the three-consumer centralization (stdio MCP, HTTP MCP tools/list, subagent registry). - src/core/minions/tools/brain-allowlist.ts: paramsToInputSchema now consumes the shared helper. - src/commands/serve-http.ts: tools/list handler now consumes the shared helper (closes the HTTP MCP items-dropped bug class). - src/core/git-remote.ts: new entry. Documents the GIT_SSRF_FLAGS (global config, pre-verb) vs GIT_SSRF_SUBCOMMAND_FLAGS (subcommand-scoped, post-verb) split, the 7-month silent regression, and the position-anchored regression guard in test/git-remote.test.ts. Regenerated llms-full.txt to match. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: rebump version to v0.35.3.0 Queue moved while this PR was open — v0.35.2.0 was claimed by master's v0.35.1.0 sibling work. Advancing one slot. No code changes; only: - VERSION + package.json: 0.35.2.0 → 0.35.3.0 - CHANGELOG.md: rewritten header + inline references - CLAUDE.md: rewritten 4 key-file annotations - llms-full.txt + llms.txt: regenerated to mirror CLAUDE.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
265 lines
8.0 KiB
TypeScript
265 lines
8.0 KiB
TypeScript
/**
|
|
* gbrain remote-source git helpers (v0.28).
|
|
*
|
|
* Single source of SSRF-defensive git invocations. parseRemoteUrl delegates
|
|
* to isInternalUrl from src/core/url-safety.ts (covers scheme allowlist,
|
|
* IPv6 loopback, IPv4-mapped IPv6, metadata hostnames, hex/octal bypass,
|
|
* and CGNAT 100.64/10).
|
|
*
|
|
* cloneRepo and pullRepo both spread GIT_SSRF_FLAGS so a future flag added
|
|
* to one path lands on both — single source of truth.
|
|
*
|
|
* Tailscale 100.64/10 trips the integrations.ts allowlist (CGNAT line in
|
|
* url-safety.ts isPrivateIpv4). For self-hosted internal git servers
|
|
* reachable only via Tailscale, set GBRAIN_ALLOW_PRIVATE_REMOTES=1; loud
|
|
* stderr warning at use site is the operator's signal.
|
|
*/
|
|
import { execFileSync } from 'child_process';
|
|
import { lstatSync, existsSync, readdirSync } from 'fs';
|
|
import { join } from 'path';
|
|
import { isInternalUrl } from './url-safety.ts';
|
|
|
|
/**
|
|
* Git CLI accepts two flag positions:
|
|
* git [global -c flags] <subcommand> [subcommand flags] [args]
|
|
*
|
|
* Global flags (the `-c key=value` config overrides) MUST come before the
|
|
* subcommand. Subcommand-specific flags (like `--no-recurse-submodules`)
|
|
* MUST come after the subcommand. Mixing the two positions makes git fail
|
|
* with `unknown option` (exit 129). Pre-v0.34 the single GIT_SSRF_FLAGS
|
|
* constant spliced both positions before the verb; real git rejected the
|
|
* subcommand flag but the test harness used a fake-git script that didn't
|
|
* validate, so every remote-source clone/pull broke silently in production.
|
|
*
|
|
* Split into two constants so the call-site spread is unambiguous and the
|
|
* type/name signal the position rule.
|
|
*/
|
|
|
|
/**
|
|
* Global git config flags. Spread BEFORE the subcommand verb.
|
|
* - http.followRedirects=false: closes DNS rebinding via redirect chains
|
|
* - protocol.file.allow=never: no local-file URLs (defense in depth)
|
|
* - protocol.ext.allow=never: no external helpers (`git-remote-foo`)
|
|
*/
|
|
export const GIT_SSRF_FLAGS = [
|
|
'-c', 'http.followRedirects=false',
|
|
'-c', 'protocol.file.allow=never',
|
|
'-c', 'protocol.ext.allow=never',
|
|
] as const;
|
|
|
|
/**
|
|
* Subcommand-level flags. Spread AFTER the subcommand verb (clone/pull).
|
|
* - --no-recurse-submodules: .gitmodules cannot become a second fetch surface
|
|
*/
|
|
export const GIT_SSRF_SUBCOMMAND_FLAGS = [
|
|
'--no-recurse-submodules',
|
|
] as const;
|
|
|
|
export type RemoteUrlErrorCode =
|
|
| 'invalid_url'
|
|
| 'unsupported_scheme'
|
|
| 'embedded_credentials'
|
|
| 'path_traversal'
|
|
| 'internal_target';
|
|
|
|
export class RemoteUrlError extends Error {
|
|
constructor(public code: RemoteUrlErrorCode, message: string) {
|
|
super(message);
|
|
this.name = 'RemoteUrlError';
|
|
}
|
|
}
|
|
|
|
export interface ParsedRemoteUrl {
|
|
url: string;
|
|
hostname: string;
|
|
}
|
|
|
|
/**
|
|
* Validate a remote git URL for clone safety. https:// only.
|
|
* Rejects: non-https schemes, embedded credentials, path traversal, and
|
|
* internal/private targets via isInternalUrl.
|
|
*
|
|
* GBRAIN_ALLOW_PRIVATE_REMOTES=1 lets the URL through with a stderr warning.
|
|
* Needed for self-hosted git over Tailscale (CGNAT 100.64/10) and similar.
|
|
*/
|
|
export function parseRemoteUrl(s: string): ParsedRemoteUrl {
|
|
if (!s || typeof s !== 'string') {
|
|
throw new RemoteUrlError('invalid_url', 'URL is empty or not a string');
|
|
}
|
|
let url: URL;
|
|
try {
|
|
url = new URL(s);
|
|
} catch {
|
|
throw new RemoteUrlError('invalid_url', `URL malformed: ${s}`);
|
|
}
|
|
if (url.protocol !== 'https:') {
|
|
throw new RemoteUrlError(
|
|
'unsupported_scheme',
|
|
`URL scheme not supported (https:// only): ${url.protocol}`,
|
|
);
|
|
}
|
|
if (url.username || url.password) {
|
|
throw new RemoteUrlError(
|
|
'embedded_credentials',
|
|
'URL must not contain embedded credentials (https://user:pass@host)',
|
|
);
|
|
}
|
|
if (s.includes('..')) {
|
|
throw new RemoteUrlError('path_traversal', 'URL must not contain path-traversal (..)');
|
|
}
|
|
if (isInternalUrl(s)) {
|
|
if (process.env.GBRAIN_ALLOW_PRIVATE_REMOTES === '1') {
|
|
console.error(
|
|
`[gbrain] WARN: GBRAIN_ALLOW_PRIVATE_REMOTES=1, accepting internal/private URL: ${url.hostname}`,
|
|
);
|
|
} else {
|
|
throw new RemoteUrlError(
|
|
'internal_target',
|
|
`URL targets internal/private network: ${url.hostname} ` +
|
|
`(set GBRAIN_ALLOW_PRIVATE_REMOTES=1 for self-hosted git over Tailscale or similar)`,
|
|
);
|
|
}
|
|
}
|
|
return { url: s, hostname: url.hostname };
|
|
}
|
|
|
|
export interface CloneOpts {
|
|
depth?: number; // default 1; 0 means full clone
|
|
branch?: string;
|
|
timeoutMs?: number; // default 600_000 (10 min)
|
|
}
|
|
|
|
export class GitOperationError extends Error {
|
|
constructor(
|
|
public op: 'clone' | 'pull' | 'remote_get_url',
|
|
message: string,
|
|
public cause?: unknown,
|
|
) {
|
|
super(message);
|
|
this.name = 'GitOperationError';
|
|
}
|
|
}
|
|
|
|
const GIT_ENV = {
|
|
// Confine to the gbrain SSRF model — no credential helpers, no SSH askpass,
|
|
// no GUI prompts. Inherit PATH so git itself is findable.
|
|
GIT_TERMINAL_PROMPT: '0',
|
|
GCM_INTERACTIVE: 'never',
|
|
GIT_ASKPASS: '/bin/false',
|
|
SSH_ASKPASS: '/bin/false',
|
|
} as const;
|
|
|
|
/**
|
|
* Clone a remote git repo with SSRF-defensive flags.
|
|
* - destDir must NOT exist or must be empty.
|
|
* - Default --depth=1 (no history); pass {depth: 0} for full clone.
|
|
* - Throws GitOperationError on failure; caller is responsible for cleanup.
|
|
*/
|
|
export function cloneRepo(url: string, destDir: string, opts: CloneOpts = {}): void {
|
|
if (existsSync(destDir)) {
|
|
let entries: string[];
|
|
try {
|
|
entries = readdirSync(destDir);
|
|
} catch (e) {
|
|
throw new GitOperationError(
|
|
'clone',
|
|
`Cannot inspect destination ${destDir}: ${(e as Error).message}`,
|
|
e,
|
|
);
|
|
}
|
|
if (entries.length > 0) {
|
|
throw new GitOperationError(
|
|
'clone',
|
|
`Destination ${destDir} exists and is not empty; refusing to clone`,
|
|
);
|
|
}
|
|
}
|
|
|
|
const args: string[] = [...GIT_SSRF_FLAGS, 'clone', ...GIT_SSRF_SUBCOMMAND_FLAGS];
|
|
if (opts.depth !== 0) {
|
|
args.push(`--depth=${opts.depth ?? 1}`);
|
|
}
|
|
if (opts.branch) {
|
|
args.push('--branch', opts.branch);
|
|
}
|
|
args.push(url, destDir);
|
|
|
|
try {
|
|
execFileSync('git', args, {
|
|
stdio: ['ignore', 'pipe', 'pipe'],
|
|
timeout: opts.timeoutMs ?? 600_000,
|
|
env: { ...process.env, ...GIT_ENV },
|
|
});
|
|
} catch (e) {
|
|
throw new GitOperationError(
|
|
'clone',
|
|
`git clone failed for ${url}: ${(e as Error).message}`,
|
|
e,
|
|
);
|
|
}
|
|
}
|
|
|
|
/** Pull a repo with --ff-only and the same SSRF-defensive flags as cloneRepo. */
|
|
export function pullRepo(repoPath: string, opts: { timeoutMs?: number } = {}): void {
|
|
const args: string[] = ['-C', repoPath, ...GIT_SSRF_FLAGS, 'pull', ...GIT_SSRF_SUBCOMMAND_FLAGS, '--ff-only'];
|
|
try {
|
|
execFileSync('git', args, {
|
|
stdio: ['ignore', 'pipe', 'pipe'],
|
|
timeout: opts.timeoutMs ?? 300_000,
|
|
env: { ...process.env, ...GIT_ENV },
|
|
});
|
|
} catch (e) {
|
|
throw new GitOperationError(
|
|
'pull',
|
|
`git pull failed in ${repoPath}: ${(e as Error).message}`,
|
|
e,
|
|
);
|
|
}
|
|
}
|
|
|
|
export type RepoState =
|
|
| 'healthy'
|
|
| 'missing'
|
|
| 'not-a-dir'
|
|
| 'no-git'
|
|
| 'url-drift'
|
|
| 'corrupted';
|
|
|
|
/**
|
|
* Classify the on-disk state of a clone. Used by performSync to decide
|
|
* whether to run pull (healthy), re-clone (missing/no-git/not-a-dir),
|
|
* refuse with corruption error (corrupted), or refuse with rebase-clone
|
|
* hint (url-drift).
|
|
*/
|
|
export function validateRepoState(
|
|
repoPath: string,
|
|
expectedRemoteUrl?: string,
|
|
): RepoState {
|
|
let stat;
|
|
try {
|
|
stat = lstatSync(repoPath);
|
|
} catch (e: any) {
|
|
if (e?.code === 'ENOENT') return 'missing';
|
|
return 'not-a-dir';
|
|
}
|
|
if (!stat.isDirectory()) return 'not-a-dir';
|
|
if (!existsSync(join(repoPath, '.git'))) return 'no-git';
|
|
|
|
let remoteUrl: string;
|
|
try {
|
|
const out = execFileSync('git', ['-C', repoPath, 'remote', 'get-url', 'origin'], {
|
|
stdio: ['ignore', 'pipe', 'pipe'],
|
|
timeout: 10_000,
|
|
env: { ...process.env, ...GIT_ENV },
|
|
});
|
|
remoteUrl = out.toString().trim();
|
|
} catch {
|
|
return 'corrupted';
|
|
}
|
|
|
|
if (expectedRemoteUrl !== undefined && remoteUrl !== expectedRemoteUrl) {
|
|
return 'url-drift';
|
|
}
|
|
return 'healthy';
|
|
}
|