fix: enforce descendant permissions for recursive file mutations #40
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "t3/filesystem-recursive-move-permissions"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Recursive file operations checked only their directory endpoints, so a writable folder could carry restricted descendants across permission boundaries. Enforce descendant permissions under ADRs 0015 and 0034 before moving, removing, purging, or restoring a stored subtree.
What's in
Tests
Browser verification and limits
Permission and Rename behavior were exercised through the real file-system facade, provider fixture, browser read models and rendered bUnit components. A separate browser session was skipped because the affected live fixture mount is absent from the production sample world. No screenshots are attached.
A separate pre-existing side effect remains: a stale move into an enabled but unmaterialized stored mount can create its empty mount row before returning Stale. The reviewer reproduced this on the base code too. It is outside this descendant-permission fix; the permission-refusal tests do not claim universal stale-operation atomicity.
No schema, seed or submodule changes.
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 95.6%
Shikigami.Capabilities.Contracts - 97.8%
Shikigami.Capabilities.TestFixture - 97.9%
Shikigami.Domain - 98.1%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 97.2%
on
ration
shot
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Pattern_0
Shikigami.Kernel - 90.9%
Shikigami.Server - 100%
Shikigami.UseCases - 95.7%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_12
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_11
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__HexColour_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IdPattern_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IntervalBody_10
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_5
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NewerXY_8
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Number_7
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_9
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_2
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__ToolPattern_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_4
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A permission fix — my favorite kind~ ♡ The bug was deliciously sneaky: recursive operations only patted the directory endpoint on the head and never asked the children underneath. A writable folder could smuggle restricted descendants straight across the permission boundary like a book hidden in a coat pocket. Not on my watch~ ♪
And yes, I noticed
41a3ab9is an empty retrigger commit — the tree is byte-identical tof49b0bb, which I probed thoroughly last time (my earlier comment fell victim to a tool hiccup, so this replaces it). Every finding below was re-verified against this head.Verdict: ✅ Looks good to me~
✅ What I liked~
CheckSourcesover the unfiltered stored tree (FileSystem.cs:583) — this is the heart of the fix and it's done right. The comment even names the trap: a hidden or restricted child must refuse the whole operation, never silently vanish from the preflight. Iterating rawListTreeAsyncrows instead of a permission-filtered listing is exactly what closes the hole. fufu~ beautiful~RemoveAsync(trash and purge branches) andCheckMoveAsync(moves and restores) all share the same source preflight, and the per-row target check composes the sameWritable ?? Taken ?? Placeabletriple the root-level check uses. Consistency makes me so happy~ ♡RemoveAsyncordering is correct: preflight runs after the version check but before the trash grows any directories — the tests assert rows, revisions and notifications stay untouched after a refusal. No half-mutated state escapes~FileSystem.cs:275–279), not a dodge:StatAsyncfirst (soNotFoundpropagates properly), then refuse every live directory — empty ones and owner ops included — because the expected-version contract can't snapshot a stable subtree through the provider's mutation boundary. Documented three ways: ADR 0034 amendment,IMovableCapabilityMountXML docs, and the browser flag. The stale-mount side effect you disclosed is pre-existing on base and fairly out of scope — I read the note, I accept the boundary~CanMove = !IsReadOnly && (!IsDirectory || StoredMount)mirrors the kernel exactly — stored dirs movable, live files movable, live dirs refuse withUnsupported. No lying buttons in the UI~MoveCalls == 0proving the provider is never even invoked on refused directory moves. 17 cases, every new branch accounted for.Mutation probes (run against the identical
f49b0bbtree, 3 run / 0 survived): deletingCheckSourcesfromRemoveAsync→ 4/4 tests fail; deleting the destinationWritable/Taken/Placeablepreflight → 4/4 fail; deleting the liveIsDirectoryguard → 6/6 fail. Nothing survived. The tests guard this code for real~ ♡Fufu~ a security fix with a documented contract boundary, aligned UI, and mutation-proof tests. This is the good stuff. Merge it before I change my mind~ ♪
Automated review by Jibril · 2026-10-22
CI: green for head SHA
41a3ab9(build / image / test) · Mutation probes: skipped this run — code+tests unchanged sincef49b0bb(3 run, 0 survived there) · Local suite: skipped (CI green)