Repository navigation
feat(embed): add configurable workspace discovery - #1169
Conversation
📝 WalkthroughWalkthroughWorkspace discovery now supports configurable boundary policies, brace-expanded patterns, canonical path validation, and authoritative workspace YAML handling. The embedding facade re-exports related types and exposes workspace detection and package discovery helpers with tests. ChangesWorkspace discovery and embedding facade
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Embedder
participant EmbedFacade
participant WorkspaceDiscovery
participant Filesystem
Embedder->>EmbedFacade: discover_workspace_packages(project_dir, options)
EmbedFacade->>WorkspaceDiscovery: find_workspace_packages_with_options(project_dir, options)
WorkspaceDiscovery->>Filesystem: load workspace configuration and match patterns
Filesystem-->>WorkspaceDiscovery: candidate package paths
WorkspaceDiscovery->>Filesystem: canonicalize and validate paths
WorkspaceDiscovery-->>EmbedFacade: validated package paths or error
EmbedFacade-->>Embedder: package paths or report
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0c72216. Configure here.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThe PR adds configurable workspace discovery for embedders.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; confined discovery now checks canonical package paths against the canonical root and returns those validated paths, resolving both previously reported symlink issues. Important Files Changed
Reviews (4): Last reviewed commit: "test(embed): compare canonical workspace..." | Re-trigger Greptile |
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/aube-workspace/src/lib.rs (1)
489-613: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing confined-discovery test for negated patterns.
confined_discovery_rejects_parent_relative_patternsandconfined_discovery_validates_expanded_brace_alternativesonly cover positive patterns.validate_workspace_patternis applied identically to negated patterns (Line 127 runs before theif negatedbranch), but that path has no direct test — worth a quick regression test (e.g.packages:\n - 'packages/*'\n - '!../outside/**'\n) to lock in the behavior.🤖 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 `@crates/aube-workspace/src/lib.rs` around lines 489 - 613, Add a confined-discovery regression test alongside confined_discovery_rejects_parent_relative_patterns that includes a valid positive pattern and a negated parent-relative pattern such as !../outside/**. Call find_workspace_packages_with_options with WorkspaceDiscoveryOptions::confined_to_root(), assert Error::Parse, and verify the error reports that the pattern cannot escape the workspace root.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@crates/aube-workspace/src/lib.rs`:
- Around line 219-266: Bound brace expansion in expand_braces by enforcing a
finite limit on recursion/groups, total generated alternatives, or pattern size,
and propagate limit violations as Error::Parse through its callers rather than
silently truncating results. Preserve normal expansion behavior below the
configured threshold while ensuring attacker-controlled workspace patterns
cannot cause exponential memory/CPU use or unbounded recursion.
---
Nitpick comments:
In `@crates/aube-workspace/src/lib.rs`:
- Around line 489-613: Add a confined-discovery regression test alongside
confined_discovery_rejects_parent_relative_patterns that includes a valid
positive pattern and a negated parent-relative pattern such as !../outside/**.
Call find_workspace_packages_with_options with
WorkspaceDiscoveryOptions::confined_to_root(), assert Error::Parse, and verify
the error reports that the pattern cannot escape the workspace root.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: b0eeb79e-68e5-434e-b2d7-ae19dc5e63ba
📒 Files selected for processing (3)
crates/aube-workspace/src/lib.rscrates/aube/src/embed.rscrates/aube/tests/embed.rs
| fn expand_braces(pattern: &str) -> Vec<String> { | ||
| let mut depth = 0; | ||
| let mut open = None; | ||
| let mut commas = Vec::new(); | ||
| for (index, character) in pattern.char_indices() { | ||
| match character { | ||
| '{' => { | ||
| if depth == 0 { | ||
| open = Some(index); | ||
| commas.clear(); | ||
| } | ||
| depth += 1; | ||
| } | ||
| ',' if depth == 1 => commas.push(index), | ||
| '}' if depth > 0 => { | ||
| depth -= 1; | ||
| if depth == 0 { | ||
| let Some(open_index) = open else { | ||
| continue; | ||
| }; | ||
| if commas.is_empty() { | ||
| open = None; | ||
| continue; | ||
| } | ||
| let mut boundaries = Vec::with_capacity(commas.len() + 2); | ||
| boundaries.push(open_index + 1); | ||
| boundaries.extend(commas.iter().map(|comma| comma + 1)); | ||
| boundaries.push(index + 1); | ||
| return boundaries | ||
| .windows(2) | ||
| .flat_map(|window| { | ||
| let end = window[1] - 1; | ||
| let expanded = format!( | ||
| "{}{}{}", | ||
| &pattern[..open_index], | ||
| &pattern[window[0]..end], | ||
| &pattern[index + 1..] | ||
| ); | ||
| expand_braces(&expanded) | ||
| }) | ||
| .collect(); | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| vec![pattern.to_string()] | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unbounded brace expansion can blow up combinatorially / recurse deeply on attacker-controlled patterns.
expand_braces has no limit on the number of alternatives produced or on recursion depth. Sequential brace groups compound multiplicatively ({a,b}/{a,b}/{a,b}/... → 2^N results) and each group adds a recursive call, so a crafted pnpm-workspace.yaml/package.json#workspaces entry with many groups causes exponential memory/CPU use and/or a deep call stack. This directly undermines the security goal of WorkspaceBoundary::ConfinedToRoot, which exists specifically for "embedding hosts that discover projects inside a preselected repository" — i.e., repos whose contents (including this very config) the host doesn't necessarily trust.
Consider capping either the number of brace groups per pattern, the total expanded-alternative count (bail with Error::Parse past a threshold), or the pattern string length before expansion.
💡 Example guard
fn expand_braces(pattern: &str) -> Vec<String> {
+ const MAX_ALTERNATIVES: usize = 64;
let mut depth = 0;
let mut open = None;
let mut commas = Vec::new();
@@
return boundaries
.windows(2)
.flat_map(|window| {
let end = window[1] - 1;
let expanded = format!(
"{}{}{}",
&pattern[..open_index],
&pattern[window[0]..end],
&pattern[index + 1..]
);
expand_braces(&expanded)
})
- .collect();
+ .collect::<Vec<_>>();
}(Sketch only — the cap needs to be threaded through as a Result so callers can surface a validation error instead of silently truncating.)
🤖 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 `@crates/aube-workspace/src/lib.rs` around lines 219 - 266, Bound brace
expansion in expand_braces by enforcing a finite limit on recursion/groups,
total generated alternatives, or pattern size, and propagate limit violations as
Error::Parse through its callers rather than silently truncating results.
Preserve normal expansion behavior below the configured threshold while ensuring
attacker-controlled workspace patterns cannot cause exponential memory/CPU use
or unbounded recursion.
Summary
aube::embedConfinedToRootboundary for repository-scoped embedding hostsWhy
Embedding hosts such as mise need Aube’s workspace semantics without allowing a workspace pattern to scan outside a host-selected repository root. Previously they had to duplicate discovery or call lower-level crates with incompatible boundary behavior.
The default remains pnpm-compatible and continues to allow parent-relative patterns such as
../**; only hosts that explicitly select the confined option get the stricter policy.Stack
Validation
cargo test -p aube-workspacecargo test -p aube embed::testscargo test -p aube --test embedcargo clippy -p aube-workspace -p aube --all-targets -- -D warningscargo fmt --checkAI-assisted — Tool: Codex; model: openai/gpt-5; version: unavailable.
Note
Medium Risk
Confined discovery changes security-sensitive path resolution for embed hosts; the empty-YAML precedence tweak can alter which packages are discovered for projects with both files.
Overview
Workspace discovery gains
find_workspace_packages_with_optionswithWorkspaceBoundary::ConfinedToRoot, which rejects parent-relative or absolute globs, canonicalizes matched packages, and errors on symlinks that resolve outside the workspace root; the default remains pnpm-compatible (../**still allowed).Discovery now expands brace alternates in positive and negative patterns (e.g.
{apps,packages}/*), and treats an on-disk workspace YAML as authoritative even whenpackages:is empty, sopackage.json#workspacesis not used as a fallback in that case.The
aube::embedfacade exposesis_workspace_project_root,discover_workspace_packages, and re-exportsWorkspaceDiscoveryOptions,PackageJson, and related manifest types for embedding hosts (e.g. repository-scoped tooling).Reviewed by Cursor Bugbot for commit 0ab2087. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes