(MOT-4156) feat(fp,harness,console): scoped pipe steps under the harness fs_scope + trigger visualization - #559
Merged
Merged
Conversation
fp::pipe steps dispatch under the fp worker's authority, so shell::*/ coder::* steps never received the per-session filesystem scope and had to be refused outright. Stamp the same trusted fs_scope object onto fp::pipe call args (is_stamped_function beside is_scoped_function); fp forwards it per scoped step. All six inject() call sites — turn loop pre/post-hook, function_trigger, deferred approval release — ride the one guard change. filesystem_boundary() probes fp::pipe as a representative scoped call (shell::exec) so pipe steps get the same jail a direct call would — the access watch binds shell::*/coder::* only, and without the probe a pipe would run under the wider configured_roots boundary wherever approval-gate is live. The watch itself deliberately stays unbound from fp::pipe: a grant release re-invokes the whole pipe, re-running completed steps.
Lift the static shell::*/coder::* refusal: scoped steps now ride the harness-stamped filesystem scope, closing the biggest token sink the pipe could not cover (coder::search → transform → state::set without the results ever entering the chat). Fail-closed trust chain: - PipeRequest.fs_scope (schemars-skipped) captures the harness stamp; the harness overwrites or strips anything model-supplied at that key. - validate() refuses scoped steps when no stamp arrived (cron and worker-to-worker callers, sessions without a working directory) and statically refuses `into` pointers targeting /fs_scope. - bus_step_args() re-stamps each scoped step as the LAST write before dispatch — after `into` threading — as a full key replace, so neither an authored payload.fs_scope nor a value threaded to /fs_scope survives. Non-scoped steps pass through untouched; the scope never leaks. is_scoped_step mirrors harness filesystem_scope::is_scoped_function byte-for-byte (divergence would open a stamp/strip gap). Permissions comments now spell out that allowlisting fp::pipe grants un-approved shell/coder execution — the pipe call stays the approval surface. Live-verified: search→map→uniq→set (3225ch → 360ch stored, receipt only), exec→get→split→when→set anchored in the session root, forged scopes overwritten, unstamped callers refused.
The top live failure (hit twice in one session): a producer's OBJECT
response (coder::search {content_matches,…}, shell::fs::grep {matches,…})
piped straight into an array/string transform, answered by a bare
"map needs an array value (got object)". One shared needs() helper now
names the object's top-level keys and the fp::get fix across all fifteen
collection transforms, so a failed step self-corrects in one round trip —
fp::get's miss message was already the model for this.
Guidance tightened from the same evidence: the recipe it taught
(coder::search → fp::map) was itself the trap — it now includes the
fp::get select step; transform arg signatures ride along ("no schema
lookup needed", one live session burned ~30k tokens on
engine::functions::info dumps before its first pipe); producer response
shapes (search/grep/exec/fetch) are named so the right field is selected
first try.
…ions Two register_trigger card gaps, both of which fell through to raw JSON: - cron bindings: the WHEN pane now reads "at 17:00 on Jul 21" beside an expression chip. describeCron() covers the common shapes (fixed time, */N steps, hourly, weekday lists, day-of-month/month; 5-field classic and the seconds-first 6/7-field form) and returns null for anything a translation would get wrong — ranges, L/#, dom+dow OR semantics, pinned years — which keep the raw expression. The config pane is suppressed when the config is only the expression. - call-mode reactions (metadata.call, e.g. turn-completed → fp::pipe): reactSpecSchema now models CallSpec (model/task optional — the view decides the mode, and metadata with neither call nor task still falls back to raw). The THEN pane names the binding's real effect (CALL fp::pipe + event-into chip) and renders a PIPELINE pane via PipeStepList, extracted from the fp::pipe chat card for reuse; non-pipe call targets get a compact payload pane. Storybook fixture engineRegisterTriggerCallPipe mirrors the live binding.
Local path-dep versions (harness 1.4.3, iii-helpers 1.3.0) moved in earlier release commits without regenerating dependents' lockfiles; cargo refreshed them on the next build here. No dependency changes.
A call-mode harness::react payload is model-authored at registration time and dispatches OUTSIDE the turn loop, so it never passed the trusted fs_scope stamp path — a registered reaction could carry an arbitrary root/grants to shell::*/coder::* directly, or to fp::pipe (which trusts its top-level scope as the harness stamp). Strip the key from stamped targets after event injection (so a scope threaded via event_into cannot survive either) using the existing fail-closed inject(root=None) path. Scoped calls from reactions are now refused downstream until a trusted per-reaction scope source exists (stamping the registering session's scope is possible follow-up). The direct-shell variant of this hole pre-dates the pipe feature.
shell::config-status (agent-policy hard-denied — can surface operator paths) and shell::workspace::* (console picker plumbing) sit inside the scoped shell::* prefix, so the fs_scope stamp gate would have admitted them as pipe steps under fp worker authority. Both ignore fs_scope entirely; refuse them statically in forbidden_step, per the rule that every agent-policy hard-denied class must be re-refused in a pipe. Also formats pipe.rs/util.rs — cargo fmt --check gates the per-worker CI.
…cope describeCron corrections: - Seconds are honored, not ignored: every description except "every second" speaks at minute granularity, so wild or nonzero seconds now return null instead of claiming "at 17:00" for a schedule that fires sixty times in that minute. - Weekday ordinals follow the dialect: the seconds-first form is the Rust cron crate's Quartz-style numbering (1=Sun..7=Sat, verified against cron-0.12.1 DaysOfWeek — 0 invalid), while classic five-field cron stays 0=Sun..6=Sat with 7 as Sunday. Numeric weekdays previously rendered one day late in the Rust dialect. PipeView now surfaces the harness-stamped fs_scope (root chip + grants count) — the pipe call is the approval surface, and the custom view replaces the raw request pane, so an approver reviewing relative shell/coder steps could not previously see where they would run.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
skill-check — worker0 verified, 47 skipped (no docs/).
Four for four. Nicely done. |
andersonleal
changed the title
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (15)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
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.
What
Lets
shell::*/coder::*run asfp::pipesteps under the session's trusted filesystem scope — closing the biggest token sink the pipe could not cover (coder::search → fp::get → fp::map → state::setwith the results never entering the chat) — plus the console rendering to make cron and pipe-reaction bindings legible.How
Scope forwarding (fail-closed at every hop)
fs_scopeit puts on direct shell/coder calls ontofp::pipecall args (is_stamped_function); all sixinject()sites ride the one guard change, including approval release.filesystem_boundary()probesfp::pipeas a representative scoped call so pipe steps get the same jail as direct calls.bus_step_args, afterintothreading, full key replace) — neither an authoredpayload.fs_scopenor a value threaded to/fs_scopesurvives. No stamp (cron, worker-to-worker, no working directory) → scoped steps refused with a teachable error.intopointers targeting/fs_scopeare statically refused.harness::reactcall dispatch strips authored scopes from stamped targets (a registered reaction dispatches outside the turn loop; the direct-shell variant of that hole pre-dated this branch).shell::config-status,shell::workspace::*) stay statically forbidden as steps.fp usability (from live-session evidence)
fp::getfix — the top live failure was piping a producer's object response straight into an array transform.engine::functions::infodumps), and names producer response shapes.Console
describeCroncovers common shapes and returns null (raw expression) for anything a translation would get wrong — ranges,L/#, dom+dow OR semantics, wild/nonzero seconds, and dialect-correct weekday ordinals (Rust cron crate is Quartz-style 1=Sun, verified againstcron-0.12.1).metadata.call→fp::pipe): THEN pane names the real target and renders the pipeline step route (PipeStepList, shared with the fp::pipe chat card).PipeViewsurfaces the stamped scope so approvers see where relative steps anchor.Security posture
The pipe call remains the single
needs_approvalsurface; the approver seesthrough[]plus the harness-stamped scope (stamped before hooks). Permissions comments now state that allowlistingfp::pipegrants un-approved shell/coder execution. Documented residual: a direct bus caller can still hand-craft a scope — but such a caller can already call shell directly; shell-side caller verification is tracked follow-up.Verified
cargo test+ clippy + fmt clean: harness (245), fp (34); console web: 1052 tests, typecheck clean.coder::search → fp::map → fp::uniq → state::set(3225ch → 360ch stored, receipt only),shell::exec → fp::get /stdout → fp::split → fp::when → state::setanchored in the session root, forged payload scopes overwritten,into:"/fs_scope"statically refused, unstamped callers fail closed.