Repository navigation
Reject canonical escapes in private storage policies - #3834
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
debt-src at 0442fd0, every finding: |
This was referenced Nov 1, 2026
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
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
2 files, +112/-11. Merged at
e26a1897b1c8eff5fa76f06da96e764e10b02179; independent Core isolation controls passed 14/14 in 1.60s (rc0). Installed in the shared main-only update; combined frontend/runtime and affected live acceptance remain open. Fix the private-policy counterexample whereroot/checkoutspoints outside the verification root and both the stored rule and its projection canonicalize to that same escaping path.The storage policy now owns both the private directory mapping and the pure containment query. No frontend path roster, second Comms, new store, environment setting, fallback or policy migration.
Before → after
Before:
StorageRule.withinsupplied four path overrides separately from their field declarations.checkout_base.__post_init__resolved a symlink, so equality of two rules could agree outside the intended root.After:
ScopedDirectory(ValueDirective[Path])extends the existing field-directive mechanism. Each scoped field declares its own private relative location.FieldCodec.field_planssupplies discovery; adding another scoped field needs no separate roster edit.StorageRule.withinderives its replacements from those declarations. The four-location keyword table is deleted.StorageRule.require_isolated(root) -> Noneis the side-effect-free observation contract. Existing preparation consumes it and preserves existing private limits and stored bytes on refusal.initial_rootsuses the same directory behavior, so a future default allocation cannot follow an escaping child link under its scratch base.The existing FieldCodec wire fields and defaults are unchanged. No policy file is rewritten to manufacture isolation.
/procremains an unscoped read source; storage limits and global HOME-dependent defaults keep their existing owners.Performance retains the paired #1393 consumer change: consume the actual already-observing service's
registry.storage_rule, then callrequire_isolated, without opening Comms or writing during observation. Candidate02 does not contain this API and is not retrofitted or qualified by this source checkpoint.Source evidence
Baseline
e87abcc3f3cb5cc2df4407e363698a30ea643902; allocated owned checkout reused, clean tracked baseline, pushed branch verified remote=HEAD. Open-PR file check found no overlapping storage-rule/test edit at allocation.Retained NRA query over current Core/owned Toad src/tests/tools plus the actual physical consumer tools: 1,898 file retrieval corpus, 75 selected modules, 961 original ClassDefs, 929 matching canonical projections, 32 OPEN, 2,024 source sites, zero parse omissions. It reuses the existing applied-map projections only on exact source-hash/line/name matches; changed, local, ambiguous and external cases stay OPEN. This is lexical ownership evidence, not runtime binding or global equivalence proof. Original-field declarations, assignments/checks/imports and allocation/preparation consumers are retained in
storage-canonical-source-map.json.The first source query hit its90s INT bound in repeated ast.get_source_segment line splitting. Its failure is retained. The corrected query reused NRA's traversal and indexed source lines, completed0; the raw second log's CANONICAL_MATCHED938 includes nine retained OPEN rows, so the counts above are derived from actual row statuses, not that label.
This is an authored owner repair, not an NRA-admitted transformation claim or an expansion of the separately stopped
-infproof work.Verification so far
Source/import/codec command completed0, using existing read-only dependencies and
PYTHONPATHpointing only at this allocated Core source:/proc;ScopedDirectory → ValueDirective → FieldDirective → Generic → objectMRO;Receipt:
storage-canonical-source-codec.{log,rc}. Targeted NRA syntax reproof of the two changed modules is recorded separately; it is not behavioral equivalence.Eleven new filesystem behavior cases passed within the 14-case selection: the three declared child-directory roles cover pre-write outward refusal, stored/reopened-policy refusal without byte changes, and inward-link preservation; plus future-child link refusal and new-field declaration derivation. Existing durable-policy/private-limit and foreign-policy no-rewrite controls remain, with the existing actual CheckRoot/reopen/future-child/PiRedeal publication control.
Completed bounded Core selection, run serially by pi-commander under the existing test lease on exact head
0c6633f85f408b15adcb65976f1fc7f70928975f:tests/test_storage_rules.py::TestIsolatedDirectoriestests/test_storage_rules.py::test_future_child_cannot_follow_an_escaping_scratch_linktests/test_storage_rules.py::test_private_projection_uses_new_field_declarations_without_another_rostertest_isolated_policy_is_durable_and_preserves_existing_private_limitstest_isolated_preparation_refuses_an_existing_foreign_policy_without_rewritingtests/test_check_root.py::test_check_roots_contain_new_child_allocations_and_actual_rule_publicationEvidence read back:
/home/ts/.cache/agent-scratch/pi-commander/heartbeat-publication/isolation-3834-controls-1.logand.rc: exact source/import path and HEAD printed, 14 passed, 2 warnings in 1.60s, rc0. Pi-commander reports the serial, leased selection above (9 directory cases + 5 individual cases); the retained log independently confirms the imported storage-rule path, exact HEAD and completed result. No reviewer rerun. Forge merge readback retains the exact tested head and merge commit.This establishes the independent storage owner and actual CheckRoot/reopen/default-child/PiRedeal control, not the combined #3825/#3834/#1393 runtime consumer or physical journey. Actual paired frontend qualification, fresh combined-runtime admission and affected native/CLI/channel acceptance remain open. The independent main-only shared installation is recorded below. No full suite, local NRA gate, candidate/provider/physical launch or installer was run for this PR. Failed roots and Candidate02 remain unchanged.
Shared installation (not full live acceptance)
The shared installer completed the main-only update to
e26a1897b1c8eff5fa76f06da96e764e10b02179: 93 phases in 70.0s, rc0, no pending carries,reconcile-storeschanged 0, 792 scanned files identical, 2,881 thread records read back. Its retained result is/home/ts/.cache/agent-scratch/pi-commander/heartbeat-publication/main-only-live-install-1.{log,rc}. Protected old Toads were not signalled.Independent readback confirmed
currentresolves to/home/ts/.local/share/agent-comms/runtimes/core-e26a1897b-lock-b7cd09e875fefab3; itsbuild.jsonrecords the exact Core commit and Toadec224a6c4d8bb510502a46a75886726f0969707c(also confirmed in Toad's installeddirect_url.json). The installedstorage_rules.py:313–394containsScopedDirectory, declaration-derivedwithin, purerequire_isolatedand future-child containment. The install source tree had already retired; no attempt was made to reuse or recreate it.The initial readback above established installed source, not behavior. A subsequent bounded run used that exact installed interpreter with
-I -Band the productionStorageRuleAPI on fresh owned filesystem paths: rc0, four cases completed. An outward checkout link refused before policy creation; a stored/reopened policy refused a new outward link without changing its SHA256; an inward link prepared and reopened successfully; a future-child scratch link refused. The external marker hash and its directory contents stayed unchanged. Receipt:/home/ts/.cache/agent-scratch/pi-toad-errors/installed-isolation-e26-01.{log,rc}. Run paths and the one-shot were retired after exit and result retention.That is actual installed storage-API behavior, not an end-to-end CheckRoot/frontend/native/CLI/channel journey or expanded Toad#1389 acceptance. No pytest, Comms root, model, native, physical or candidate launch was performed; the existing 14-case control remains separate source-head evidence. The #3825/#1393 combined qualification remains open.
Separately, #3831 integration exposed an enclosing-Git versus registered-scratch write-projection failure in the existing
homedregression. Pi-toad-errors owns its source-only follow-up with pi-commander retaining reruns. The containment API pass does not clear that different failure.Assumed: the selected filesystem is stable through each checked operation. These are point-in-time canonical-containment checks, not a hostile-filesystem atomic sandbox or an attestation of arbitrary manually edited registry rows. Original sessions, failed/UNKNOWN roots, external policies and peer/live trees were not rewritten.