Skip to content

fix(recycle): a DENIED release stamp refuses the recycle instead of disposing the hub anyway - #2815

Merged
rbuergi merged 1 commit into
mainfrom
fix/recycle-refuses-when-denied
Aug 30, 2026
Merged

rbuergi merged 1 commit into
mainfrom
fix/recycle-refuses-when-denied

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Reported from production (memex, 2026-08-30 17:11:21Z) — and it is my own regression from #2763.

What happened

Recycle on a NodeType is two halves of one authorised operation: stamp a release request on the node, then dispose its hub so the next activation acts on that stamp. The stamp goes through the access pipeline; the dispose does not. The failure handler I added in #2763 caught every exception and disposed anyway:

.Catch((Exception ex) => {
    logger.LogWarning(ex, "Recycle: failed to stamp release request for {Path} — disposing the hub anyway", …);
    return Observable.Return(false);
});

I wrote that reasoning for a timeout and then applied it to everything, including UnauthorizedAccessException. An operator without Update on a GitSynced module space — where the only Admin is system-security, by design — called Recycle:

AccessControlPipeline: Access denied: user 'rbuergi' lacks Update permission on 'Crm/Migration'
   (triggered by message=PatchDataRequest sender=cache/_ciqCj6MNUWJjheeu08jRg)
[UpdateRemote] OWNER_DENIED … [UpdateQueue] FAILED path=Crm/Migration seq=632
Recycle: failed to stamp release request for Crm/Migration — disposing the hub anyway

The NodeType was left with no release request to act on and its watcher went silent. A caller who may not write the node may not recycle it either — proceeding after the write was refused hands an unauthorised caller the destructive half for free. It now throws and disposes nothing; anything that is not a denial still proceeds, because that caller was allowed to ask.

🚨 Two layers, and the test can only reach the first — stated, not papered over

Recycle already pre-checks Update and answers with a legible refusal. RecycleDeniedStampTest pins that, and it is worth pinning: the operator in the incident received a settle-timeout instead and could not tell "the trigger did not dispatch" from "you were refused". (Same complaint, other end: MeshWeaver.Plugins#985, where the compile tool's description claimed no Update permission was required at all.)

But the incident proves the pre-check is not sufficient — the operator got past it and the owner refused the write. The owner's answer is the authoritative one and it arrives after the optimistic local fold has said yes. That is the gap this fix closes, and no deterministic local repro exists for it because it needs the pre-check and the owner to disagree. I did not manufacture one: a test that faked the disagreement would be pinning its own scaffolding. It is recorded in the doc comment instead.

Two real traps hit while writing the test, both now documented in it

  • It must recycle a NodeType node. On anything else the Update lambda returns the node unchanged — a NO-OP that never reaches the access pipeline — so a Markdown node would have passed having proved nothing.
  • MeshOperations resolves IWorkspace from the hub it is given, and a client hub has none ("An exception was thrown while activating MeshWeaver.Data.IWorkspace"). The identity is switched on the mesh's AccessService around the Subscribe, because Recycle is cold.

Verified

MeshWeaver.Mesh.Operations and MeshWeaver.Hosting.Monolith.Test each -c Release -warnaserror → 0 Error(s); RecycleDeniedStampTest 1/1 green.

🤖 Generated with Claude Code

…isposing the hub anyway

Reported from production (memex, 2026-08-30 17:11:21Z) and it is my own
regression from #2763.

Recycle on a NodeType is two halves of ONE authorised operation: stamp a
release request on the node, then dispose its hub so the next activation acts
on that stamp. The stamp goes through the access pipeline; the dispose does
not. The stamp's failure handler I added in #2763 caught EVERY exception and
disposed anyway:

    .Catch((Exception ex) => { logger.LogWarning(ex,
        "Recycle: failed to stamp release request for {Path} — disposing the
         hub anyway", …); return Observable.Return(false); });

I wrote that reasoning for a TIMEOUT and then applied it to everything,
including UnauthorizedAccessException. On memex an operator without Update on
a GitSynced module space (where the only Admin is system-security, by design)
called Recycle:

  AccessControlPipeline: Access denied: user 'rbuergi' lacks Update
    permission on 'Crm/Migration' (triggered by message=PatchDataRequest …)
  [UpdateRemote] OWNER_DENIED … [UpdateQueue] FAILED path=Crm/Migration seq=632
  Recycle: failed to stamp release request for Crm/Migration — disposing the
    hub anyway

The NodeType was then left with no release request to act on and its watcher
went silent. A caller who may not WRITE the node may not RECYCLE it either:
proceeding after the write was refused hands an unauthorised caller the
destructive half for free. It now throws and disposes nothing. Anything that
is NOT a denial still proceeds — that caller was allowed to ask.

🚨 TWO LAYERS, and the test can only reach the first. Recycle already
PRE-CHECKS Update and answers with a legible refusal; RecycleDeniedStampTest
pins that, because the memex operator got a settle-TIMEOUT instead and could
not tell "the trigger did not dispatch" from "you were refused" (the same
complaint drives MeshWeaver.Plugins#985, where the compile tool's own
description claimed no Update permission was required). But memex proves the
pre-check is NOT sufficient: the operator got past it and the OWNER refused
the write — the authoritative answer, arriving after the optimistic local fold
has said yes. No deterministic local repro exists for that disagreement, and
I did not manufacture one: a test that faked it would pin its own scaffolding.
Recorded in the doc comment instead.

Test notes, both real traps hit while writing it:
 - it must recycle a NODETYPE node — on anything else the Update lambda
   returns the node unchanged, a NO-OP that never reaches the access pipeline,
   so a Markdown node would pass having proved nothing;
 - MeshOperations resolves IWorkspace from the hub it is given, and a CLIENT
   hub has none ("An exception was thrown while activating
   MeshWeaver.Data.IWorkspace") — the identity is switched on the MESH's
   AccessService around the Subscribe, because Recycle is cold.

Verified: MeshWeaver.Mesh.Operations + Hosting.Monolith.Test each
-c Release -warnaserror → 0 Error(s); RecycleDeniedStampTest 1/1 green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 30, 2026 17:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a security/operational regression in Recycle where a release-stamp denial (lack of Update permission) previously still disposed the node’s hub, allowing an unauthorized caller to trigger the destructive half of the operation.

Changes:

  • Treat UnauthorizedAccessException during the release-request stamp as a hard refusal: propagate the error and do not dispose the hub.
  • Add a regression test that pins the permission pre-check refusal behavior for callers lacking Update permission.
  • Add a “What’s New” entry describing the behavior change and the production incident context.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/MeshWeaver.Mesh.Operations/MeshOperations.cs Stops Recycle from disposing hubs when the release-stamp was denied (Unauthorized).
test/MeshWeaver.Hosting.Monolith.Test/RecycleDeniedStampTest.cs Adds a regression test ensuring a caller without Update permission is refused (no success payload).
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-a-refused-recycle-no-longer-tears-the-hub-down.md Documents the incident and the corrected refusal semantics.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

|| (outcome?.Contains("Update permission", StringComparison.OrdinalIgnoreCase) ?? false)
|| (outcome?.Contains("denied", StringComparison.OrdinalIgnoreCase) ?? false);

outcome.Should().NotContain("\"status\":\"Recycled\"",
@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

787 tests   783 ✅  4m 15s ⏱️
  7 suites    4 💤
  7 files      0 ❌

Results for commit 2ebf623.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

943 tests   943 ✅  5m 7s ⏱️
  7 suites    0 💤
  7 files      0 ❌

Results for commit 2ebf623.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 085 tests   1 085 ✅  5m 29s ⏱️
    7 suites      0 💤
    7 files        0 ❌

Results for commit 2ebf623.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    8 files      8 suites   5m 57s ⏱️
1 521 tests 1 520 ✅ 1 💤 0 ❌
2 195 runs  2 194 ✅ 1 💤 0 ❌

Results for commit 2ebf623.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

3 007 tests   2 815 ✅  6m 19s ⏱️
    8 suites    192 💤
    8 files        0 ❌

Results for commit 2ebf623.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

1 165 tests   1 165 ✅  7m 36s ⏱️
    7 suites      0 💤
    7 files        0 ❌

Results for commit 2ebf623.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

   44 files     44 suites   34m 46s ⏱️
8 508 tests 8 311 ✅ 197 💤 0 ❌
9 182 runs  8 985 ✅ 197 💤 0 ❌

Results for commit 2ebf623.

@rbuergi
rbuergi added this pull request to the merge queue Aug 30, 2026
Merged via the queue into main with commit 8d0a15d Aug 30, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants