chore(sessions): don't allow packages to request user data for the session - #993
Conversation
📝 WalkthroughWalkthroughPackage-supplied patches and user-data variables are filtered from contributions before session composition. Tests verify that static package variables and equivalent entries from other sources remain available. ChangesPackage contribution filtering
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/sessions/src/daemon/composer.rs (1)
156-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a composer-level regression test for this filtering boundary.
The helper tests validate the predicate, but no test shown here proves that
SessionComposer::composeinvokes it before either the fast path or pending transformation. Assert that package patches and user-data vars are absent from the resulting response/state while eligible static and non-package entries remain.🤖 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/sessions/src/daemon/composer.rs` around lines 156 - 164, Add a regression test around SessionComposer::compose that supplies package patches and user-data vars alongside eligible static and non-package entries. Assert the composed response/state excludes the package-supplied data while retaining eligible entries, covering both the fast path and pending transformation boundary as applicable.
🤖 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.
Nitpick comments:
In `@crates/sessions/src/daemon/composer.rs`:
- Around line 156-164: Add a regression test around SessionComposer::compose
that supplies package patches and user-data vars alongside eligible static and
non-package entries. Assert the composed response/state excludes the
package-supplied data while retaining eligible entries, covering both the fast
path and pending transformation boundary as applicable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb9af8bd-004d-4518-b345-8d8ba25fb284
📒 Files selected for processing (2)
crates/sessions/src/core/compose.rscrates/sessions/src/daemon/composer.rs
Summary
Removed the ability for packages to request user data on session creation including file patches and inherited environment variables.
Testing
Verified that my claude config was not uploaded when creating a session with claude-code
Note
Strip package-supplied patches and user data vars from sessions before composition
Contribution::drop_package_supplied_patches_and_user_data_vars()in compose.rs that removes package-sourced patches and package-sourced vars carrying user data from aContributionin-place.SessionComposer::composein composer.rs now calls this method before destructuring and gating, so packages can no longer inject patches or user data vars into the final session composition or the pending client state.Macroscope summarized 49c3545.
Summary by CodeRabbit