4 commits

Author	SHA1	Message	Date
ChunxueLi	678a0e11c9	fix(server,web): honor the grep tool contract — real regex, fileFilter, caseSensitive (#3109) ... * fix(server,web): honor grep tool contract — real regex, fileFilter, caseSensitive (Patch 12) Background ========== The web chat's grep tool schema has always promised regex search with an optional path-substring fileFilter and caseSensitive control, but the GET /api/grep handler escapeRegExp()'d every pattern into a literal substring (a ReDoS hardening from fa36254e / #1317 that never re-synced the tool contract). Consequences, verified in production use against the sr-next backend repo (23k-file Java monorepo): - An agent sending the documented alternation form ("sign|Sign") got zero hits and concluded the sign/签署 interface did not exist. - The schema's own example pattern ("console\\.log") could never match: the escaped literal searched for a backslash in the source. - fileFilter / caseSensitive were read by nobody — pure schema fiction. - The web handler worked around the server with a (?=.*filter).*pattern lookahead splice that the same escaping also defeated. - Collateral: the impact tool's grep fallback (\b${escapeRegex(name)}\b) was silently dead code under literal semantics; it comes back to life with this fix (expected improvement, noted for reviewers). Fix === Server (gitnexus): - New src/server/grep-params.ts — pure query-param parser (no Express / native imports, per the #2790 helper-extraction convention): regex construction (default real regex; literal=1 restores the old escaped-substring semantics as an opt-out), lowercase path-substring fileFilter, caseSensitive flag, limit clamp [1,200] default 50, BadRequestError error paths (mapped to 400 by statusFromError). - /api/grep handler in api.ts becomes thin wiring: fileFilter path filtering before the (unchanged) traversal guard, a 5s wall-clock budget checked between files (partial results plus timedOut: true), read-only DB open unchanged. The regex is deliberately built WITHOUT the 'g' flag: the handler tests line-by-line and a stale lastIndex would skip matches (the old code had to reset it manually); 'm' is likewise omitted — each test sees one line, so ^/$ already anchor at string boundaries. Web (gitnexus-web): - tools.ts: drop the lookahead splice; description now tells the model the truth (real regex, alternation works, path-substring filter, case-insensitive default, result cap and time budget). - backend-client.ts: grep() takes GrepOptions {fileFilter,caseSensitive} and forwards them as query params. - useAppState.tsx: assembly site threads the options through. Security — residual ReDoS exposure (read this before deploying) =============================================================== The literal-only era was accidentally ReDoS-immune; this patch knowingly trades that immunity back for the promised contract. The bounds (200-char pattern cap, line-by-line matching, result cap, 5s budget) do NOT cover a single catastrophically backtracking regex.test(): it blocks the Node event loop synchronously, the budget (checked between files) cannot interrupt it, and the whole server is unresponsive for the duration (measured: (a+)+$ against a 35-char line exceeds 120 seconds). Accepted because local serve binds loopback by default and hosted deploys gate /api/grep behind the edge token; documented in SECURITY.md (new section) with the worker_threads+terminate / optional-re2 follow-up called out. literal=1 restores full immunity for untrusted callers. Compatibility audit =================== Repo-wide: /api/grep's only HTTP caller is backend-client.grep(); the MCP tool surface has no grep tool; eval/ uses shell grep, not this endpoint; the endpoint is undocumented (docs/llms.txt) with no known third-party consumers. Breaking surface ≈ zero. Pattern metacharacter semantics change for direct curl users ("array[0]" now needs escaping or literal=1). Tests ===== +20 cases in test/unit/grep-params.test.ts: alternation (the regression that burned the agent), the schema's own example, case flags, literal compat, CJK patterns, ^/$ line anchors, fileFilter normalization + array-form rejection, limit clamping, type-confusion guards, invalid regex, and a source-level handler wiring assertion (api-readonly-wiring style). Full unit suite: no new failures (22 pre-existing failures reproduced identically with this patch stashed — analyzer-identity dist fingerprint + lbug native-env classes). Upstream plan ============= Issue + PR to abhigyanpatwari/GitNexus; the PR description must front the ReDoS trade-off with the worker-isolation follow-up. Repro for the issue: curl ".../api/grep?pattern=TODO%7CFIXME" — 0 hits under literal semantics, both marker classes under regex semantics. Custom-patch ledger: CUSTOM_PATCHES.md Patch 12. * style: prettier * fix(web): surface grep timedOut so partial scans are not silent misses Propagate the server timeout flag through the backend client and chat tool, check the 5s budget between lines, and document the accepted regex-injection CodeQL finding next to new RegExp. * Address PR review feedback (#3109) - Cover empty and null fileFilter in the grep client test - Keep timedOut as a required boolean and reuse GrepOptions - Sample the grep deadline every 256 lines instead of every line Note: pre-existing failure in impact-tool.test.ts not addressed by this PR. * Address PR review feedback (#3109) Run /api/grep matching in a worker_threads worker so terminate() can cut a catastrophic regex.test without blocking the parent event loop. * Address PR review feedback (#3109) Reset lastIndex per line, restore the missing-pattern 400 message, and make the traversal test create a real outside file. * fix(web): align agent grep opts with GrepOptions Use GrepOptions so fileFilter null is accepted by the local GraphRAGBackend stub. * fix(server): silence CodeQL js/regex-injection on intentional grep regex Split literal vs regex construction and suppress with the correct rule id (js/regex-injection). Real regex remains the default contract; literal=1 still escapes. * Address PR review feedback (#3109) Clean up grep-scan temp dirs after each test, and exclude the intentional grep-params RegExp site from CodeQL so js/regex-injection does not re-file. --------- Co-authored-by: l.cx <l.cx@winning.com.cn> Co-authored-by: ChunxueLi <mecoloud@users.noreply.gitee.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>	2026-08-31 20:43:28 +01:00
FenjuFu	dc5c816a02	fix(serve): protect MCP route with optional bearer auth (#3100) ... * fix(serve): protect MCP route with optional bearer auth Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com> * fix(serve): clarify MCP auth proxy boundaries Document the Render token incompatibility, expose serve auth in CLI help, and replace source-order assertions with live middleware coverage. Note: full test suite has pre-existing worktree failures because generated parse-worker.js is absent; targeted auth and proxy suites pass. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(docs): preserve existing table formatting Keep the auth clarifications focused without reformatting unrelated Markdown tables. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(proxy): inject backend MCP credentials Replace the consumed edge credential with the configured protocol token only for MCP routes so proxied serve authentication remains composable. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>	2026-08-30 17:12:46 +01:00
Shifra Williams	f2717c6a7c	feat(render): add one-click deploy to render support (#2804)	2026-08-06 00:19:44 +00:00
Gergő Magyar	342721f06d	ci(security): add automated security and vulnerability scans (#1297) ... * ci(security): add CodeQL SAST workflow for JS/TS and Python CodeQL analyzes both languages on PR, main push, and weekly schedule. Findings upload to the Security tab as SARIF. Advisory only on introduction; promote to required check after baseline triage. Plan: docs/plans/2026-05-03-001-feat-automated-security-scans-plan.md (U1) * ci(security): add Dependency Review PR gate Blocks PRs introducing high+ severity dependency vulnerabilities. Posts inline summary comment on failure. Required-check candidate after one week of clean runs. Plan: docs/plans/2026-05-03-001-feat-automated-security-scans-plan.md (U2) * ci(security): add Gitleaks secret scanning PR runs scan the diff; main pushes scan full history. Defense-in-depth on top of GitHub native push protection (documented as a recommended Settings toggle in SECURITY.md). Plan: docs/plans/2026-05-03-001-feat-automated-security-scans-plan.md (U3) * ci(security): add OpenSSF Scorecard workflow Weekly + on main push. SARIF uploads to Security tab; public badge URL resolves after first scheduled run lands. Plan: docs/plans/2026-05-03-001-feat-automated-security-scans-plan.md (U4) * ci(security): add zizmor workflow lint Lints .github/workflows/** for known Actions security misconfigurations (unpinned actions, dangerous interpolation, missing permissions). Triggered only on PRs touching .github/**. Plan: docs/plans/2026-05-03-001-feat-automated-security-scans-plan.md (U5) * ci(security): add Trivy container image scanning Builds Dockerfile.cli and Dockerfile.web, then scans images for HIGH/CRITICAL CVEs. Findings record-only on Security tab; not PR-blocking. Weekly schedule + main push for freshness. Plan: docs/plans/2026-05-03-001-feat-automated-security-scans-plan.md (U6) * docs(security): add SECURITY.md policy and Scorecard badge Vulnerability disclosure policy points to GitHub Private Vulnerability Reporting. Documents in-CI scans landed in this branch and recommended admin actions for forks. Plan: docs/plans/2026-05-03-001-feat-automated-security-scans-plan.md (U7) * fix(review): apply autofix feedback - CodeQL paths-ignore: replace brace expansion (parser.{c,js}) with two explicit entries — CodeQL uses .gitignore-style globs that do NOT support brace expansion, so the original pattern matched no files. - Trivy: pin aquasecurity/trivy-action from @master to @0.28.0 — mutable refs are a supply-chain risk and are exactly what zizmor (added in this same plan) is meant to flag. ce-code-review run: /tmp/compound-engineering/ce-code-review/20260503-104259-279c3bc4/ * docs(review): record residual review findings ce-code-review autofix run flagged three downstream-resolver items that are not blockers but should land before promoting any of the new security workflows to required PR checks. Source: /tmp/compound-engineering/ce-code-review/20260503-104259-279c3bc4/ * fix(ci-security): address all zizmor + dependency-review violations Resolves all GitHub Advanced Security findings on PR #1297: - Add 'persist-credentials: false' to actions/checkout in 5 workflows (codeql, dependency-review, gitleaks, trivy, workflow-lint). Prevents the GITHUB_TOKEN from persisting in .git/config for downstream steps to read. Scorecard already had it. - Pin every net-new third-party Action to a commit SHA (was: major-tag refs flagged by zizmor as 'unpinned action reference'): github/codeql-action -> v3.35.3 (0daab03) actions/dependency-review-action -> v4.9.0 (2031cfc) gitleaks/gitleaks-action -> v2.3.9 (ff98106) ossf/scorecard-action -> v2.4.3 (4eaacf0) docker/build-push-action -> v6.19.2 (10e90e3) - Bump aquasecurity/trivy-action 0.28.0 -> 0.36.0 (ed142fd). Versions < 0.35.0 are flagged by GHSA-69fq-xp46-6x23 (briefly compromised supply chain). Caught by Dependency Review on the introducing PR. - Pin pipx-installed zizmor to 1.24.1 (was unpinned 'pipx install zizmor' resolving to latest at run time). Removes the now-stale residual-findings doc since every item it recorded is resolved on this branch. * fix(ci-security): clear remaining zizmor findings After landing the new security workflows, zizmor reported 5 high+ findings against pre-existing workflows (none introduced by this PR's new files, all introduced by zizmor's wider scope). Resolved per research at docs.zizmor.sh and PyO3/maturin issue #2425: Real fixes (cache-poisoning): - publish.yml + release-candidate.yml: add 'package-manager-cache: false' to actions/setup-node. setup-node v5+ enables caching by default when a packageManager field is present in package.json; explicit opt-out keeps release installs hermetic and clears the audit. Cost: ~30s slower per release run. Documented exemptions (dangerous-triggers, .github/zizmor.yml): - ci-report.yml: workflow_run is REQUIRED to post sticky comments on fork PRs (forks have read-only GITHUB_TOKEN on pull_request). - claude.yml: pull_request_target is required by claude-code-action to access secrets and post fork-PR review comments. PR checkouts pin fork HEAD SHA to mitigate TOCTOU. - pr-labeler.yml: pull_request_target on the autolabel job needs pull-requests:write. release-drafter runs with dry-run:true and reads config from the BASE ref only. Each exemption carries the documented mitigation in zizmor.yml. workflow-lint.yml now passes --config to both the SARIF and the gate invocations. Local 'zizmor --config .github/zizmor.yml --min-severity high .' reports: No findings to report. Good job!	2026-05-04 08:21:53 +01:00