Skip to content

Security/harden shell cicd #211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vlad-activeloop wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Security/harden shell cicd #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bde56b8
remove sed/awk/tar/env from VFS allowlist, block ANSI-C quoting
vlad-activeloop May 27, 2026
769fbc0
eliminate shell fallback in VFS hook, escape Grep pattern
vlad-activeloop May 27, 2026
9c47809
pin auto-update to exact discovered npm version
vlad-activeloop May 27, 2026
9dd2f4c
use npm ci in CI to enforce lockfile
vlad-activeloop May 27, 2026
445c912
mark backend notifications user-visible-only to prevent model injection
vlad-activeloop May 27, 2026
8b9ef58
scope 1password clawhub token to login step only
vlad-activeloop May 27, 2026
91e4469
edits
vlad-activeloop May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
node-version: 22

- name: Install dependencies
run: npm install
run: npm ci

- name: Run jscpd
# Threshold 7% is the current baseline (see .jscpd.json). The job
Expand Down Expand Up @@ -64,7 +64,7 @@ jobs:
node-version: 22

- name: Install dependencies
run: npm install
run: npm ci

- name: Build (typecheck + emit bundle artefacts)
# `build` runs `tsc && esbuild`, which is a strict superset of
Expand Down Expand Up @@ -331,7 +331,7 @@ jobs:
# tree-sitter.mjs exit 1 here, failing the job at the earliest
# possible step (instead of swallowing the warning and failing
# later in tsc with a confusing "Cannot find module" error).
run: npm install
run: npm ci

- name: Build (typecheck + emit bundle artefacts)
# Second backstop: if the strict-postinstall path somehow doesn't
Expand Down
10 changes: 7 additions & 3 deletions .github/workflows/release.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,10 @@ jobs:
cache: "npm"

- name: Load secrets from 1Password
id: op_secrets
uses: 1Password/load-secrets-action@v4.0.0
with:
export-env: true
export-env: false
env:
OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
CLAWHUB_TOKEN: "op://GitHub Actions/hivemind/CLAWHUB_TOKEN"
Expand Down Expand Up @@ -319,8 +320,11 @@ jobs:
- name: Authenticate ClawHub CLI
# `clawhub login --token` writes a credential file inside the
# runner's $HOME, which is ephemeral and discarded when the job
# ends. The token only ever appears as ${{ secrets.* }}, which
# GitHub auto-masks in logs.
# ends. The token is scoped to this step only (not exported to all
# steps via export-env) to limit the blast radius if any build step
# is compromised.
env:
CLAWHUB_TOKEN: ${{ steps.op_secrets.outputs.CLAWHUB_TOKEN }}
run: clawhub login --token "$CLAWHUB_TOKEN" --no-browser

- name: Publish openclaw bundle to ClawHub
Expand Down
159 changes: 159 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions src/cli/update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ export async function runUpdate(opts: UpdateOptions = {}): Promise<number> {
switch (detected.kind) {
case "npm-global": {
if (opts.dryRun) {
log(`(dry-run) Would run: npm install -g ${PKG_NAME}@latest`);
log(`(dry-run) Would run: npm install -g ${PKG_NAME}@${latest}`);
log(`(dry-run) Would re-run: hivemind install --skip-auth`);
return 0;
}
Expand All @@ -297,10 +297,14 @@ export async function runUpdate(opts: UpdateOptions = {}): Promise<number> {
try {
log(`Upgrading via npm…`);
try {
spawn("npm", ["install", "-g", `${PKG_NAME}@latest`]);
// Pin to the exact version fetched from the registry rather than
// re-resolving `@latest` at install time — avoids a race where a
// new (potentially malicious) publish lands between the version
// check and the install.
spawn("npm", ["install", "-g", `${PKG_NAME}@${latest}`]);
} catch (e: any) {
warn(`npm install failed: ${e.message}`);
warn(`Try running it manually: npm install -g ${PKG_NAME}@latest`);
warn(`Try running it manually: npm install -g ${PKG_NAME}@${latest}`);
return 1;
}
log(``);
Expand Down
15 changes: 11 additions & 4 deletions src/hooks/memory-path-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,29 @@ export const HOME_VAR_PATH = "$HOME/.deeplake/memory";
export const SAFE_BUILTINS = new Set([
"cat", "ls", "cp", "mv", "rm", "rmdir", "mkdir", "touch", "ln", "chmod",
"stat", "readlink", "du", "tree", "file",
"grep", "egrep", "fgrep", "rg", "sed", "awk", "cut", "tr", "sort", "uniq",
// sed and awk removed: sed supports `-e '1e <cmd>'` (execute shell command)
// and awk supports `system()` / `|` pipelines — both enable arbitrary code
// execution through the just-bash fallback.
"grep", "egrep", "fgrep", "rg", "cut", "tr", "sort", "uniq",
"wc", "head", "tail", "tac", "rev", "nl", "fold", "expand", "unexpand",
"paste", "join", "comm", "column", "diff", "strings", "split",
"find", "xargs", "which",
"jq", "yq", "xan", "base64", "od",
"tar", "gzip", "gunzip", "zcat",
// tar removed: --to-command=<cmd> executes an arbitrary program per entry.
// env removed: `env <cmd>` runs an arbitrary program.
"gzip", "gunzip", "zcat",
"md5sum", "sha1sum", "sha256sum",
"echo", "printf", "tee",
"pwd", "cd", "basename", "dirname", "env", "printenv", "hostname", "whoami",
"pwd", "cd", "basename", "dirname", "printenv", "hostname", "whoami",
"date", "seq", "expr", "sleep", "timeout", "time", "true", "false", "test",
Comment on lines 8 to 25
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

isSafe() is still bypassable via shell wrappers/control syntax.

This still only validates the first token of each stage, but the allowlist includes tokens that can hide a second command. if true; then curl ~/.deeplake/memory/x; fi, timeout 1 curl ..., and find / -exec curl ... \; all pass this check today because the dangerous command is never the first token. Please switch this to explicit validation of the exact Bash shapes you later intercept, or remove every keyword/wrapper that can dispatch a child command.

Also applies to: 30-38

🤖 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/memory-path-utils.ts` around lines 8 - 25, The isSafe() check
relying on SAFE_BUILTINS is bypassable because it only checks the first token
and allows shell wrappers/control syntax to dispatch child commands; update the
validation in isSafe() (and any related code that iterates SAFE_BUILTINS) to
perform strict token-level checks: parse the entire command line into tokens and
ensure the first token exactly matches an allowed binary name and that no
control/operator tokens (e.g., ; && || | `$( )` backticks, $(), `exec`, `if`,
`then`, `else`, `for`, `while`, `find -exec`, `timeout`, meta-words like `env`)
or redirections are present anywhere in the token list, or alternatively remove
wrapper/keyword names from SAFE_BUILTINS so only literal safe binaries remain;
in short, replace the current first-token substring check with exact-token
matching plus a reject-list of shell control keywords to prevent wrappers from
invoking child commands (update SAFE_BUILTINS usage and the isSafe()
implementation accordingly).

"alias", "unalias", "history", "help", "clear",
"for", "while", "do", "done", "if", "then", "else", "fi", "case", "esac",
]);

export function isSafe(cmd: string): boolean {
if (/\$\(|`|<\(/.test(cmd)) return false;
// $'...' is ANSI-C quoting: bash expands escape sequences inside it before
// the child process sees them, bypassing the single-quote stripping below.
if (/\$\(|`|<\(|\$'/.test(cmd)) return false;
const stripped = cmd.replace(/'[^']*'/g, "''").replace(/"[^"]*"/g, '""');
const stages = stripped.split(/\||;|&&|\|\||\n/);
for (const stage of stages) {
Expand Down
32 changes: 15 additions & 17 deletions src/hooks/pre-tool-use.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env node

import { existsSync, mkdirSync, writeFileSync } from "node:fs";
import { mkdirSync, writeFileSync } from "node:fs";
import { homedir } from "node:os";
import { join, dirname, sep } from "node:path";
import { fileURLToPath } from "node:url";
Expand Down Expand Up @@ -31,9 +31,6 @@ export { isSafe, touchesMemory, rewritePaths };
const log = (msg: string) => _log("pre", msg);

const __bundleDir = dirname(fileURLToPath(import.meta.url));
const SHELL_BUNDLE = existsSync(join(__bundleDir, "shell", "deeplake-shell.js"))
? join(__bundleDir, "shell", "deeplake-shell.js")
: join(__bundleDir, "..", "shell", "deeplake-shell.js");

export interface PreToolUseInput {
session_id: string;
Expand Down Expand Up @@ -131,7 +128,11 @@ export function getShellCommand(toolName: string, toolInput: Record<string, unkn
const flags: string[] = ["-r"];
if (toolInput["-i"]) flags.push("-i");
if (toolInput["-n"]) flags.push("-n");
return `grep ${flags.join(" ")} '${pattern}' /`;
// Single-quote the pattern safely: escape any embedded single quotes
// so the string can never break out of the shell quoting context if
// this command string is ever forwarded to a shell executor.
const escaped = pattern.replace(/'/g, "'\\''");
return `grep ${flags.join(" ")} '${escaped}' /`;
}
break;
}
Expand Down Expand Up @@ -189,13 +190,6 @@ export function extractGrepParams(
return null;
}

function buildFallbackDecision(shellCmd: string, shellBundle = SHELL_BUNDLE): ClaudePreToolDecision {
return buildAllowDecision(
`node "${shellBundle}" -c "${shellCmd.replace(/"/g, '\\"')}"`,
`[DeepLake shell] ${shellCmd}`,
);
}

interface ClaudePreToolDeps {
config?: ReturnType<typeof loadConfig>;
createApi?: (table: string, config: NonNullable<ReturnType<typeof loadConfig>>) => DeeplakeApi;
Expand All @@ -209,7 +203,6 @@ interface ClaudePreToolDeps {
readCachedIndexContentFn?: typeof readCachedIndexContent;
writeCachedIndexContentFn?: typeof writeCachedIndexContent;
writeReadCacheFileFn?: typeof writeReadCacheFile;
shellBundle?: string;
logFn?: (msg: string) => void;
}

Expand All @@ -233,7 +226,6 @@ export async function processPreToolUse(input: PreToolUseInput, deps: ClaudePreT
readCachedIndexContentFn = readCachedIndexContent,
writeCachedIndexContentFn = writeCachedIndexContent,
writeReadCacheFileFn = writeReadCacheFile,
shellBundle = SHELL_BUNDLE,
logFn = log,
} = deps;

Expand Down Expand Up @@ -290,7 +282,7 @@ export async function processPreToolUse(input: PreToolUseInput, deps: ClaudePreT
}

if (!shellCmd) return null;
if (!config) return buildFallbackDecision(shellCmd, shellBundle);
if (!config) return null;

Comment on lines 284 to 286
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Do not fall through to the real Bash tool for accepted memory commands.

Once a memory-touching Bash command has made it past getShellCommand(), returning null here hands the original command back to Claude Code's host shell. That is not harmless: sort /etc/passwd ~/.deeplake/memory/index.md > /tmp/out will still read/write real files if no virtual handler matches. These paths should deny the tool or emit the existing retry guidance instead of no-oping.

Also applies to: 512-518

🤖 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/pre-tool-use.ts` around lines 284 - 286, The early return of null
when shellCmd and config are present lets dangerous real-shell commands run;
instead, inside the pre-tool-use flow (the branch after getShellCommand()
returns a shellCmd and config exists) detect memory-touching/virtual-memory
paths and do not return null — either reject the tool invocation or return the
existing retry guidance response used elsewhere; update the logic around
getShellCommand(), shellCmd and config to call the same denial/retry handler
used for other memory-touching cases (reuse the retry guidance path) so commands
like sort /etc/passwd ~/.deeplake/... are blocked rather than forwarded to the
real Bash tool.

const table = process.env["HIVEMIND_TABLE"] ?? "memory";
const sessionsTable = process.env["HIVEMIND_SESSIONS_TABLE"] ?? "sessions";
Expand Down Expand Up @@ -514,10 +506,16 @@ export async function processPreToolUse(input: PreToolUseInput, deps: ClaudePreT
}
}
} catch (e: any) {
logFn(`direct query failed, falling back to shell: ${e.message}`);
logFn(`direct query failed: ${e.message}`);
}

return buildFallbackDecision(shellCmd, shellBundle);
// No compiled handler matched (or a direct query failed). Do NOT fall
// through to the shell executor: the shell bundle runs commands via
// just-bash which may invoke real host binaries, creating a code-execution
// surface for injected patterns in unhandled commands (e.g. `sort`, `cut`).
// Return null so the original tool runs unmodified; it will fail harmlessly
// because the virtual paths do not exist on the real filesystem.
return null;
}

/* c8 ignore start */
Expand Down
4 changes: 4 additions & 0 deletions src/notifications/sources/backend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ function toClient(n: ServerNotification): Notification | null {
// dedup_key is hashed in here so a server that reuses the same UUID
// with a fresh dedup_key (rare but supported) re-fires for the user.
dedupKey: { id: n.id, dedup_key: n.dedup_key ?? "" },
// Server-controlled content must not reach the model's additionalContext:
// an attacker who can push a notification can otherwise inject arbitrary
// instructions into the agent's system prompt.
userVisibleOnly: true,
};
}

Expand Down
Loading
Loading