fix(#4716): a submission inbox on a public partition — the grant shape both permission folds agree on, and a boot heal that stopped deleting it

[rbuergi]

Refs #4716 (sev:H, area:security). The half #4969 deliberately left open: the policy on the Feedback partition, and the fold asymmetry — which is real, and is now confirmed in both directions rather than asserted in one. #4969 documents and falsifies; this closes it, and does not touch its files' hunks.

1. What Feedback/_Policy declares — shipped, and live

Shipped: nothing. There is no _Policy node anywhere in the Feedback package tree (MeshWeaver.Plugins/Feedback/). The node is MINTED at install time by PackageInstaller.EnsureDeclaredAccess, and the manifest is the whole declaration:

{ "preInstalled": true, "protectedSegments": ["_Submissions"] }

Feedback is the only package in the fleet that declares protectedSegments (one hit across MeshWeaver.Plugins/*/index.json).

Live on memex.meshweaver.cloud, read-only, sampled twice minutes apart — identical both times:

Read	Result
Feedback/_Policy	{"$type":"PartitionAccessPolicy"} — no publicRead · v198207 · lastModified 2026-09-20T22:32:06.197014Z · system-security
Feedback/_Access/{Public,Anonymous}_Access	Public — Viewer / Anonymous — Viewer — root role GRANTS, from 2026-07-24
Feedback/_Submissions/_Access/Public_Access	Public — Viewer DENIED (gated) · version 1 · created 2026-09-20T22:32:07.829904Z
Feedback/_Submissions/_Access/Anonymous_Access	same · created 2026-09-20T22:32:08.001704Z
Feedback/_Submissions children	one node, demo-preview (v1, 2026-07-21) — the demo item

And on the CONTROL instance memex.systemorph.com, read-only, the same day: Feedback/_Policy also carries no publicRead (v184, lastModified 2026-09-21T03:27:26Z, system-security), and that partition holds no _Submissions at all — no inbox, no denies. That second reading is what caught a widening in my own first attempt; see §4.

So the live state moved between #4969's reading and mine. #4969 measured publicRead: true at v198193 on 2026-09-20; at 22:32:06Z that same day the policy was rewritten without it and the _Submissions deny pair was created 1.6 s later — the write order and the node names are PluginGate.SeedGating's pre-installed arm exactly ("close, then open"). The exposure is therefore not open right now, and the inbox still holds only the demo item, so nothing a user wrote was ever public: latent, and now also closed — by live state alone.

2. Is the exposure real? Yes — measured, in both folds

C# fold — executed. AnonymousCannotReadAProtectedSubmissionTest (new, test/MeshWeaver.Graph.Test/, real monolith mesh, ConfigureMeshBase so no Public Admin) seeds the two shapes side by side, differing in ONE node:

• OpenPkg — _Policy { PublicRead = true } + the declared deny pair on the inbox satellite. Anonymous and Public both READ OpenPkg/_Submissions/theirs. The protection is present, intact, and does nothing.
• GatedPkg — root Public+Anonymous Viewer GRANTS + _Policy { PublicRead = false } + the same deny pair. Both are DENIED, the cover stays public, and the reviewer and System keep reading.

Each half carries its own public-half control, so a denial cannot pass for the right answer on a partition nobody could read. 5 cases, all passing.

SQL fold — NOT executed here, and it cannot be: core has no Postgres test lane at all (no Testcontainers/Npgsql reference in any core test .csproj; the one declared version is consumed by nothing). What replaces the measurement is executable SQL plus tests that already run in MeshWeaver.Plugins:

• PostgreSqlSchemaInitializer projects a PublicRead policy as allow-Read rows for Public/Anonymous at the policy's prefix — the same row shape a root grant produces.
• The read side resolves DISTINCT ON (user_id) … ORDER BY LENGTH(node_path_prefix) DESC in three independent emitters (PostgreSqlSqlGenerator.BuildPerSchemaAccessClause, GenerateAccessControlClause, public.search_across_schemas), so a deny row at a longer prefix wins.
• AccessControlQueryTests.PaywalledContent_StaysInvisibleToAnonymous and PerSubjectAccessFoldTests pin that deeper-deny-beats-shallower-public-grant behaviour against a real Postgres.

So #4969's "the SQL fold says it does" is not only its author's comment — the read-side half is executable and covered. The folds genuinely disagree, which is the paywall-bypass shape: readable by exact path, correctly absent from every listing.

3. The two root causes, both in core

(a) protectedSegments was dead metadata. NodeRepoPackageSource.Peek dropped it — the same defect class preInstalled, publicSegments and contactEmail each had (#920) — while the Store's PluginGate authored and honoured it in mesh source no dotnet build or grep --include='*.cs' over this repo can see. So the partition took the fully-public shape, under which the Store's protection protects nothing on the C# path.

(b) 🚨 The boot heal DELETES that protection. EnsurePartitionPublicRead's legacy heal reads "a policy withholding public read + Public/Anonymous denies on the children" as pre-#902 damage: it retires the denies and rewrites the policy to PublicRead. The live _Submissions deny pair matches that fingerprint exactly, and Feedback is pre-installed, so the heal's restriction does not save it. EnsureDeclaredAccess runs on every install path and as a boot re-assert (InstalledPackageRepairService). The live shape above is one boot repair pass away from being destroyed, logged at Information as a legacy migration. That is what this PR fixes, and nothing in the repo reproduced it before.

4. The fix

• PackageManifest.ProtectedSegments, read by Peek — core can see the declaration.
• A partition declaring one is published through root Public+Anonymous Viewer GRANTS, a deny on each declared segment, and a _Policy that withholds PublicRead. Grants and denies are ROLE rows, which both folds resolve by the same per-subject longest-prefix rule — so the shape cannot answer two ways. Takes precedence over both older branches, pre-installed included. Denies first, policy second, root grants last, so an interrupted run leaves the PUBLIC half dark rather than the segment open.
• The heal's fingerprint is narrowed BY CONSTRUCTION: a deny at an _ satellite scope is never retired, because GatedChildRoots skips every _ segment and so no version of this installer could have written one. The partition's own _Access container is deliberately still in scope — a deny there gates the root, which IS the shape the heal exists to undo.
• An EVIDENCE arm, for the live fleet. The repair pass drives the re-assert from the install RECORD's manifest, and every record stamped before the field was read carries no declaration — so a declaration-only fix would leave every already-installed portal republishing its inbox on the next boot, and the policy write alone is enough to do it, no delete involved. A satellite deny this installer cannot have written therefore takes the partition off the blanket policy even when the manifest says nothing.
• The two rules compose: genuine pre-memex: platform Agents catalog is GONE — 'Agent' is a ghost partition root (exists to create, 404s to read, no version history); preInstalled is dead code #902 damage on ordinary children is still retired (pre-installed only — the restriction that keeps core out of the gating reconcile's ping-pong).
• 🚨 The shape MOVES a publication off the policy; it never ADDS one. The denies are written on every declaring partition (a deny can only narrow). The policy flip and the root grants happen only where this step would have published anyway: no policy yet, a policy declaring PublicRead, or the legacy fingerprint the heal opens on. My first attempt got this wrong, and the control instance's copy is what showed it: with Feedback/_Policy withholding public read there and no inbox present, the declaration would have added root Public+Anonymous grants and published a partition somebody had closed — a widening introduced by the fix for an exposure, which main does not do (create-only leaves such a policy alone). ADeclaredProtectedSegment_OnAPartitionThatWithholdsPublicRead_IsGatedButNotOpened pins the rule.

The submit flow is untouched. Feedback/Feedback/Source/FeedbackSubmitter.cs writes under ImpersonateAsSystem(), and System short-circuits the fold — asserted, not assumed (TheGrantShape_LeavesTheReviewerAndSystemReadingTheInbox). Nothing in the policy made the partition writable: the two well-known subjects hold Viewer, which is Read only.

No ping-pong with the Store. PluginGate writes OpenPolicy (publicRead: true) only when PolicyMatches — i.e. the current policy is a gating one (publicRead off and redirectOnDenied). The policy this writes carries no redirect, so PolicyMatches is false and OpenPolicy is never queued. Core now writes and preserves exactly the shape the Store's gate converged on live.

5. The control

AnUndeclaredButProtectedSegment_IsNotRetiredAndNotRepublishedByTheHeal reproduces the live shape (policy withholding PublicRead + the satellite deny pair) with a manifest that declares nothing, plus a deny on an ordinary child as the control on the other side of the split. With the behaviour reverted in place (the satellite split and the declaration routing disabled, field kept so the tests compile), it fails on the assertion that names the delete:

Expected value not to be <null> because THE assertion: a deny on an `_` satellite is outside the
legacy fingerprint BY CONSTRUCTION … retiring it deletes security state a boot pass had no
business touching (MeshWeaver#4716)

and the declaration arm fails on Expected value to be false … but found True (the policy it must not write). A third control — forcing publish true — fails the create-only arm on ClosedPkg/_Access/Public_Access, the grant it must not write. All green with the fix restored. The ordinary-child deny is still retired — without that the narrowing could have been a blanket "never retire anything" wearing the right answer's clothes; it is also what caught my first attempt, where the evidence arm shadowed the heal entirely.

6. Verified locally

dotnet build -c Release -warnaserror, one project per invocation, each 0 Warning(s) 0 Error(s): MeshWeaver.PluginCatalog, MeshWeaver.Documentation, Memex.Portal.Shared, Memex.Portal.Shared.Test, MeshWeaver.Graph.Test, MeshWeaver.Documentation.Test, MeshWeaver.Layout.Test, MeshWeaver.Compiler.Pipeline.Test (every project referencing the two changed ones).

Tests: the whole Memex.Portal.Shared.Test project — the suite that owns EnsureDeclaredAccess — 1761 total, 0 failed; ADeclaredProtectedSegmentSurvivesTheBootHealTest + ProtectedSegmentDiscriminatorTest 18/18; the access-control regression PublicReadPolicyScopeTest + ShippedAccessGrantsTest + AccessAssignmentGuardTest + the new file 69/69; the doc guards DocumentationLinkIntegrityTest, DocumentationFrontMatterGuard, DocumentationEmbedIntegrityTest, ArchitectureTopicMapTest 17/17.

7. What I did NOT establish

• No Postgres case was executed — see §2. In particular no test anywhere writes a real PublicRead = true _Policy at a shallow namespace together with a real deeper deny against Postgres: the Plugins tests that cover the shape seed the rows through access_control, which simulates the projection rather than exercising it. That pairing belongs in the repo that has the lane and is named in the doc page as the open gap.
• Whether a grant-published partition keeps the same public.partition_access membership for an anonymous fan-out as a PublicRead-published one. The projection derives those rows from user_effective_permissions where permission='Read' AND is_allow=true, which the root grants satisfy, and the live Feedback partition has been in the grant shape since 2026-09-20 — but that is a read of the SQL plus a live observation, not a measurement.
• Who wrote the live policy at 2026-09-20T22:32:06Z. The write order and node names match PluginGate.SeedGating's pre-installed arm, and lastModifiedBy is system-security, but I did not confirm the provenance and did not look for an unrelated live mutation.
• A separate divergence I found and did not fix (reported, not touched): in PostgreSqlSchemaInitializer, the BULK rebuild's policy-cap fold emits deny rows only for subjects already in the shadow table (CROSS JOIN (SELECT DISTINCT user_id FROM user_effective_permissions_shadow)), and Public/Anonymous are injected later — so in a partition whose only read surface is a _Policy, a deeper _Policy { Read = false } may emit no deny row for Public at all, while the per-user rebuild function's equivalent fold is unconditional. That contradicts AccessControl.md's own table row. It is a read of the SQL, nothing executed, in another repo — filed rather than folded in here.
• I did not touch a live policy, on either portal. Every mesh call against them was get/search. The one live WRITE is the bug-triage filing above, through the /feedback skill on the control instance — a direct create under Feedback/_Submissions there is refused (Create permission required), which is the submit-as-System design working as intended; whether the agent filed it New rather than Draft I had not confirmed at the time of writing.

No addresses need recycling for this change: it alters src/ code that runs in the install and boot-repair pipelines, not a NodeType's source or node content a per-node hub binds once. The live Feedback partition needs no write — the fix's purpose is to stop the next boot from changing it.

No public surface removed, no interface members added without a default, no i18n keys, no PackageVersion change — so no Pairs-with:, Implementers:, Mirror-sync: or Satellite-pins: declaration applies. PackageManifest.ProtectedSegments is an ADDED member with a default, on a record no sibling repo implements.

8. Review round

All eight automatic-review findings answered on their threads; seven accepted and fixed, one declined with reasoning. Three were defects in the change itself and two of those were security-relevant, so they are worth naming here rather than leaving in the thread:

Finding	Verdict
DeclaredAccessMarker still named _Policy for a pre-installed manifest declaring BOTH publicSegments and protectedSegments — which routes to the scoped shape and writes no policy	Real. VerifyDeclaredAccess would report a CORRECT install as a failure. The marker now mirrors the branch predicate rather than restating it — the rule its own remarks state, which I wrote and then walked into
The write order was not fail-closed with an existing PublicRead = true policy: denies first, then the flip, so a failure in between leaves the blanket grant live and the fresh denies inert	Real, and the remarks already claimed the property the order did not deliver. Now policy → denies → root grants: the flip removes the grant while no root grant exists, so the partition is momentarily CLOSED and a failure anywhere leaves it closed. I did not take the rest of the suggestion (an atomic transition / a staged fail-closed state) — that is a new permission primitive, and the reorder gets fail-closed with no new concept
CreateOnly decided by PRESENCE, so a Public/Anonymous assignment at a declared-protected scope that happened to be a grant survived and the root grant then published the segment	Real — the one way the shape could be defeated silently. The protected-segment denies now supersede a non-deny for those two subjects (the Store gate's StillNeedsDeny predicate). Narrowing only, and deliberately not applied to the scoped shape's ordinary-child gating
Both evidence decisions consumed the query's first emission rather than its Initial snapshot	Real, and it fails OPEN: a pre-Initial frame omitting the satellite deny leaves the protected set empty and the heal proceeds to retire denies and write PublicRead. Filtered for Initial (the idiom at 8 sites here). The legacy retire had been taking a delete decision off the same unfiltered read
Nothing covered the index.json → Peek → manifest hop	Real. AProtectedSegmentDeclarationSurvivesDiscoveryTest covers it through the real ListPackages with an injected fetch; disabling the read fails it, and a manifest without the key still comes back empty
Two XML doc statements still said the shape writes NO _Policy after it began writing one	Real — my own inconsistency from moving the design mid-change. Corrected, with the marker dependency named
new List<MeshNode>() against the collection policy	Real. ImmutableArray
Convert this file's Observable.Using(ImpersonateAsSystem) to RunAsSystem	Declined. That line is unchanged by this diff, and the same shape is used by Retire and the scoped child walk — converting one leaves the belief that the file uses the sealed boundary while two sites do not. A sweep of its own, with its own measurement. I have not reproduced a wrong-flow restore and am not claiming the concern is wrong

MeshWeaver Build and Test was completed/success on a8018c03 (the head before this round).

9. …and the Initial filter needed a second half, from a measurement I did not make

The review's strongest finding (filter for the Initial snapshot) is right about the fail-open, and on its own it would have been worse than the read it replaced. A finding already sitting in the fleet's triage inbox names the reason, with the verbatim log line:

Query provider(s) [StorageAdapterMeshQueryProvider] have not emitted an Initial after 20s for query 'path:Chess scope:subtree nodeType:AccessAssignment limit:2000' (user 'system-security')

That is this query's exact shape and identity — 200+ occurrences in a 400-minute window on memex, truncated at the log limit, because DefersToNativeProvider is false for satellite reads and the pedestrian walk emits nothing until every per-path read completes. So on a Postgres portal the Initial can genuinely never arrive, and a filter that turned that into "the read says there are no denies" fails open deterministically instead of occasionally.

So the listing now answers a tri-state and every arm declines on Ok: false: nothing retired, and no blanket PublicRead policy written over a shape the pass could not see. That restores a rule this method's own remarks had always stated — "healing on an unknown deny set is the one outcome worse than not healing" — which the original-create arm had quietly broken by writing the policy without consulting the listing at all. A host with no query surface is deliberately a different answer (Ok: true, empty): nothing to enumerate is not the same as not knowing.

🚨 That one if is NOT covered by a test — forcing the listing to fail or to withhold its Initial needs a query-provider fault seam this suite has no hook for, so it is reasoned from the production measurement rather than executed. Named here and on the doc page rather than left to look covered.

🚨 A CI reading that would have fooled me, recorded because it nearly did: for ~7.5 minutes after the push of c02d7e68, the only run on that head was the pull_request_target one (Arm auto-merge) — and the github-actions check-suite read completed/success with latest_check_runs_count: 1, while an unrelated PR six minutes earlier already had its full set. That reads exactly like "the pull_request workflows never fired", and I had written it up as such. They then arrived and the build went in_progress. A check-suite's completed/success is a snapshot, not a verdict — it closes over whatever runs exist at that moment and reopens when more are created, so it cannot distinguish "nothing was triggered" from "nothing has been triggered YET". The instrument for "did the gate run" is the named workflow's own run, which is what the rule in AGENTS.md already says.

10. Coordination with #4969, now that it is in main

origin/main had moved 9 commits (including #4969). Merged it in. git merge-tree reported no textual conflicts — both topic-map entries and both AccessControl.md pointers landed side by side with nothing to hand-resolve — and that was the risk, because #4969 left three prose claims the code beneath them no longer matches. A clean merge would have shipped all three:

1. "That is its author's comment, NOT a test result … 'the two read paths disagree' is the thing to go and confirm" → confirmed (§2), with the standing caveat that core executes no Postgres case.
2. "Order matters: denies first, policy last" → true of the legacy sweep, and the opposite of what the grant shape does, because opening a partition and retiring a gate fail closed in opposite directions. Both orders now stand, each naming why it differs.
3. "That delegation is sound for THIS shape and a real gap for the other one" → that gap is what this closes, and it closes it the way docs(#4716): a Public/Anonymous deny does not beat PublicRead in the C# fold — the SQL fold says it does #4969's own last sentence demanded: read the declaration, never gate satellites from the _ prefix. The prohibition stands, now cited as the reason for the shape.

The two doc pages are cross-linked, and #4969's PublicReadIsNotSuppressedByADenyTest is expected to keep measuring the divergence — nothing here closes it; this stops depending on it.

Re-verified after the merge, each -c Release -warnaserror at 0 warnings / 0 errors: MeshWeaver.PluginCatalog, MeshWeaver.Documentation, Memex.Portal.Shared, MeshWeaver.Documentation.Test, MeshWeaver.Graph.Test, Memex.Portal.Shared.Test, MeshWeaver.Layout.Test, MeshWeaver.Compiler.Pipeline.Test. Tests: this change's AnonymousCannotReadAProtectedSubmissionTest + #4969's PublicReadIsNotSuppressedByADenyTest + the 14-case PublicReadPolicyScopeTest together 24/24; the whole Memex.Portal.Shared.Test project 1765 total, 0 failed; the four documentation guards 17/17.

11. What this does to the LIVE Feedback partition: nothing, on every path

The claim in §7 that no live write and no recycle are needed, traced against the readings in §1 (_Policy withholding public read, root Public/Anonymous Viewer GRANTS from 2026-07-24, the _Submissions deny pair, and no legacy deny on any ordinary child):

Path taken on the next install/boot	Outcome
Record re-listed, so the manifest carries protectedSegments → declaration arm	publish is false (the policy already withholds public read), so only the denies are ensured — and they exist. No write.
Record still stale, no declaration → EnsurePartitionPublicRead, listing readable	Evidence arm fires on the satellite denies; publish is false (Legacy.Count == 0, so the legacy fingerprint does not apply). No write.
Record still stale, listing stalls (the measured Postgres case)	Ok: false — every arm declines. No write.

On main today, the second row instead retires both _Submissions denies and rewrites the policy to PublicRead = true. That difference is the change, and it is why the fix belongs in core rather than in a live edit.

🤖 Generated with Claude Code

[github-actions]

Test Results (shard 3)

519 tests   519 ✅  32s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit b94cb93.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 1)

396 tests   396 ✅  46s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit b94cb93.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 0)

  1 files    1 suites   2m 36s ⏱️
300 tests 300 ✅ 0 💤 0 ❌
302 runs  302 ✅ 0 💤 0 ❌

Results for commit b94cb93.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 5)

1 624 tests   1 624 ✅  2m 41s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit b94cb93.

♻️ This comment has been updated with latest results.

[Copilot]

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 5 · 1 · 2

Open (8)

• Align declared marker with protected package shape · New
• Make policy transition atomic and fail closed · New
• Reject conflicting existing assignments in CreateOnly · New
• Use RunAsSystem boundary for repair query · New
• Wait for Initial query snapshot before evaluating evidence · New
• Add discovery test for protectedSegments projection · New
• Correct XML documentation for protected-segment policy marker · New
• Use immutable collections for mesh node sequences · New

What changed in this PR

Fixes protected child segments on public package partitions by propagating protectedSegments, using grant/deny access shapes, and preserving protections during boot repair.

Changes:

• Adds manifest parsing and grant-based publication.
• Narrows legacy deny healing.
• Adds regression tests and architecture documentation.

File	Description
test/​MeshWeaver.Graph.Test/​AnonymousCannotReadAProtectedSubmissionTest.cs	Updated as part of this pull request.
test/​Memex.Portal.Shared.Test/​ADeclaredProtectedSegmentSurvivesTheBootHealTest.cs	Updated as part of this pull request.
src/​MeshWeaver.PluginCatalog/​PackageInstaller.cs	Updated as part of this pull request.
src/​MeshWeaver.PluginCatalog/​Package.cs	Updated as part of this pull request.
src/​MeshWeaver.PluginCatalog/​NodeRepoPackageSource.cs	Updated as part of this pull request.
src/​MeshWeaver.Documentation/​Data/​Architecture/​ProtectedSegmentsOnAPublicPartition.md	Updated as part of this pull request.
src/​MeshWeaver.Documentation/​Data/​Architecture/​AccessControl.md	Updated as part of this pull request.
src/​MeshWeaver.Documentation/​Data/​Architecture.md	Updated as part of this pull request.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[rbuergi]

Accepted — real, and it is the exact trap my own remarks on DeclaredAccessMarker warn about ("a marker naming a node the chosen shape does not write turns the post-condition into an error on a correct install"), so I wrote the warning and then walked into it.

The case is a pre-installed manifest declaring BOTH publicSegments and protectedSegments: EnsureDeclaredAccess routes it to EnsureScopedPublicRead, which writes root grants and no policy, while the marker still names {partition}/_Policy. VerifyDeclaredAccess would then report a correct install as a failure.

Fixing it by making the marker mirror the selection exactly, rather than by restating it: the grant marker whenever declaredPublic.Count > 0 && (protectedPaths.Count > 0 || !PreInstalled) — the same predicate the branch uses. A pinned case for the both-declared pre-installed manifest goes in beside it, because that combination is what nothing covered.

Worth stating for the record: EnsureOpenWithProtectedSegments (protected only, no publicSegments) does keep _Policy as its marker, and that is safe in both of its arms — when it publishes it writes the policy, and when it declines to publish (create-only) it is because a policy already exists.

[rbuergi]

Accepted, and you are right that the property I documented is not the property the order delivers.

The remarks claim "the worst interruption leaves the PUBLIC half dark". That holds on a fresh partition, and it does NOT hold on the transition that actually matters: _Policy { PublicRead = true } already present, denies written, crash before the flip — the old public grant is still live and the new denies are inert on the C# path, i.e. exactly the state the change exists to end.

Reordering to policy flip -> denies -> root grants, which is fail-closed at every point: the flip removes the blanket grant while no root grant exists yet, so the partition is momentarily closed rather than momentarily open; the denies land while it is closed; the grants re-open only the public half, with the gate already in place. A crash anywhere leaves the partition closed, which is the half it is safe to fail on, and the next pass completes it.

I am not taking the other half of the suggestion — an atomic transition or a staged state readers fail closed on. There is no multi-node atomic write here, and introducing a new access state for this would be a new permission primitive on the fold with the incident history; the reorder achieves fail-closed with no new concept, so adding one would be cost without a gain. Saying so explicitly rather than silently doing half.

[rbuergi]

Accepted — real, and it is the one way the shape could be defeated silently.

CreateOnly asks only "is a node there", so a Public/Anonymous assignment at a declared-protected scope that is a GRANT (or grants any read-capable role) survives, and the root grant then publishes the segment. The protection would read as established and not be.

The house rule for this already exists next door: the Store gate does not use presence either, it uses StillNeedsDeny (fresh is null || !IsDenyAssignment(fresh)). Adopting the same predicate for the protected-segment denies via the existing IsWellKnownDeny: write when absent, and write when the existing assignment for that well-known subject is not an all-denied role set.

That is deliberately narrow and it does not breach create-only, because it can only ever NARROW: the only node it overwrites is a Public/Anonymous assignment at a scope the manifest declares protected, and it replaces it with a deny. Every other assignment — a real user, a group, any grant at any other scope — stays untouched. A case with a pre-existing Public grant at the protected scope goes in with it.

[rbuergi]

Declining this one, with the reasoning rather than a dismissal — and flagging where I think the finding is misaddressed.

That Observable.Using(() => ImpersonateAsSystem(), ...) is not new in this diff. The method was ContradictingDenies; I changed its .Select projection (splitting one deny list into two) and its return type, and left the impersonation and the query untouched. git diff origin/main...HEAD on the file shows the Observable.Using line as unchanged context.

I also do not want to fix it here, because a partial pattern fix is worse than none: the same Observable.Using(ImpersonateAsSystem) shape is used by Retire and by the scoped-shape child walk in this same file, and by call sites elsewhere. Converting one of them leaves the belief "this file uses RunAsSystem" while two sites still do not, which is how the next reader stops checking. If the AsyncLocal-restore-on-the-wrong-flow concern is right it is a sweep of its own, with its own measurement of what actually latches.

What I have checked and what I have not: AccessService.RunAsSystem does exist as the sealed boundary, so the remedy you name is available. I have NOT reproduced a wrong-flow restore on this path, and I am not claiming the concern is wrong — only that it is pre-existing and wider than this diff.

[rbuergi]

Accepted — this is the strongest of the eight, because it is a fail-OPEN direction on a decision that is now a security decision.

If the consumed frame omits the satellite deny, found.Protected is empty, the evidence arm does not fire, and the pre-installed heal proceeds to retire denies and write PublicRead — republishing the protected segment, which is the defect the arm exists to prevent. That the pre-existing code already took a security decision off the same unfiltered .Take(1) (the legacy retire DELETES nodes on it) makes it worse, not acceptable.

Filtering for the Initial snapshot before Take(1). It is the established idiom in this repository rather than something new — eight sites already do it, including DeploymentReportService, PlanTierLadder, GitHubSyncService and PathResolutionService — so the fix is .Where(change => change.ChangeType == QueryChangeType.Initial) ahead of the existing .Take(1), and the .Timeout stays where it is so a snapshot that never arrives is a diagnosed failure rather than a hang.

Note the failure mode after the fix is the one I want: a listing that cannot produce an Initial frame times out into the existing Catch, which yields nothing and leaves the partition exactly as it is. No retire, no policy write.

[rbuergi]

Accepted — and the observation is sharper than "add a test": it names how the whole change set could regress into being a no-op while every assertion I wrote stays green.

Every test I added constructs PackageManifest in-process, so the projection under review — the nested content.protectedSegments read in Peek, the PeekedRoot field, and the assignment into the manifest — is covered by nothing. That is precisely the shape of the defect being fixed: the field existed in the authored index.json and core dropped it on the way in, silently, and installed node-repo packages took the blanket PublicRead path. A regression there restores the exposure and reds nothing.

Adding a source-listing test that goes through NodeRepoPackageSource.ListPackages with an injected fetch (the constructor takes the fetch delegate, so no network and no git), over an index.json that carries protectedSegments beside preInstalled, asserting the declaration survives discovery — and one arm asserting a manifest without the key comes back empty rather than defaulted to something.

[rbuergi]

Accepted — my own inconsistency, introduced when the design moved mid-change.

The shape originally wrote no _Policy at all. It acquired one precisely so DeclaredAccessMarker keeps holding (and so the partition carries the decision rather than leaving the next reader to infer it from the grants), and I updated the method that writes it without going back to the two places that describe it. So the documentation now says the opposite of the code, on exactly the node a future change must not remove.

Correcting both — the EnsureDeclaredAccess bullet and the EnsureOpenWithProtectedSegments summary — to state the actual shape: root Public+Anonymous Viewer grants, a deny per declared segment, and a _Policy that WITHHOLDS PublicRead, which is the install post-condition marker and must stay. Naming the marker dependency in the text, since that is the part that bites if it is removed.

[rbuergi]

Accepted. new List<MeshNode>() in src/ is against the repository collection policy, and I introduced two of them in PublishExceptProtected while removing a pre-existing one from the scoped shape — so the diff moved the violation rather than clearing it.

Building both sequences with LINQ into ImmutableArray instead: the denies as a SelectMany over the protected scopes producing the Public/Anonymous pair, the grants as an ImmutableArray.Create of the two root assignments. CreateOnly already takes IEnumerable<MeshNode>, so nothing else changes, and the scoped shape next door already composes its sequence this way after this diff — which makes the two shapes read the same.

[github-actions]

Test Results (shard 2)

    1 files      1 suites   5m 16s ⏱️
1 887 tests 1 887 ✅ 0 💤 0 ❌
1 888 runs  1 888 ✅ 0 💤 0 ❌

Results for commit b94cb93.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 4)

    1 files      1 suites   6m 47s ⏱️
1 761 tests 1 761 ✅ 0 💤 0 ❌
1 765 runs  1 765 ✅ 0 💤 0 ❌

Results for commit b94cb93.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results

    7 files      7 suites   18m 40s ⏱️
6 487 tests 6 487 ✅ 0 💤 0 ❌
6 494 runs  6 494 ✅ 0 💤 0 ❌

Results for commit b94cb93.

♻️ This comment has been updated with latest results.