Skip to content

fix: add security guardrails and cleanup lint warnings - #96

Open
Misaka477 wants to merge 1 commit into
stepfun-ai:mainfrom
Misaka477:fix/security-shell-path-traversal-redos
Open

fix: add security guardrails and cleanup lint warnings#96
Misaka477 wants to merge 1 commit into
stepfun-ai:mainfrom
Misaka477:fix/security-shell-path-traversal-redos

Conversation

@Misaka477

@Misaka477 Misaka477 commented Jul 4, 2026

Copy link
Copy Markdown

Summary

Aligns command safety with the existing policy/inspection and approval flow, fixes workspace containment comparison, and adds ReDoS fallback guards.

Changes

  1. Forced confirmation for dangerous commands — moves dangerous-command handling to ToolPolicy; native Bash now exposes the command through inspect(). Dangerous commands require explicit confirmation even in auto mode or with a tool-level allow override; bashExecute no longer hard-blocks commands.
  2. Path helper consolidation — moves duplicated native resolveWorkspacePath logic into @step-cli/utils/path. Native tools retain their existing support for absolute paths outside the workspace; resolveInWorkspace now uses a path.relative containment check to reject prefix siblings such as /repo-evil, including case-insensitive Windows comparison.
  3. ReDoS fallback guard — validates JS grep patterns before compilation, returns the skipped pattern in the result, and covers a nested-quantifier risk case plus a benign pattern.
  4. Targeted lint cleanup — retains only the two warnings still present on the rebased base branch.

Verification

  • pnpm check
  • 1532 passed, 9 skipped
  • Lint has no errors; remaining warnings are pre-existing on current main.

Closes #88, Closes #89, Closes #90

@github-actions github-actions Bot added area/core packages/core area/realtime packages/realtime + extensions/realtime-* area/extensions extensions/llm, extensions/mcp (non realtime-*) labels Jul 4, 2026
@ZouR-Ma

ZouR-Ma commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

先说认可的部分:#88/#89/#90 三个安全 issue 都是真问题,jsGrep 的 ReDoS 兜底和「先探测再编译」的写法也对。但这个 PR 需要的返工比较多,按优先级列出:

  1. (必须)rebase,并砍掉已被 #10 覆盖的文件。 你开 PR 之后 chore: fix lint warnings in VAD, AEC, and stateless adapter #10 已合入,把 VAD/AEC/stateless adapter 的 lint 警告清掉了——所以本 PR 现在是 CONFLICTING,cli-handlers.ts/resolver.ts/stepfun-stateless.ts/browser-audio-driver.ts 这些改动大概率已多余。rebase 后 lint 清理保留 factory.test.tsmocks.ts 两处即可(正是 main 当前仍有的警告),body 里「0 warnings」的说法也请同步改。
  2. (必须)shell 命令拦截改挂到既有的 policy/inspection 层,不要在 bashExecute 里硬拦。 仓库已经有一套命令安全架构:packages/core/src/policy/tool-policy.ts#2 建立)+ inspectToolCall 风险分级 + 网关审批流——危险命令的既定路径是「识别 → 提风险 → 人工确认」。在工具实现层加无条件硬拦截等于开了第二套互不知晓的策略层,而且硬 blocklist 双向都不牢:dd/chown 全禁会误伤合法操作,echo "别跑 rm -rf /" 这种提及危险命令的输入也被拦且用户无法 override;反过来 rm -rf /*bash -c '…'、变量拼接全都绕得过去。建议把这组 pattern 变成 inspection 层的风险信号:匹配则强制 confirm(任何审批模式下都弹),防御纵深还在、人在环上、单一事实来源。验收标准:bashExecute 无硬拦截 + 一条「危险 pattern 强制审批」用例。
  3. (必须)修 resolveWorkspacePath 检查自身的洞,并论证行为变更。 (a) startsWith(path.resolve(workspaceRoot)) 有经典前缀漏洞:workspace 是 /repo/repo-evil/x 能通过——请改成 normalized === root || normalized.startsWith(root + path.sep),Windows 注意大小写;(b) 绝对路径此前是放行的,改后一律拒绝 workspace 外,读 /tmp、全局配置这类合法场景会直接坏——这是行为变更不是纯加固,请在 body 里写清影响面,收紧与否我们拍板;(c) 同一函数在 file-tools.tsshell-tools.ts 复制了两份,请抽成共享辅助(你在 #98 里正在做同类去重)。
  4. (必须)撤销 .gitignore 改动。 AGENTS.md §7 明确暂不接收对它的修改;个人工作文件用本地 .git/info/exclude

ReDoS 防护可以保留,建议给「pattern skipped」的返回文案带上原 pattern 方便改写,并补两条启发式的边界用例。流程上:测试矩阵没跑是首次贡献者 workflow 需要我们批准,修完 push 后我们来点。三个 PR 之间有文件重叠,这个建议排在 #98/#97 之后进。改完 @ 我们。

@Misaka477
Misaka477 force-pushed the fix/security-shell-path-traversal-redos branch from bc07d01 to 36faec7 Compare July 27, 2026 16:26
@github-actions github-actions Bot added the area/utils packages/utils label Jul 27, 2026
@Misaka477

Copy link
Copy Markdown
Author

@ZouR-Ma 已按反馈完成并 force-push:

  • 已 rebase 到最新 main,去掉 chore: fix lint warnings in VAD, AEC, and stateless adapter #10 已覆盖的 VAD/AEC/stateless lint 改动;.gitignore 与本地工作文档均已移除。
  • 删除 bashExecute 的硬拦截;native Bash 增加 inspection,危险命令现在由既有 ToolPolicy 强制返回 confirm,auto mode 和 tool-level allow override 都不能绕过。
  • native 工具继续保留 workspace 外绝对路径的既有支持;重复 helper 已抽到 @step-cli/utils/pathresolveInWorkspace 改用 path.relative,覆盖 /repo-evil 前缀及 Windows 大小写。
  • ReDoS 返回会包含原 pattern,新增 (a+)+b 风险与正常 regex 边界测试。
  • pnpm check 通过(1532 passed,9 skipped;lint 无 error,当前 main 的既有 warning 仍存在)。

PR body 也已同步更新。麻烦批准首次贡献者 workflow 跑三平台矩阵。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core packages/core area/extensions extensions/llm, extensions/mcp (non realtime-*) area/realtime packages/realtime + extensions/realtime-* area/utils packages/utils

Projects

None yet

2 participants