fix(security+quality): full-codebase sweep - 10 High/Critical security fixes, 15 quality fixes, 14 new tests#270
Open
thenotoriousllama wants to merge 35 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds the Cursor extension and its loaders, moves Cursor bundle outputs under ChangesCursor extension, hardening, and documentation sweep
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+102
to
+107
| headers: { | ||
| Authorization: `Bearer ${creds.token}`, | ||
| "Content-Type": "application/json", | ||
| "X-Activeloop-Org-Id": creds.orgId, | ||
| "X-Deeplake-Client": "hivemind", | ||
| }, |
| "X-Deeplake-Client": "hivemind", | ||
| }, | ||
| signal: AbortSignal.timeout(timeoutMs), | ||
| body: JSON.stringify({ query: sql }), |
Comment on lines
+133
to
+136
| headers: { | ||
| Authorization: `Bearer ${creds.token}`, | ||
| ...(creds.orgId ? { "X-Activeloop-Org-Id": creds.orgId } : {}), | ||
| }, |
Comment on lines
+32
to
+35
| headers: { | ||
| Authorization: `Bearer ${creds.token}`, | ||
| "Content-Type": "application/json", | ||
| }, |
|
|
||
| const { mkdirSync, writeFileSync } = await import("node:fs"); | ||
| mkdirSync(deeplakeConfigDir(), { recursive: true, mode: 0o700 }); | ||
| writeFileSync(credentialsPath(), JSON.stringify(creds, null, 2), { mode: 0o600 }); |
Add the full extension source under harnesses/cursor/extension/:
- src/ TypeScript source (auth, bridge, graph, health,
statusbar, utils, webview with dashboard panel)
- scripts/ Dashboard data loaders (deeplake.mjs, load-*.mjs)
- media/ icon.svg, d3.v7.min.js
- package.json + package-lock.json (independent build)
- tsconfig.json + webpack.config.js
- .vscodeignore + README.md
.vsix build artifacts and extension dist/ are gitignored.
The extension has its own webpack build and does not couple to
the root npm run build. Root tsc, esbuild, and test suite are
unaffected (4487/4487 passing).
QA M1 (DashboardViewProvider disposable leak): - Dispose previous controller before re-resolving the view - Route controller-owned disposables through context.subscriptions - Add webviewView.onDidDispose handler to release FileSystemWatcher and editor listeners immediately on panel close QA M2 (README overstates VSIX bundle provisioning): - Clarify that a standalone VSIX does not ship the hook bundle; hivemind cursor install or Wire Hooks is required for non-monorepo installs Security M1 (esc() missing quote escaping): - Add " and ' replacements to the webview HTML escaper; already contained by CSP nonce but defence-in-depth Security M2 (missing -- before user-controlled CLI args): - Prepend -- to args in runHivemindCliAsync so positional values cannot be misinterpreted as option flags by the hivemind CLI Security M3 (apiUrl not validated in mjs loaders): - Add sanitizeApiUrl() to scripts/lib/deeplake.mjs with an ALLOWED_API_ORIGINS allowlist; apply at loadCreds() so all callers receive a safe value; simplify load-dashboard.mjs fallback
Audit of src/cli/ + scripts/. No Critical/High findings. Hardened pack-check.mjs publish secret gate (M1) to block private-key material and credentials.json. Report at library/qa/repo-sweep/c2/security.md.
Audit of src/cli/ (15 files) + scripts/. tsc --noEmit clean. One Medium type-safety finding fixed (M1): three catch (e: any) handlers in update.ts + install-claude.ts switched to catch (e: unknown) with instanceof Error narrowing, matching the repo convention and the C1 pass. No Critical findings. Report at library/qa/repo-sweep/c2/quality.md.
Harden src/hooks against credential exposure and Deeplake SQL injection. - Write the wiki-worker config.json (carries the Activeloop token) with mode 0600 inside a 0700 tmp dir, instead of world-readable defaults in the predictable /tmp path (all four harness spawn helpers). - Wrap every config-driven table identifier (HIVEMIND_TABLE / HIVEMIND_SESSIONS_TABLE) in sqlIdent across the hook query/insert sites, matching the existing convention in DeeplakeApi, session-queue, and context-renderer. Closes a defense-in-depth SQL-injection gap on the parameterless Deeplake SQL API. - Sanitize the harness-supplied session id before using it as a query-cache path segment (path-traversal guard), mirroring writeReadCacheFile. Report: library/qa/repo-sweep/c3/security.md
Close a Critical SQL-injection gap: five Deep Lake SQL statements interpolated a config-driven table identifier without sqlIdent(), unlike the rest of the pipeline. Wrap each in sqlIdent (pull.ts, skill-invocations.ts x2, skillify-worker.ts x2). Adds the C4 security audit report.
Quality pass over src/hooks/ (47 files). Two Warning-level correctness divergences found and fixed; tsc --noEmit clean. - Port the codex wiki-worker exec-failure upload guard to the claude, cursor, hermes, and pi forks. On a resumed session a failed agent-CLI run was re-uploading the unchanged summary and advancing the JSONL offset via finalizeSummary, marking unsummarized events as done. - Release the summary lock on spawn failure in cursor/ and hermes/ session-end, matching session-end.ts and codex/stop.ts (was leaking the lock until the 10-minute stale reclaim). Report: library/qa/repo-sweep/c3/quality.md
Close a path-traversal + prompt-injection sink in the graph VFS: loadSnapshotOrError built the snapshot path from an unvalidated commit_sha/snapshot_sha256 read out of .last-build.json. readLastBuild validates only the type (string), not the hex shape, so a tampered file could escape the snapshots/ dir and have an arbitrary *.json file read and rendered into agent context. Add the same hex guard that session-context.ts and diff.ts already apply to the identical field. Adds the C5 security audit report under library/qa/repo-sweep/c5/.
Audit of src/skillify (38 files) against the C4 quality brief. tsc clean and 228 in-scope unit tests pass. Fixed one Warning: skill-writer.ts wrote SKILL.md with a plain writeFileSync while its siblings (manifest.ts, state.ts) write atomically. A crash mid-write in writeNewSkill leaves a torn file the function can never self-heal (it always throws "already exists"), so the skill is lost until manual removal. Added an atomicWriteFile (tmp+rename) helper and routed writeNewSkill + mergeSkill through it; final content is byte- identical so all skill-writer tests pass unchanged. Adds the C4 quality audit report.
Re-verification of the Cursor extension surface (PR #4 audit follow-up). Confirmed all three PR #4 fixes survived the rebase/merge: - esc() escapes " and ' in dashboard-shell.ts - runHivemindCliAsync prepends ["--", ...args] in data-bridge.ts - sanitizeApiUrl() present and called from loadCreds() in deeplake.mjs Fixed one new High finding (C8-SEC-01): sanitizeApiUrl() in scripts/lib/deeplake.mjs used a startsWith prefix check for the API host allowlist, which accepts hostile values like "https://api.deeplake.ai@evil.com" and "https://api.deeplake.ai.evil.com" and would redirect the Activeloop bearer token to an attacker host. Switched to parsed-origin equality, matching the TypeScript sibling src/auth/safe-url.ts.
Clean pass: audited all 110 .ts files under tests/claude-code/ for hardcoded secrets, unsafe shell helpers, SQL injection, predictable temp paths, and tainted process spawns. Zero Critical/High findings; no source changes required. Report-only commit.
Critical: the PR #4 security pass's global "--" separator broke every CLI-backed dashboard action. The hivemind CLI reads its subcommand from argv[0] and has no leading-"--" handling, so "hivemind -- <cmd>" exited with "Unknown command: --" (embeddings toggle, graph build, goal/rules CRUD, skill promote, workspace/org switch, activation auto-pull). This is the root cause of the "--scope after --" symptom flagged by security. spawn() is shell-less so the "--" gave no injection protection. Fix: drop the prepend and pass args directly. Warning: pushSettings called syncSkillsToCursor() (symlink + manifest mutation) ignoring HIVEMIND_AUTOPULL_DISABLED, which the poller and the syncSkills handler both honor. Gate it consistently. Confirmed: QA M1 DashboardViewProvider disposable fix and sanitizeApiUrl origin-equality hardening present; npx tsc --noEmit passes clean.
Clean audit of all 69 .ts files under tests/shared/. No Critical/High findings: fake placeholder tokens only, all writes under unique mkdtemp dirs with rmSync cleanup, literal-only shell-outs, mocked Deep Lake SQL. No code changes required.
Clean test-quality pass over all 110 .ts files under tests/claude-code/. No vacuous assertions, leftover focus, debugging artifacts, or isolation leaks found. Added six targeted tests covering C1-C8 sweep security fixes that lacked direct coverage: - sqlIdent guard on config-driven table names (virtual-table-query) - token-config file modes 0o600/0o700 (spawn-wiki-worker, POSIX-gated) - atomic skill write / no leftover .tmp (skill-writer) tsc --noEmit clean; touched files run 82 green tests.
Add negative-path coverage for the sweep's sqlIdent SQL-injection hardening on three shared write paths: updateColumns column-key + table-name guards and createIndex column guard (deeplake-api.test.ts), and the listSkillInvocations sessions-table guard (skill-invocations.test.ts). tsc --noEmit clean; touched files run 88 green tests. Report at library/qa/repo-sweep/c10/quality.md.
C3 security fix wrapped table identifiers in sqlIdent() in capture hooks,
changing bundle INSERT strings from "${sessionsTable}" to
"${sqlIdent(sessionsTable)}". Update plugin-version-resolution.test.ts
regex patterns to accept both forms.
C5 security fix added a hex-shape guard to vfs-handler.ts loadSnapshotOrError.
Update vfs-handler.test.ts fixtures to use valid 4-64 char hex commit SHAs
("cafe1234"/"babe1234"/"face1234") instead of invalid values ("c9"/"c1"/"c2",
"z".repeat(64), "w".repeat(64)) that now correctly fail the guard.
Add --allowedTools Write to the claude -p headless invocation in wiki-worker-spawn.ts. This limits the blast radius of a prompt-injection payload embedded in captured session content: the spawned agent can write the summary file but cannot invoke Bash, Read, computer-use, or any other tool. bypassPermissions is still required for headless TTY-less operation. A follow-up (pr/06-claude-code-security) will pivot to a stdout-capture model so bypassPermissions can be removed entirely. Addresses the High documented finding in library/qa/repo-sweep/c3/security.md.
thenotoriousllama
force-pushed
the
pr/05-security-quality-repo-sweep
branch
from
a387aa3 to
dcce3f2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/codex/pre-tool-use.ts (1)
374-387:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop returning
guidefor intercepted memory commands.This branch can hand a memory-touching command back to the real host shell (
guide), which breaks the VFS isolation contract.🔧 Suggested fix
- const isWriteRedirect = /^\s*(echo|printf|tee)\b/.test(rewritten) && /\s>>?\s/.test(rewritten); + const isWriteRedirect = /^\s*(echo|printf|tee)\b/.test(rewritten) && /\s>>?\s/.test(rewritten); @@ - // Write redirects: use "guide" so Codex reports success (not "blocked"). - // Other commands: keep "block" so the host shell never runs them. - return { action: isWriteRedirect ? "guide" : "block", output, rewrittenCommand: rewritten }; + // Never allow a memory-touching command to execute on the host shell. + // Keep all intercepted commands as "block" after serving VFS output. + return { action: "block", output, rewrittenCommand: rewritten };As per coding guidelines, "The key invariant to flag is that a memory-touching command must never be handed to the real host shell."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/codex/pre-tool-use.ts` around lines 374 - 387, The return statement in the memory command fallback block incorrectly returns "guide" for write redirects (commands matching isWriteRedirect), which violates the VFS isolation contract that memory-touching commands must never reach the host shell. Change the ternary operator in the return statement (currently `action: isWriteRedirect ? "guide" : "block"`) to always return "block" regardless of whether it is a write redirect, ensuring all intercepted memory commands are blocked and never handed to the real host shell.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@harnesses/cursor/extension/README.md`:
- Around line 56-65: The commands table in the README is missing an entry for
the `hivemind.unwireHooks` command that is defined in package.json. Add a new
row to the commands table in the README (in the location of the existing command
entries) that documents the `hivemind.unwireHooks` command alongside its title,
following the same format and structure as the other command entries like
`hivemind.wireHooks`.
In `@harnesses/cursor/extension/scripts/load-dashboard.mjs`:
- Around line 203-207: The sha variable obtained from reading the pointer file
content is used directly in a path.join() operation without validation, which
allows path traversal attacks. Before using the sha value in the
join(snapshotsDir, `${sha}.json`) statement, validate that it matches a valid
commit hash pattern (typically a 40-character hexadecimal string for git
commits). Add this validation check after the trim() operation and before the
candidate path construction to ensure only legitimate commit hashes are used in
path operations.
In `@harnesses/cursor/extension/scripts/load-rules.mjs`:
- Around line 10-12: After the `limit` variable is assigned using
`parseInt(process.argv[3] || "10", 10)`, add validation logic to handle edge
cases where parseInt returns NaN or negative values. Clamp the `limit` to ensure
it is a valid positive number (e.g., use Math.max to ensure it is at least 0 or
1, and use a fallback value or Math.abs if NaN is detected). This will prevent
slice(0, limit) from returning unexpected results when limit is NaN (empty
array) or negative (excluding trailing elements).
In `@harnesses/cursor/extension/scripts/load-sessions.mjs`:
- Around line 38-40: The hadRecall value is incorrectly derived from COUNT(*) in
the SQL query, which is always positive for returned rows, causing every session
to be marked as having a recall event. Instead of using COUNT(*) to determine
recall status, identify an existing column in the sessions table that actually
indicates whether a recall event occurred, or add a new column to the database
schema to track recall-specific events. Update the SQL query in the
load-sessions.mjs SELECT statement to use the correct field or condition to
properly determine hadRecall status rather than relying on the COUNT(*)
aggregate.
In `@harnesses/cursor/extension/src/auth/logout.ts`:
- Around line 11-17: The catch block starting at line 11 treats all errors the
same way by setting removed to false, which causes the message at line 17 to
incorrectly report "No credentials file found" for permission or I/O errors.
Check the specific error type within the catch block: if the error indicates the
file was not found (check the error code), set removed to false; for other
errors like permission denied or I/O failures, either rethrow the error to
surface it properly or log the actual error details. Update the message logic to
distinguish between a missing credentials file and actual errors that prevented
deletion.
In `@harnesses/cursor/extension/src/bridge/skill-sync.ts`:
- Around line 180-184: The manifest is being read, modified, and written
repeatedly for each skill entry, creating race conditions and lost-update
windows. Refactor the sync and backfill operations to load the manifest once at
the start, accumulate all mutations in-memory for all entries, then write the
manifest once at the end. This applies to the sync flow (around the
writeManifest calls in the skill iteration loops), the backfill operation, and
any other manifest mutation sites mentioned in the comment range references.
Remove repeated read-modify-write cycles and instead batch all changes before
persisting to disk.
In `@harnesses/cursor/extension/src/statusbar/poller.ts`:
- Around line 21-29: The start method uses fire-and-forget polling with
pollOnce(), which can cause unhandled rejections if the promise rejects, and
allows overlapping polls to start before the previous one completes. Add a guard
variable to track whether a poll is currently in progress and only invoke
pollOnce() if no poll is already running. Additionally, attach a catch handler
to the pollOnce() promise to properly handle any rejections and prevent them
from surfacing as unhandled rejections. Apply these same changes to all polling
invocations in the class, including the direct call to pollOnce() at the start
of the start method and any other polling loops that exist in the codebase.
In `@harnesses/cursor/extension/src/utils/paths.ts`:
- Around line 38-44: The monorepoRoot() function is missing one level of
directory traversal in its relative path. Currently it uses four parent
directory references (..) which only gets to the harnesses directory, but it
needs to go up one more level to reach the actual monorepo root. Add one more
".." to the join() call in the monorepoRoot() function so that the path
resolution correctly reaches the repository root, which will then allow
hivemindCursorBundleSrc() to properly construct the correct bundle path.
In `@harnesses/cursor/extension/src/webview/DashboardPanel.ts`:
- Around line 463-475: In the pushGoals method, when the goals loader returns a
failure message but loggedOut is false, the error message is being dropped
instead of surfaced to the user. After checking if loggedOut is true, add an
additional check for when loggedOut is false but result.message exists
(indicating a real failure), and post that failure message to the user similar
to how the loggedOut case handles it. Only proceed to post the successful goals
list when there is no error message present.
- Around line 563-573: When the DashboardPanel resolves the webview (in the
section where this.controller?.dispose() is called), the previous disposables
array that was pushed to context.subscriptions is not being cleaned up, leaving
old event handlers alive and causing duplicate command execution on re-resolve.
Store a reference to the previous disposables array as an instance variable, and
before creating the new DashboardController and disposables array, iterate
through the previous disposables and call dispose() on each item to clean them
up from context.subscriptions. Then update the instance variable to reference
the newly created disposables array so they can be properly disposed on the next
re-resolve.
In `@harnesses/cursor/extension/src/webview/data-bridge.ts`:
- Around line 452-454: The current timeout handler only sends SIGTERM to the
child process with no fallback mechanism, which can leave the promise unsettled
if the process ignores the termination signal. Modify the setTimeout callback to
implement a two-stage termination: first send SIGTERM via child.kill("SIGTERM"),
then set up a secondary timeout for 1-2 seconds later that sends SIGKILL via
child.kill("SIGKILL") to force process termination and ensure the promise always
settles.
- Around line 213-240: The `loadDashboardData()` function and the other four
loader functions (`loadRecentSessions()`, `loadRulesList()`, `loadGoalsList()`,
`loadSessionSummary()`) lack hard timeouts, allowing stalled child processes to
block indefinitely. Implement the timeout pattern already used in
`runHivemindCliAsync()` for all five loaders: wrap each promise with a
`setTimeout()` that kills the child process if it doesn't complete within a
reasonable time window, and clear the timeout in all event handlers (`close`,
`error`) to prevent race conditions. This ensures dashboard actions cannot be
blocked by hung child processes.
In `@harnesses/cursor/extension/src/webview/html/dashboard-shell.ts`:
- Around line 612-613: The snapshotKey assignment for graphUi at line 612-613
uses a weak cache key that only considers total count and node length, so
changes to edges or node content with the same node count won't trigger a
rebuild. Enhance the snapshotKey to include meaningful information about edges
and node content (such as edge count, edge properties, or a hash of node/edge
data) so that any substantive changes are reflected in the key and trigger
snapshot updates. Apply the same fix to the other snapshotKey assignment
location at lines 721-727, ensuring both cache key constructions use the same
strengthened logic.
In `@harnesses/cursor/extension/webpack.config.js`:
- Around line 21-25: The `@hivemind` alias in the resolve.alias section uses
four parent directory references (`../../../../`) when only three are needed to
reach the repository root from the `harnesses/cursor/extension/` directory.
Update the path in the alias configuration to use three parent directory
segments (`../../../`) instead, matching the correct depth used in
tsconfig.json, so that `@hivemind/*` imports resolve to the correct src
directory at the repository root.
In `@src/hooks/hermes/spawn-wiki-worker.ts`:
- Around line 93-97: The temporary directory name in the spawn-wiki-worker.ts
file is predictable using sessionId and Date.now(), and mkdirSync with
recursive: true accepts pre-existing paths, creating a race condition
vulnerability where an attacker can pre-create the directory and control the
token-bearing config.json location. Replace the mkdirSync call with mkdtemp
(from the fs module) to generate an unpredictable temporary directory with a
secure random suffix, which atomically creates the directory and prevents
pre-creation attacks. Use a template string like join(tmpdir(),
'deeplake-wiki-XXXXXX') with mkdtemp instead of the predictable sessionId-based
path.
In `@src/hooks/spawn-wiki-worker.ts`:
- Around line 95-99: The tmpDir variable uses a predictable path pattern
combined with recursive: true, which can be exploited even with strict
permissions since an attacker can pre-create the directory structure. Replace
the manual path construction (using join, tmpdir, sessionId, and Date.now())
with Node.js's mkdtemp() function, which atomically creates a directory with a
random suffix and ensures confidentiality of the sensitive Activeloop token
stored in the config file. Apply this same pattern fix to all other locations in
the file where similar predictable temp directories are created (approximately
lines 103-120).
In `@tests/claude-code/virtual-table-query.test.ts`:
- Around line 274-303: The test assertions in the four test cases are using
generic substring regex patterns like /Invalid SQL identifier/ in their
.rejects.toThrow() calls, which provides weak regression guarantees. Replace
each of the four .rejects.toThrow() assertions in the test cases for
readVirtualPathContents (with EVIL database parameter), readVirtualPathContents
(with EVIL sessions table parameter), listVirtualPathRowsForDirs (with EVIL
database parameter), and findVirtualPaths (with EVIL sessions table parameter)
with the exact expected error message string instead of the regex pattern to
provide stronger, more specific regression testing.
In `@tests/shared/deeplake-api.test.ts`:
- Around line 324-343: Replace the generic regex pattern `/Invalid SQL
identifier/` in the three test cases (for updateColumns with non-identifier
column key, updateColumns with non-identifier table name, and createIndex with
non-identifier column) with specific expected error messages or values unique to
each scenario. Determine the exact error messages thrown by the
api.updateColumns and api.createIndex methods for each input case and use those
specific strings in the rejects.toThrow assertions instead of the generic
substring match, so that each test locks the exact behavior expected for its
particular failure case.
In `@tests/shared/graph/vfs-handler.test.ts`:
- Line 108: The assertion in the test for the no-graph case is too broad when
checking that r.message contains "parse". Replace the toContain("parse")
assertion with a more specific assertion that checks for a stable, complete
parse-error message phrase instead. This will ensure the test only passes when
the actual expected parse-failure error occurs, rather than passing for any
unrelated error containing the word "parse". Reference the actual error message
that should be thrown for parse failures and assert against that specific
message to align with the coding guideline of preferring specific value
assertions over generic substrings.
In `@tests/shared/skill-invocations.test.ts`:
- Around line 96-101: The test at line 100 uses a broad regex pattern /Invalid
SQL identifier/ for asserting the thrown error, which could match any error
message containing that substring and miss actual contract drift. Replace the
regex-based assertion in the toThrow() call with an exact expected error message
string to ensure the test fails only for real contract changes, not partial
substring matches. Check the actual error message thrown by listSkillInvocations
when given an invalid SQL identifier and use that exact message in the assertion
instead of the regex pattern.
---
Outside diff comments:
In `@src/hooks/codex/pre-tool-use.ts`:
- Around line 374-387: The return statement in the memory command fallback block
incorrectly returns "guide" for write redirects (commands matching
isWriteRedirect), which violates the VFS isolation contract that memory-touching
commands must never reach the host shell. Change the ternary operator in the
return statement (currently `action: isWriteRedirect ? "guide" : "block"`) to
always return "block" regardless of whether it is a write redirect, ensuring all
intercepted memory commands are blocked and never handed to the real host shell.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 411cb632-59a6-4f30-b815-921eee02d70a
⛔ Files ignored due to path filters (4)
harnesses/cursor/extension/media/d3.v7.min.jsis excluded by!**/*.min.jsharnesses/cursor/extension/media/icon.svgis excluded by!**/*.svgharnesses/cursor/extension/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (138)
.github/workflows/ci.yaml.github/workflows/release.yaml.gitignoreesbuild.config.mjsharnesses/cursor/extension/.vscodeignoreharnesses/cursor/extension/README.mdharnesses/cursor/extension/package.jsonharnesses/cursor/extension/scripts/lib/deeplake.mjsharnesses/cursor/extension/scripts/load-dashboard.mjsharnesses/cursor/extension/scripts/load-goals.mjsharnesses/cursor/extension/scripts/load-rules.mjsharnesses/cursor/extension/scripts/load-session-summary.mjsharnesses/cursor/extension/scripts/load-sessions.mjsharnesses/cursor/extension/src/auth/api-key.tsharnesses/cursor/extension/src/auth/detector.tsharnesses/cursor/extension/src/auth/device-flow.tsharnesses/cursor/extension/src/auth/index.tsharnesses/cursor/extension/src/auth/logout.tsharnesses/cursor/extension/src/auth/safe-url.tsharnesses/cursor/extension/src/bridge/auto-sync.tsharnesses/cursor/extension/src/bridge/skill-sync.tsharnesses/cursor/extension/src/extension.tsharnesses/cursor/extension/src/graph/editor-sync.tsharnesses/cursor/extension/src/graph/impact-overlay.tsharnesses/cursor/extension/src/graph/snapshot-loader.tsharnesses/cursor/extension/src/graph/types.tsharnesses/cursor/extension/src/health/checker.tsharnesses/cursor/extension/src/health/index.tsharnesses/cursor/extension/src/health/wirings.tsharnesses/cursor/extension/src/statusbar/commands.tsharnesses/cursor/extension/src/statusbar/detail-view.tsharnesses/cursor/extension/src/statusbar/indicator.tsharnesses/cursor/extension/src/statusbar/poller.tsharnesses/cursor/extension/src/types/health.tsharnesses/cursor/extension/src/utils/fs-json.tsharnesses/cursor/extension/src/utils/output.tsharnesses/cursor/extension/src/utils/paths.tsharnesses/cursor/extension/src/webview/DashboardPanel.tsharnesses/cursor/extension/src/webview/data-bridge.tsharnesses/cursor/extension/src/webview/html/dashboard-shell.tsharnesses/cursor/extension/tsconfig.jsonharnesses/cursor/extension/webpack.config.jslibrary/knowledge/private/architecture/desktop-harness-overview.mdlibrary/knowledge/private/integrations/desktop-app-interception.mdlibrary/knowledge/private/security/desktop-egress-and-trust.mdlibrary/qa/repo-sweep/c1/quality.mdlibrary/qa/repo-sweep/c1/security.mdlibrary/qa/repo-sweep/c10/quality.mdlibrary/qa/repo-sweep/c10/security.mdlibrary/qa/repo-sweep/c11/quality.mdlibrary/qa/repo-sweep/c11/security.mdlibrary/qa/repo-sweep/c2/quality.mdlibrary/qa/repo-sweep/c2/security.mdlibrary/qa/repo-sweep/c3/quality.mdlibrary/qa/repo-sweep/c3/security.mdlibrary/qa/repo-sweep/c4/quality.mdlibrary/qa/repo-sweep/c4/security.mdlibrary/qa/repo-sweep/c5/quality.mdlibrary/qa/repo-sweep/c5/security.mdlibrary/qa/repo-sweep/c6/quality.mdlibrary/qa/repo-sweep/c6/security.mdlibrary/qa/repo-sweep/c7/quality.mdlibrary/qa/repo-sweep/c7/security.mdlibrary/qa/repo-sweep/c8/quality.mdlibrary/qa/repo-sweep/c8/security.mdlibrary/qa/repo-sweep/c9/quality.mdlibrary/qa/repo-sweep/c9/security.mdlibrary/requirements/backlog/prd-006-desktop-memory-harness/prd-006-desktop-memory-harness-index.mdlibrary/requirements/backlog/prd-006-desktop-memory-harness/prd-006a-interception-proxy.mdlibrary/requirements/backlog/prd-006-desktop-memory-harness/prd-006b-capture-lifecycle.mdlibrary/requirements/backlog/prd-006-desktop-memory-harness/prd-006c-recall-and-hooks.mdlibrary/requirements/backlog/prd-006-desktop-memory-harness/prd-006d-installer-health-consent.mdlibrary/requirements/backlog/prd-006-desktop-memory-harness/prd-006e-open-decisions.mdlibrary/requirements/backlog/prd-006-desktop-memory-harness/qa/README.mdpackage.jsonscripts/pack-check.mjssrc/cli/install-claude.tssrc/cli/install-cursor.tssrc/cli/update.tssrc/commands/auth.tssrc/commands/session-prune.tssrc/deeplake-api.tssrc/graph/extract/python.tssrc/graph/vfs-handler.tssrc/hooks/capture.tssrc/hooks/codex/capture.tssrc/hooks/codex/pre-tool-use.tssrc/hooks/codex/session-start-setup.tssrc/hooks/codex/spawn-wiki-worker.tssrc/hooks/codex/stop.tssrc/hooks/codex/wiki-worker.tssrc/hooks/cursor/capture.tssrc/hooks/cursor/session-end.tssrc/hooks/cursor/session-start.tssrc/hooks/cursor/spawn-wiki-worker.tssrc/hooks/cursor/wiki-worker.tssrc/hooks/hermes/capture.tssrc/hooks/hermes/session-end.tssrc/hooks/hermes/session-start.tssrc/hooks/hermes/spawn-wiki-worker.tssrc/hooks/hermes/wiki-worker.tssrc/hooks/pi/wiki-worker.tssrc/hooks/query-cache.tssrc/hooks/session-start.tssrc/hooks/spawn-wiki-worker.tssrc/hooks/upload-summary.tssrc/hooks/virtual-table-query.tssrc/hooks/wiki-worker-spawn.tssrc/hooks/wiki-worker.tssrc/mcp/server.tssrc/notifications/index.tssrc/notifications/sources/backend.tssrc/notifications/sources/org-stats.tssrc/notifications/sources/primary-banner.tssrc/notifications/state.tssrc/notifications/transcript-parser.tssrc/notifications/usage-tracker.tssrc/shell/deeplake-fs.tssrc/skillify/pull.tssrc/skillify/skill-invocations.tssrc/skillify/skill-writer.tssrc/skillify/skillify-worker.tstests/claude-code/embeddings-bundle-scan.test.tstests/claude-code/plugin-version-resolution.test.tstests/claude-code/skillify-bundle-scan.test.tstests/claude-code/skillify-session-start-injection.test.tstests/claude-code/skillify-skill-writer.test.tstests/claude-code/spawn-wiki-worker.test.tstests/claude-code/version-define-bundles.test.tstests/claude-code/virtual-table-query.test.tstests/cli/cli-install-cursor-fs.test.tstests/cursor/cursor-wiki-worker-source.test.tstests/cursor/cursor-wiki-worker.test.tstests/hermes/hermes-wiki-worker-source.test.tstests/hermes/hermes-wiki-worker.test.tstests/shared/deeplake-api.test.tstests/shared/graph/vfs-handler.test.tstests/shared/skill-invocations.test.ts
khustup2
reviewed
Jun 16, 2026
khustup2
left a comment
khustup2
left a comment
Contributor
There was a problem hiding this comment.
Token-file perms fix (0o700/0o600) is a solid High. Blockers:
- CI red,
wiki-worker-windows.test.tsexpects 6 args, source now emits 8 (--allowedTools Write). "4501/4501" isn't true on CI. --allowedTools Writeis Write-only, but the summarizer prompt has to read the JSONL + existing summary. It'll summarize blind. UseRead Writeor revert (inline).- sqlIdent sweep skips the biggest surfaces,
grep-core.tsanddeeplake-fs.tsstill interpolate table names raw (inline). - Those table names are env-only (
HIVEMIND_*), so that class is Medium defense-in-depth, not High.
Worth splitting the real fixes (token perms, query-cache, vfs hex guard, sqlIdent) into a focused PR off main with green CI.
Contributor
Author
|
All issues will be fixed in 15 minutes. |
- wiki-worker-spawn: widen --allowedTools from Write to Read Write so the summarizer can read the JSONL/existing summary before writing; fixes the summarizer-blind regression khustup2 flagged - wiki-worker-windows.test: sync CLAUDE_FLAGS to source (add --allowedTools Read Write); fixes the CI red (test expected 6 args, source emitted 8) - deeplake-fs: import sqlIdent and apply it to all four raw table-name interpolations (lines 453/464/582/595 in the original) - grep-core: import sqlIdent and apply it to all six raw table-name interpolations (memoryTable/sessionsTable across hybrid + lexical paths) Table names are env-only (HIVEMIND_*), so severity is Medium defense-in-depth as khustup2 noted -- but the fix is correct regardless.
Critical:
- load-dashboard.mjs: validate sha from latest-commit.txt with
/^[a-f0-9]{7,64}$/i before path.join(); prevents path traversal
(same guard already present in vfs-handler.ts)
Major (security):
- spawn-wiki-worker.ts (main + codex + cursor + hermes forks): replace
predictable mkdirSync(join(tmpdir(), sessionId+Date.now()), {recursive})
with mkdtempSync + chmodSync 0o700; eliminates TOCTOU pre-creation race
on the token-bearing config staging directory
- codex/pre-tool-use.ts: always return action:"block" after VFS shell
executes a memory command; returning "guide" for write-redirects was
passing control back to the real host shell, violating VFS isolation
- auth/logout.ts: distinguish ENOENT (file not found) from real I/O or
permission errors in the unlinkSync catch block; surface real errors
instead of silently reporting "no credentials file found"
Minor (quality):
- load-rules.mjs: clamp parseInt(argv[3]) with Number.isFinite +
Math.min/max to prevent NaN/negative values from making slice() return
an empty array or truncate unexpectedly
- extension README.md: add missing hivemind.unwireHooks row to the
commands table (present in package.json, absent from docs)
Contributor
Author
|
OK well maybe like...25 minutes. Committing and pushing now. |
- grep-core.test.ts: update FROM "memory"/"sessions" assertions to FROM memory/sessions; sqlIdent() returns validated names without surrounding double-quotes (quoting is unnecessary for simple idents) - grep-interceptor.test.ts: same fix for FROM "test" custom table mock - openclaw/hivemind-tools.test.ts: fix hivemind_search assertions (uses grep-core, unquoted) while keeping hivemind_read assertions quoted (uses virtual-table-query which wraps sqlIdent in explicit "...") - cursor + hermes wiki-worker-source tests: update source-code lock-in assertions from mkdirSync + recursive pattern to mkdtempSync + chmodSync 0o700 to match the TOCTOU fix All 4501 tests pass.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/openclaw/hivemind-tools.test.ts (1)
214-215: ⚡ Quick winUse exact table-identifier matching for SQL assertions.
toContain('FROM ...')is broad; prefer bounded regex (or exact clauses) so the test fails on near-miss identifiers.Suggested update
- expect(sql).toContain('FROM memory'); - expect(sql).toContain('FROM sessions'); + expect(sql).toMatch(/\bFROM\s+memory\b/); + expect(sql).toMatch(/\bFROM\s+sessions\b/);As per coding guidelines, "Prefer asserting on specific values (paths, messages) over generic substrings."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/openclaw/hivemind-tools.test.ts` around lines 214 - 215, The SQL assertions using toContain for 'FROM memory' and 'FROM sessions' are too broad and could pass with partial matches or typos. Replace these toContain assertions with bounded regex patterns that use word boundaries to ensure exact table identifier matching. For example, use a regex that explicitly matches the full table name with appropriate delimiters (like word boundaries) rather than simple substring matching, so the test will properly fail on near-miss identifiers like 'FROM memorys' or 'FROM session'.Source: Coding guidelines
tests/claude-code/grep-interceptor.test.ts (1)
105-106: ⚡ Quick winTighten SQL table assertions to exact identifiers instead of generic substrings.
These checks are currently broad and can pass on accidental matches. Use anchored patterns/word boundaries (or exact clause fragments) to assert the intended table identifiers precisely.
Suggested assertion tightening
- expect(sqls.some(s => /FROM test/.test(s) && /ILIKE|LIKE/.test(s))).toBe(true); - expect(sqls.some(s => /FROM sessions/.test(s) && /ILIKE|LIKE/.test(s))).toBe(true); + expect(sqls.some(s => /\bFROM\s+test\b/.test(s) && /\b(?:ILIKE|LIKE)\b/.test(s))).toBe(true); + expect(sqls.some(s => /\bFROM\s+sessions\b/.test(s) && /\b(?:ILIKE|LIKE)\b/.test(s))).toBe(true); - expect(sql).toContain('FROM test'); - expect(sql).toContain('FROM sessions'); + expect(sql).toMatch(/\bFROM\s+test\b/); + expect(sql).toMatch(/\bFROM\s+sessions\b/);As per coding guidelines, "Prefer asserting on specific values (paths, messages) over generic substrings."
Also applies to: 124-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/claude-code/grep-interceptor.test.ts` around lines 105 - 106, The regex patterns in the expect statements checking for table names are overly broad and can accidentally match unintended substrings. Replace the generic patterns `/FROM test/` and `/FROM sessions/` with anchored patterns using word boundaries (such as `/FROM\s+test\b/` and `/FROM\s+sessions\b/`) to ensure the assertions match the exact table identifiers precisely. Apply the same fix to the similar assertions mentioned at lines 124-125.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/claude-code/grep-interceptor.test.ts`:
- Around line 105-106: The regex patterns in the expect statements checking for
table names are overly broad and can accidentally match unintended substrings.
Replace the generic patterns `/FROM test/` and `/FROM sessions/` with anchored
patterns using word boundaries (such as `/FROM\s+test\b/` and
`/FROM\s+sessions\b/`) to ensure the assertions match the exact table
identifiers precisely. Apply the same fix to the similar assertions mentioned at
lines 124-125.
In `@tests/openclaw/hivemind-tools.test.ts`:
- Around line 214-215: The SQL assertions using toContain for 'FROM memory' and
'FROM sessions' are too broad and could pass with partial matches or typos.
Replace these toContain assertions with bounded regex patterns that use word
boundaries to ensure exact table identifier matching. For example, use a regex
that explicitly matches the full table name with appropriate delimiters (like
word boundaries) rather than simple substring matching, so the test will
properly fail on near-miss identifiers like 'FROM memorys' or 'FROM session'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a756da5-ad71-4bb3-8f2a-78a611d02748
📒 Files selected for processing (5)
tests/claude-code/grep-core.test.tstests/claude-code/grep-interceptor.test.tstests/cursor/cursor-wiki-worker-source.test.tstests/hermes/hermes-wiki-worker-source.test.tstests/openclaw/hivemind-tools.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/claude-code/grep-core.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/cursor/cursor-wiki-worker-source.test.ts
- tests/hermes/hermes-wiki-worker-source.test.ts
Closed
4 tasks
…t review Brings the focused-PR (activeloopai#271) hardening back into the full sweep so activeloopai#270 is the single shippable PR (it correctly keeps the approved cursor relocation from activeloopai#268). Changes: CodeQL Critical (auth.ts): the OAuth browser opener fed a server-derived verification URL into `cmd /c start` on Windows (cmd re-parses its own command line). Now validate the URL (https scheme, parsed) and open via `rundll32 url.dll,FileProtocolHandler` with no shell interpreter. CodeQL High (tests): cli-install-cursor-fs.test.ts creates its temp root with mkdtempSync instead of a predictable tmpdir join. Wiki-summarizer prompt-injection blast radius (Option B, full fix): pivot the Claude summarizer to the stdout model. Session transcript + existing summary are inlined into the prompt as untrusted DATA (delivered over stdin), the agent emits the summary to stdout, and the worker sanitizes (control-char strip + 100k cap) and uploads it from memory. CLAUDE_FLAGS drops bypassPermissions and all --allowedTools, so the agent has zero tools. No tmp summary file is written or read back, removing the file-system race too. CodeRabbit review: pack-check secret patterns are case-insensitive (ID_RSA / Credentials.JSON / .ENV can't bypass the publish gate); grep-core.ts and deeplake-fs.ts wrap sqlIdent() identifiers in double quotes to match the codebase convention and preserve PostgreSQL case semantics; vfs-handler test asserts the stable "Failed to parse snapshot" prefix. Full suite green (cli-update failures are a pre-existing parallel-isolation flake that passes in isolation and on CI).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shell/deeplake-fs.ts (1)
638-639:⚠️ Potential issue | 🟠 MajorIncomplete
sqlIdent()wrapping — many table interpolations remain unprotected.Lines 453, 464, 582, and 595 correctly use
sqlIdent()to escape config-driven table names. However, this file has 18+ additional sites that still interpolate table names directly into SQL. Sincetable,sessionsTable,goalsTable, andkpisTableare all user-configurable parameters passed to the constructor andcreate()method, they require the same protection to prevent SQL injection.Unprotected sites:
- Bootstrap: line 222 (
FROM "${table}")- Upserts: lines 493, 501, 511, 542, 550, 558
- Prefetch: lines 639, 654
- File reads: lines 687, 698, 721, 746, 756
- Append: line 835
- Delete: lines 1078, 1081
Apply
sqlIdent()to all remaining raw table name interpolations to match the pattern already established in lines 453, 464, 582, and 595.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shell/deeplake-fs.ts` around lines 638 - 639, The query at line 638-639 and 18+ other sites throughout the file are directly interpolating user-configurable table names (this.table, this.sessionsTable, this.goalsTable, this.kpisTable) into SQL strings without escaping, creating a SQL injection vulnerability. Apply the sqlIdent() function to wrap all raw table name interpolations, following the established pattern already correctly used in lines 453, 464, 582, and 595. Replace all instances of `"${this.table}"`, `"${this.sessionsTable}"`, `"${this.goalsTable}"`, and `"${this.kpisTable}"` with `sqlIdent(this.table)`, `sqlIdent(this.sessionsTable)`, `sqlIdent(this.goalsTable)`, and `sqlIdent(this.kpisTable)` respectively across all the affected query locations including the prefetch method, upsert operations, file read sections, append, and delete operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/shell/deeplake-fs.ts`:
- Around line 638-639: The query at line 638-639 and 18+ other sites throughout
the file are directly interpolating user-configurable table names (this.table,
this.sessionsTable, this.goalsTable, this.kpisTable) into SQL strings without
escaping, creating a SQL injection vulnerability. Apply the sqlIdent() function
to wrap all raw table name interpolations, following the established pattern
already correctly used in lines 453, 464, 582, and 595. Replace all instances of
`"${this.table}"`, `"${this.sessionsTable}"`, `"${this.goalsTable}"`, and
`"${this.kpisTable}"` with `sqlIdent(this.table)`,
`sqlIdent(this.sessionsTable)`, `sqlIdent(this.goalsTable)`, and
`sqlIdent(this.kpisTable)` respectively across all the affected query locations
including the prefetch method, upsert operations, file read sections, append,
and delete operations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c0f2eea-3fd5-4884-b73b-ed0bde737f81
📒 Files selected for processing (12)
library/qa/repo-sweep/c3/security.mdscripts/pack-check.mjssrc/commands/auth.tssrc/hooks/spawn-wiki-worker.tssrc/hooks/wiki-worker-spawn.tssrc/hooks/wiki-worker.tssrc/shell/deeplake-fs.tssrc/shell/grep-core.tstests/claude-code/wiki-worker-windows.test.tstests/claude-code/wiki-worker.test.tstests/cli/cli-install-cursor-fs.test.tstests/shared/graph/vfs-handler.test.ts
✅ Files skipped from review due to trivial changes (1)
- library/qa/repo-sweep/c3/security.md
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/cli/cli-install-cursor-fs.test.ts
- src/shell/grep-core.ts
- scripts/pack-check.mjs
- tests/shared/graph/vfs-handler.test.ts
khustup2
reviewed
Jun 17, 2026
khustup2
left a comment
khustup2
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the cursor extension too. Mergeable with a short fixlist, no security blockers (only host is api.deeplake.ai, CSP/nonce + escaping intact, no shell). CodeRabbit's two 🔴 Critical deflate: the load-dashboard.mjs one is a false positive (snapshot path is already hex-guarded), and DashboardPanel.ts:573 is real but a Major dispose leak, not Critical.
Must-fix before merge:
health/checker.ts:15—DOCS_URLstill points at thethenotoriousllamafork. Repoint toactiveloopai/hivemind(inline).webview/data-bridge.ts— the 5 loader spawns (214/256/303/329/378) have no timeout, a hung loader hangs the dashboard pane. Add the kill-timerrunHivemindCliAsyncalready has (inline).webview/DashboardPanel.ts— controllerdispose()doesn't tear down itsdisposables[], so theonDidReceiveMessagehandler double-registers on sidebar re-resolve.- No extension tests. Add coverage for the security-critical units (
auth/safe-url.ts, thedeeplake.mjsURL validation, the snapshot sha guard) and wireharnesses/cursor/extension/**intovitest.config.ts.
Deferrable: poller overlap guard, atomic manifest write, SIGTERM→SIGKILL, weak graph cache key, unused @hivemind webpack alias.
…i#270 1. health/checker.ts: repoint DOCS_URL from the thenotoriousllama fork to activeloopai/hivemind. 2. data-bridge.ts: all five dashboard loader spawns (dashboard, sessions, rules, goals, session-summary) now go through a shared spawnLoaderScript helper with a 60s SIGTERM kill-timer, so a hung loader can't wedge the dashboard pane (mirrors runHivemindCliAsync). Also add the hex-SHA guard to resolveSnapshot's latest-commit.txt pointer (was missing here vs the .mjs loader) to prevent snapshot-path traversal. 3. DashboardPanel.ts: DashboardController.dispose() now tears down its disposables[] (the webview.onDidReceiveMessage handler above all), and the sidebar view provider lets the controller own that array instead of pushing it into context.subscriptions. Fixes the double-registered message handler on sidebar re-resolve. 4. Extension tests: new harnesses/cursor/extension/test/security.test.ts covers the security-critical units (safe-url.ts URL validation incl. userinfo / look-alike spoof rejection, deeplake.mjs bearer-token host gate, and the resolveSnapshot traversal guard), wired into vitest.config.ts. Full suite green (4519 passed). Deferred items per khustup2 (poller overlap guard, atomic manifest write, SIGTERM->SIGKILL escalation, weak graph cache key, unused @hivemind webpack alias) left as follow-ups.
Contributor
|
Extension must-fix items all look addressed, thanks (DOCS_URL, spawn timeouts, dispose, tests wired). CI's red again though: the Option B summarizer (stdout, no file write) broke Worth running |
1. webpack.config.js: drop the unused @hivemind resolve alias (nothing in the extension imports @hivemind/*). 2. data-bridge.ts: SIGTERM->SIGKILL escalation in both spawnLoaderScript and runHivemindCliAsync. After the SIGTERM timeout, a 3s grace timer force-kills a child that ignores graceful termination so it can't linger; both timers are cleared on close/error. 3. statusbar/poller.ts: overlap guard. A pollInFlight flag makes the interval skip a tick while a poll is still running, and pollOnce coalesces a concurrent call to the last snapshot. Prevents stacked health-check + skill-sync polls under a slow network. 4. bridge/skill-sync.ts: atomic pull-manifest write (temp file + rename) so a crash mid-write or a concurrent reader never sees a half-written manifest (which loadManifest would silently reset to empty, dropping pull state). 5. dashboard-shell.ts: strengthen the graph snapshot cache key to include edge count + commit sha via a shared snapshotCacheKey(), and compare by equality instead of startsWith(nodeCount). An edge-only change now busts the cache and triggers a real rebuild instead of leaving stale edges. Full suite green (the tests/cli/* failures are the known parallel-isolation flake; they pass in isolation and on CI). Root + extension tsc clean.
Contributor
Author
|
On it @khustup2 & heard on npm test. Committing that to Hivemind memory! |
…MPLATE main advanced (activeloopai#268 cursor relocation + activeloopai#269 memory-backfill merged). activeloopai#269's src/skillify/stage-memory.ts imports WIKI_PROMPT_TEMPLATE and runs its own file-writing summarizer (agent writes the summary file, stage-memory reads it back). Option B had repurposed that shared template for the stdout/no-file model, so on merge stage-memory's agent was told to emit to stdout and never wrote the file, failing all 8 stage-memory.test.ts / stage-memory-wiring.test.ts assertions. Decouple instead of rewriting activeloopai#269: WIKI_PROMPT_TEMPLATE is restored to its original file-based contract (what stage-memory and the cross-fork Next Steps contract expect), and the Option B inline/stdout prompt moves to a dedicated WIKI_STDOUT_PROMPT_TEMPLATE that spawnWikiWorker passes to the live wiki-worker. stage-memory.ts and its tests are untouched and pass as-is. Merged upstream/main into the branch to sync (relocation now lands via activeloopai#268). Full npm test green except the pre-existing cli-index --version parallel-isolation flake (passes in isolation; present on main via activeloopai#269).
Contributor
Author
|
@khustup2 Fixed. I'm making some other improvements those will ship in the next batch! |
khustup2
approved these changes
Jun 17, 2026
khustup2
left a comment
khustup2
left a comment
Contributor
There was a problem hiding this comment.
Security fixes are correct and the sweep's complete, extension must-fix list addressed, CI green. LGTM. Suggest holding further changes for a follow-up PR so this lands clean.
Contributor
Author
Excellent. Thank you for your approval. When merged I will reset local and pick another off main to push more changes later. Ty! (I can't merge) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full security + quality sweep of the entire codebase (
src/,tests/,harnesses/cursor/extension/). 11 chunks processed sequentially with an armedsecurity-guardian(fixes Critical/High, commits) followed by an armedquality-guardian(fixes Medium+, commits) on each chunk. All reports live underlibrary/qa/repo-sweep/<c1-c11>/.Final state:
npm run buildgreen,npm test4501/4501 passing (14 net-new tests added to cover sweep fixes).Security findings: all Critical/High fixed
C1 - Core data + shell (
src/config.ts,src/deeplake-api.ts,src/shell/, ...)src/deeplake-api.tsupdateColumns,createIndex,upsertRowSql: column/table identifiers interpolated withoutsqlIdentin 3 test-only public methodssqlIdent/sqlStrC2 - CLI + scripts (
src/cli/,scripts/)scripts/pack-check.mjs*.pem,*.key,id_rsa,credentials.jsonC3 - Hooks (
src/hooks/) - highest-impact chunksrc/hooks/spawn-wiki-worker.ts+ 3 harness forks/tmp/deeplake-wiki-*/config.json(default umask, no mode restriction). Token JWT readable by any local user for up to 120s.0o700, file0o600on all 4 helperssrc/hooks/HIVEMIND_TABLE/HIVEMIND_SESSIONS_TABLEconfig-driven table names interpolated raw into Deeplake SQL (nosqlIdentguard), the same gap the rest of the codebase already preventssqlIdent()at ~20 query sitessrc/hooks/query-cache.tssession_idused as a path segment without sanitizationsrc/hooks/wiki-worker-spawn.tsclaude -p --permission-mode bypassPermissionsover attacker-influenceable captured session content. Blast radius: any tool (Bash, Read, arbitrary writes).--allowedTools Writeto constrain to write-only. Full fix (stdout pivot, removebypassPermissions) tracked inpr/06-claude-code-security.C4 - Skillify (
src/skillify/)src/skillify/pull.ts,skill-invocations.ts(×2),skillify-worker.ts(×2)sqlIdentC5 - Graph (
src/graph/)src/graph/vfs-handler.tsloadSnapshotOrErrorbuilt the snapshot file path fromcommit_sha/snapshot_sha256read from.last-build.jsonwithout validating hex shape. A tampered file with../../../etc/foowould escapesnapshots/and read an arbitrary.jsoninto agent context. The sibling consumers already had the guard./^[0-9a-f]{4,64}$/guard, matchingsession-context.tsanddiff.tsC6 - MCP + embeddings + notifications (
src/mcp/,src/embeddings/,src/notifications/)src/mcp/server.tslines 132, 165hivemind_readandhivemind_indextool handlers interpolated table names withoutsqlIdent. Affects every MCP tool call from Claude Code / Codex.C7 - Commands + dashboard + rules + utils (
src/commands/,src/utils/, ...)src/commands/session-prune.tssqlIdent, includingDELETE FROM "${sessionsTable}"andDELETE FROM "${config.tableName}". Destructive SQL on the PII-bearing sessions/memory tables.C8 - Cursor extension re-verify (
harnesses/cursor/extension/)All 3 PR #4 fixes confirmed present (esc() quote escaping,
--separator,sanitizeApiUrl). New finding:harnesses/cursor/extension/scripts/lib/deeplake.mjssanitizeApiUrl()usedstartsWithprefix check, acceptinghttps://api.deeplake.ai@evil.com(userinfo spoofing) andhttps://api.deeplake.ai.evil.com(look-alike subdomain). Bearer token + org ID would be sent to the attacker host. The TypeScript sibling (safe-url.ts) already used origin equality.url.originequality checkC9-C11 - Tests
All 229 test files across
tests/claude-code/,tests/shared/,tests/cli/, and per-agent suites: clean pass. No real secrets, no unsafe shell helpers, no SQL injection vectors, correctmkdtempSync+rmSynchygiene throughout.Quality findings: all Medium+ fixed
src/shell/deeplake-fs.tsappendFilesilently dropped content when a buffered write was unflushed (echo a > f && echo b >> f producedanotab)src/deeplake-api.tscatch (e: any)inensureLookupIndexloggedundefinedfor non-Error throwscatch (e: unknown)withinstanceof Errornarrowingsrc/cli/update.ts(×2),install-claude.tscatch (e: any)sites reading.messagedirectlycatch (e: unknown)pattern, consistent with codebase conventionsrc/hooks/wiki-worker.ts+ 4 forksexecSucceeded/summaryChanged) missing in 4 of 5 harness wiki-workers. On CLI failure, unchanged summary re-uploaded + JSONL offset advanced, silently marking unsummarized events as done.src/hooks/cursor/session-end.ts,hermes/session-end.tsreleaseLockin catch, matchingcodex/stop.tssrc/skillify/skill-writer.tswriteNewSkillused plainwriteFileSync; a crash left a torn SKILL.md the function could never heal (existsSyncguard threwalready existson every subsequent run, skill lost until manualrm)atomicWriteFile(tmp + rename) helper; routed both write paths through itsrc/graph/extract/python.ts@overloadstubssrc/notifications/(6 files, 13 sites)catch (e: any)across usage-tracker, transcript-parser, backend, org-stats, primary-banner, statecatch (e: unknown)withinstanceof Errornarrowingsrc/commands/auth.tsopenBrowserusedexecSyncwith a shell-interpolated OAuth URL (the only shell-string spawn in scope - missed by the C7 security pass)execFileSyncper platform, also fixing a latent Windowsstarttitle bugharnesses/cursor/extension/src/webview/DashboardPanel.tsresolveWebviewViewnever calledcontroller.dispose()on teardown, leakingFileSystemWatcherand editor listenersonDidDisposehandler + pre-resolve dispose; routed disposables throughcontext.subscriptionsharnesses/cursor/extension/src/webview/data-bridge.tsrunHivemindCliAsyncprepended["--", ...args](from PR #4 security fix) but the hivemind CLI reads subcommand fromargv[0], sohivemind -- <cmd>exited withUnknown command: --, breaking every dashboard action--prepend;spawn()is shell-less so--gave no injection protection anywayharnesses/cursor/extension/src/webview/DashboardPanel.tspushSettingscalledsyncSkillsToCursor()ignoringHIVEMIND_AUTOPULL_DISABLED, unlike the poller and sync handlerNew test coverage added
The quality passes added 14 targeted tests covering sweep security fixes that had no prior direct coverage:
sqlIdentguard on config-driven table names (3 files - virtual-table-query, deeplake-api, skill-invocations)0o600/0o700in spawn-wiki-worker (POSIX-gated).tmpafterwriteNewSkillDocumented follow-ups (not fixed in this PR)
Wiki summarizer prompt-injection blast radius (High - partially mitigated): The wiki summarizer runs
claude -p --permission-mode bypassPermissionsover captured session content (attacker-influenceable).--allowedTools Writewas added in this PR to constrain the agent to file writes only, preventing Bash/Read/arbitrary-tool execution. Full remediation (stdout pivot, removebypassPermissionsentirely) is inpr/06-claude-code-security. Seelibrary/qa/repo-sweep/c3/security.md.Centralized
sqlIdenttable-name accessor (Low): The raw-table-identifier pattern was caught in 7 separate chunks (C1-C7). A centralized validated accessor would prevent future regressions. Flagged for a follow-up refactor.commit()swallowssqlIdentthrows viaPromise.allSettled(Low):src/deeplake-api.ts:318wrapsupsertRowSqlinallSettled, silently swallowing an identifier rejection on the insert path. Pre-existing behavior, out of sweep scope.Reports
Full per-chunk reports:
library/qa/repo-sweep/c1/throughlibrary/qa/repo-sweep/c11/. Each directory containssecurity.mdandquality.md.Contribution by Legion Code Inc. / Mario Aldayuz
Summary by CodeRabbit
Release Notes
New Features
Security & Quality
Documentation