feat(miner): wire real git worktree preparation into the attempt pipeline (#5132) - #5237
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | 5b523df | Commit Preview URL Branch Preview URL |
Jul 12 2026, 10:52 AM |
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
| const configHome = typeof env.XDG_CONFIG_HOME === "string" && env.XDG_CONFIG_HOME.trim() ? env.XDG_CONFIG_HOME.trim() : join(homedir(), ".config"); | ||
| return join(configHome, "gittensory-miner", DEFAULT_CLONE_DIR_NAME); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
P2: repoFullName path traversal allows cloning repositories outside intended clone directory
normalizeRepoFullName does not reject .. segments in owner or repo, allowing repoPath to escape cloneBaseDir.
Validate owner and repo with a regex or explicit .. rejection to prevent path traversal.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="packages/gittensory-miner/lib/repo-clone.js">
<violation number="1" location="packages/gittensory-miner/lib/repo-clone.js:30">
<priority>P2</priority>
<title>repoFullName path traversal allows cloning repositories outside intended clone directory</title>
<evidence>normalizeRepoFullName validates only that repoFullName contains exactly one slash, but does not reject path-traversal segments like ".." in owner or repo. For example, repoFullName="../foo" passes validation, and ensureRepoCloned then builds repoPath = join(cloneBaseDir, "..", "foo"), which escapes the intended clone directory. An attacker who controls repoFullName (and optionally remoteUrl) can clone arbitrary repository content to any filesystem location writable by the process.</evidence>
<recommendation>Add validation in normalizeRepoFullName to reject owner or repo segments that equal "." or "..", or use a stricter regex like /^[a-zA-Z0-9._-]+$/ for each segment. Also consider resolving the final path and verifying it is still under cloneBaseDir.</recommendation>
</violation>
</file>
superagent-security
Bot
added
the
pr:flagged
github-project-automation
Bot
moved this to Todo
in Autonomous Miner System (Loopover AMS)
JSONbored
added this to the
Miner Wave 3.5 — Autonomous Loop Wiring (maintainer) milestone
…line (#5132) packages/gittensory-miner/lib/worktree-allocator.js (given "its first real caller" in #5152) only does SQLite slot bookkeeping + mkdir -- it runs zero git commands. workingDirectory handed to runIterateLoop would have been an empty, non-git directory: the coding agent driver would have nothing real to edit. Adds repo-clone.js (a per-repo base-clone cache: git clone once, then fetch + hard-reset to the base branch on every subsequent attempt so content stays fresh) and attempt-worktree.js, which composes it with @jsonbored/gittensory-engine's addWorktree/ removeWorktree/shouldRetainWorktree primitives -- these already existed, tested, but were never called from this package (confirmed via git grep: zero production call sites anywhere in packages/gittensory-miner before this). A test caught a real wiring bug before this ever shipped: prepareAttemptWorktree initially forgot to forward its own remoteUrl/runGit test-injection options through to ensureRepoCloned, so every call silently hit the real GitHub URL regardless of what the caller passed -- caught by the REGRESSION integration test (a real local repo, real git worktree add, asserting real repo content lands on a real branch), not by review.
normalizeRepoFullName only checked for exactly one "/" separator, so a value like "../foo" passed validation and resolveRepoCloneDir/ensureRepoCloned would join it straight into the clone base dir, escaping the intended clone directory. Reject "."/".." segments and restrict owner/repo to GitHub's actual allowed character set.
JSONbored
force-pushed
the
feat/miner-worktree-preparation
branch
from
July 12, 2026 10:50
221272f to
5b523df
Compare
Contributor
|
Superagent did not find any vulnerabilities or security issues in this PR. |
superagent-security
Bot
removed
the
pr:flagged
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5237 +/- ##
=======================================
Coverage 94.34% 94.34%
=======================================
Files 473 473
Lines 39982 39982
Branches 14576 14576
=======================================
Hits 37722 37722
Misses 1585 1585
Partials 675 675
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
loopover-orb
Bot
added
the
gittensor:feature
Contributor
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 10:56:59 UTC
⏸️ Suggested Action - Manual Review
Review summary Blockers
Nits — 5 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk 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.
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.
|
github-project-automation
Bot
moved this from Todo
to Done
in Autonomous Miner System (Loopover AMS)
5 tasks
JSONbored
added a commit
that referenced
this pull request
Jul 12, 2026
…5252) Advances #5132 worktree-allocator.js only reserves a concurrency SLOT (its own `slot-N` placeholder directories never receive real git content) -- attempt-cli.js never called attempt-worktree.js's prepareAttemptWorktree (#5237) to actually clone/fetch and create a real `git worktree`, so the worktreePath reported by a blocked attempt pointed at an empty directory, not real repo content. Wires it in: prepared right after the coding-agent driver is confirmed configured, its real path replaces the allocator's slot path in every reported result, a new "blocked_worktree_preparation_failed" outcome (exit code 6) reports a real clone/fetch failure, and the worktree is cleaned up in `finally` since no real attempt runs in it yet.
5 tasks
JSONbored
added a commit
that referenced
this pull request
Jul 12, 2026
…rktree Advances #5132 runMinerAttempt's killSwitchScope needs MinerGoalSpec.killSwitch.paused from the target repo's real .gittensory-miner.yml. checkMinerKillSwitch (governor-kill-switch.js, #2341) already resolves the scope once given that value, but nothing in this package ever fetched/parsed the file itself. Unlike self-review-context.js/rejection-signal.js/ams-policy.js (which fetch live over raw.githubusercontent.com before any clone exists), this reads the file from an ALREADY-CLONED repo on disk -- by the time a real attempt reaches this point, attempt-worktree.js's prepareAttemptWorktree (#5237/#5252) has already cloned it, so no extra network round trip is needed. KNOWN GAP, same discipline as this epic's other standalone pieces: not yet wired into attempt-cli.js's runAttempt, since that wiring needs a real repoPath from #5252's worktree preparation (open, CI-green, not yet merged as of this PR). Follow-up once it lands.
JSONbored
added a commit
that referenced
this pull request
Jul 12, 2026
…rktree (#5255) Advances #5132 runMinerAttempt's killSwitchScope needs MinerGoalSpec.killSwitch.paused from the target repo's real .gittensory-miner.yml. checkMinerKillSwitch (governor-kill-switch.js, #2341) already resolves the scope once given that value, but nothing in this package ever fetched/parsed the file itself. Unlike self-review-context.js/rejection-signal.js/ams-policy.js (which fetch live over raw.githubusercontent.com before any clone exists), this reads the file from an ALREADY-CLONED repo on disk -- by the time a real attempt reaches this point, attempt-worktree.js's prepareAttemptWorktree (#5237/#5252) has already cloned it, so no extra network round trip is needed. KNOWN GAP, same discipline as this epic's other standalone pieces: not yet wired into attempt-cli.js's runAttempt, since that wiring needs a real repoPath from #5252's worktree preparation (open, CI-green, not yet merged as of this PR). Follow-up once it lands.
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
1 participant
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
packages/gittensory-miner/lib/worktree-allocator.js— given "its first real caller" in feat(miner): wire the attempt CLI subcommand's real dependencies (#5132) #5152 — only does SQLite slot bookkeeping (a bounded pool of fixedslot-Ndirectories) plusmkdirSync. It runs zero git commands.workingDirectoryhanded torunIterateLoopwould have been an empty, non-git directory: the coding agent driver would have nothing real to check out or edit, even with everything else in Wire CLI dispatch for the real attempt pipeline (attempt command) #5132's chain wired correctly.repo-clone.js: a per-repo base-clone cache. First use clones the target repo; every subsequent use doesgit fetch origin+git reset --hard origin/<baseBranch>, so an attempt always branches off fresh content, not a stale prior checkout (verified by a test: an uncommitted local edit in the cached clone is discarded on the nextensureRepoClonedcall, and a new upstream commit shows up).attempt-worktree.js, which composesrepo-clone.jswith@jsonbored/gittensory-engine'saddWorktree/removeWorktree/shouldRetainWorktreeprimitives (packages/gittensory-engine/src/miner/worktree-allocator.ts, feat(miner-hands): git-worktree-per-attempt isolation primitive #4269) — these already existed, are already tested, but were never called frompackages/gittensory-mineranywhere (confirmed viagit grep -n "addWorktree\|planWorktree" -- packages/gittensory-miner→ zero hits before this PR).prepareAttemptWorktreeis this package's first real caller of them;cleanupAttemptWorktreewires the engine's own retention policy (retain a failed attempt's worktree for post-mortem, remove a succeeded one's).execute-local-write.js'sgh pr createalready makes; never embeds a token in a clone URL.A real bug this caught
prepareAttemptWorktreeinitially forgot to forward its ownremoteUrl/runGittest-injection options through toensureRepoCloned— every call silently fell through to the realhttps://github.com/{owner}/{repo}.gitURL regardless of what the caller passed. The "REGRESSION" integration test (a real local git repo as the clone source, a realgit worktree add, asserting real repo content lands on a real branch) failed with a realremote: Repository not founderror and caught it immediately — before any review, and before this could have silently broken every attempt in production.Validation
npm run typechecknpm run test:coveragelocally (743 test files, 0 failures, 14,690 tests). 16 new tests across the two files — including real integration tests against a genuine local git repo (clone, fetch+reset with an upstream commit added between calls, a non-default base branch,git worktree addproducing real checked-out content on a real branch, cleanup retaining/removing per the engine's policy) plus DI-based unit tests for each failure path (clone/fetch/checkout/reset failures, missing stderr fallback messages).npx tsx scripts/check-engine-parity.tsnpm run build:miner+npm run test:miner-pack(added both new files to the hand-maintained check list)npm audit --audit-level=moderatenpm run test:workers/npm run build:mcp/npm run test:mcp-pack/npm run ui:*— skipped, nosrc/**,apps/**, or MCP-surface files touched.Safety
Notes
attempt-cli.js'srunAttempt— that's the final assembly step (separate follow-up, along with the coding-task-content-derivation piece this same investigation surfaced).