Skip to content

fix(miner): fail closed for unenforceable CLI house rules#5433

Closed
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-cli-coding-agents-vulnerability
Closed

fix(miner): fail closed for unenforceable CLI house rules#5433
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-cli-coding-agents-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Production CLI providers (claude-cli/codex-cli) could be selected and run non-interactively against attacker-controlled issue prompts without house-rule enforcement, enabling local secret/credential exfiltration or unauthorized edits.
  • The engine already rejects explicit hooks for CLI providers but previously defaulted hooks only for agent-sdk, leaving a reachability gap when CLI args were made non-interactive.

Description

  • Always build and forward the default house-rule hooks before provider dispatch in constructProductionCodingAgentDriver so callers receive the default hook set prior to driver construction.
  • Mirror that behavior in runHouseRulesEnforcedCodingAgentAttempt so the house-rules wrapper also defaults hooks unconditionally.
  • Rely on the engine's existing unsupported_coding_agent_driver_hooks guard to cause CLI subprocess providers to fail closed when they cannot enforce hooks, preventing unenforced runs with untrusted prompts.
  • Update regression tests to assert that production construction and the house-rules wrapper reject claude-cli/codex-cli when hooks cannot be enforced (test/unit/miner-coding-agent-construction.test.ts and test/unit/miner-coding-agent-house-rules.test.ts).

Testing

  • Ran targeted unit tests with npx vitest run test/unit/miner-coding-agent-construction.test.ts test/unit/miner-coding-agent-house-rules.test.ts, and both test files passed.
  • Built the miner package with npm run build:miner, which succeeded.
  • npm run typecheck failed due to unrelated pre-existing TypeScript issues in other files, and a full npm run test:ci attempt was interrupted by external actionlint/network and cf-typegen drift issues; these CI interruptions are environmental and not regressions introduced by this change.
  • npm audit --audit-level=moderate could not complete due to a registry access error (external to this change).

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 12, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 20955b2 Commit Preview URL

Branch Preview URL
Jul 12 2026, 07:17 PM

@JSONbored JSONbored marked this pull request as draft July 12, 2026 19:16
@superagent-security

Copy link
Copy Markdown
Contributor

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

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 12, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-12 21:34:14 UTC

4 files · 2 blockers · readiness 89/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers No AI review summary
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (draft PR; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 44 registered-repo PR(s), 36 merged, 435 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 44 PR(s), 435 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
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: not available
  • Official Gittensor activity: 44 PR(s), 435 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Mark ready when done.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.45%. Comparing base (0d78f24) to head (20955b2).
⚠️ Report is 18 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5433   +/-   ##
=======================================
  Coverage   94.45%   94.45%           
=======================================
  Files         552      552           
  Lines       44310    44310           
  Branches    14659    14659           
=======================================
  Hits        41851    41851           
  Misses       1784     1784           
  Partials      675      675           
Flag Coverage Δ
shard-1 43.90% <0.00%> (-0.27%) ⬇️
shard-2 34.13% <50.00%> (-0.49%) ⬇️
shard-3 32.12% <50.00%> (+0.60%) ⬆️
shard-4 31.30% <50.00%> (-0.21%) ⬇️
shard-5 32.93% <0.00%> (-0.41%) ⬇️
shard-6 43.91% <0.00%> (+0.57%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../gittensory-miner/lib/coding-agent-construction.js 100.00% <100.00%> (ø)
...s/gittensory-miner/lib/coding-agent-house-rules.js 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored

Copy link
Copy Markdown
Owner Author

Closing as a regression, not a fix.

This is a direct revert of #5142 (029ada8), merged into main earlier the same day (01:59) — well before this branch's own merge-base, so it isn't merely stale, it's actively undoing the last deliberate change to this exact logic. #5142's own commit message explains why the agent-sdk-only scoping exists: constructProductionCodingAgentDriver/runHouseRulesEnforcedCodingAgentAttempt used to compute default hooks unconditionally for every provider, which combined with the engine's fail-closed createCliProvider guard (throws if hooks is supplied at all to a CLI provider) meant every production claude-cli/codex-cli construction would throw regardless of caller intent.

This PR removes exactly that providerName === 'agent-sdk' ternary in both functions, restoring the unconditional default #5142 explicitly rejected. The PR's own test diff proves it: it deletes "constructs a claude-cli driver wired to an injected spawn, without invoking it during construction" and "defaults to a real (non-injected) spawn for a CLI provider when the caller supplies none" (both pass on main today), and rewrites the test literally named "REGRESSION: does NOT default-fill house-rule hooks for claude-cli/codex-cli" to assert the opposite — that default construction now throws unsupported_coding_agent_driver_hooks for both.

constructProductionCodingAgentDriver is the real, sole production driver-construction path (per #5399), and claude-cli/codex-cli are fully-configured, documented providers — not stubs. Merging this would make the miner permanently unable to construct either one under any circumstances. The underlying claim (CLI providers have no PreToolUse surface) is true but already accepted by design and already fails closed for any caller that explicitly requests hooks; there's no silent-bypass gap left for this to close. No action needed; flagging for visibility.

@JSONbored JSONbored closed this Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant