Skip to content

fix(registry): restore prefixed pipe-to-shell detection#2095

Open
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-pipe-to-shell-scanner-bypass
Open

fix(registry): restore prefixed pipe-to-shell detection#2095
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-pipe-to-shell-scanner-bypass

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • A recent change to the pipe-chain parser weakened advisory detection for dangerous install snippets by treating ;/& as hard barriers and only reading the first alphanumeric token, which produced false negatives for common shell constructs such as POSIX environment assignments and quoted URL arguments.
  • The scanner is used to surface high-risk shell patterns to reviewers, so the regression reduced reviewer triage effectiveness for remote-exec and decoded-shell patterns.

Description

  • Make pipe splitting quote-aware and respect escape sequences so separators like |, ;, and & inside single or double quotes do not break segment parsing (see pipeChainSegments).
  • Add shellTokenEnd, shellToken, and isEnvironmentAssignment helpers and an ENV_VALUE_FLAGS set to parse tokens robustly across quoting and escaping contexts.
  • Rework segmentLeadCommand to skip POSIX environment assignments and handle sudo and env wrappers (including value-taking flags) before determining the real lead command, restoring detection of prefixed curl/wget/base64 usages.
  • Add regression tests that assert detection for HTTPS_PROXY=... curl ... | sh, env ... curl ... | sh, quoted URLs containing &/;, and VAR=1 base64 -d ... | sh in tests/command-safety.test.ts.

Testing

  • Ran pnpm exec vitest run tests/command-safety.test.ts and all tests passed (1 file, 6 tests).
  • Ran pnpm validate:packages and package validation passed (Package download validation passed).
  • Ran git diff --check and no whitespace or diff issues were reported.

Codex Task

@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 11, 2026
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@JSONbored, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 25 minutes and 9 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 875115c6-2fa1-4539-b87b-20d569c3138e

📥 Commits

Reviewing files that changed from the base of the PR and between 8f302a4 and 672f031.

📒 Files selected for processing (2)
  • packages/registry/src/command-safety.js
  • tests/command-safety.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-pipe-to-shell-scanner-bypass

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gittensory

gittensory Bot commented Jun 11, 2026

Copy link
Copy Markdown

Important

Gittensory found maintainer review notes

Public GitHub metadata was checked for review readiness. Gittensor-specific context appears only when confirmed.

Readiness score: 93/100

Signal Result Evidence Action
Linked issue ⚠️ Missing No linked issue or no-issue rationale found. Explain no-issue PR.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found. No action.
Review load ✅ 20/20 Readiness component derived from cached public PR metadata and labels; size label size:L. No action.
Validation evidence ✅ 25/25 PR body includes validation/test evidence. No action.
Open PR queue ✅ 10/10 0 open PR(s), 0 likely reviewable. No action.
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 62 PR(s), 347 issue(s). No action.
Gate result ✅ Passing No configured blocker found. No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 62 PR(s), 347 issue(s).
  • PR-specific overlap: none found.
Maintainer notes
  • No linked issue detected: The planned PR does not reference a closing issue or explicit linked issue number.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Re-run Gittensory review

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers. Learn more about Gittensor contribution workflows.

@gittensory gittensory Bot added the gittensory:reviewed Gittensor contributor context label Jun 11, 2026
@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jun 11, 2026
@JSONbored JSONbored closed this Jun 11, 2026
@JSONbored JSONbored reopened this Jun 11, 2026
@reviewwed

reviewwed Bot commented Jun 11, 2026

Copy link
Copy Markdown

🔍 Maintainer advisory review

Reviewed 2 changed file(s) — two independent AI reviewers. This is an advisory review — it does not merge or close the PR.

Suggested action: 🛠️ Request changes. (reviewers split: merge / request changes) Address the items below before merging.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The patch correctly restores detection of pipe‑to‑shell patterns when they are prefixed with environment assignments, sudo, or env commands. The added token parsing handles quotes, escapes, and flag values, improving security coverage without introducing execution paths. The changes follow existing code style, and new tests verify the intended behavior. No obvious new security issues or breaking changes are visible, though more exhaustive quoting tests would be beneficial.

Suggestions

  • Add tests for complex quoting and escaped spaces to ensure the parser behaves correctly in edge cases.
  • Consider documenting the new ENV_VALUE_FLAGS set and its intended usage.

Worth double-checking

  • Potential increase in false positives for legitimate pipelines that use env prefixes but are safe.
  • Edge‑case quoting or escaped characters might not be fully covered by current tests.

Reviewer B · mistral-small-3.1-24b-instruct — recommends 🛠️ request changes
The change aims to restore prefixed pipe-to-shell detection in the command safety validation logic. It introduces handling for escaped characters, quotes, and environment assignments, which seems to align with the description.

Suggestions

  • Consider adding more comments to explain the logic, especially the new handling of escaped characters and quotes.
  • Ensure that the new logic is thoroughly tested, particularly edge cases involving escaped characters and quotes.
  • Review the performance impact of the new logic, especially for long command lines.

Worth double-checking

  • The new logic for handling escaped characters and quotes could introduce subtle bugs if not thoroughly tested.
  • There is a risk of false positives or negatives in detecting dangerous shell patterns due to the complexity of the new logic.
  • The change might introduce performance issues for very long command lines.

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

Labels

aardvark codex gittensory:reviewed Gittensor contributor context size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant