Skip to content

fix(auth): fail closed on unresolved flock-scope actors (#787) - #868

Merged
mforce merged 1 commit into
mainfrom
fix/issue-787
Sep 14, 2026
Merged

mforce merged 1 commit into
mainfrom
fix/issue-787

Conversation

@mforce

@mforce mforce commented Sep 14, 2026 •

Copy link
Copy Markdown
Owner

What and why

Closes #787.

FlockScopeGuard.CheckAsync returned success when ICurrentUser was unresolved. The feed- and water-usage handlers do not write audit events, so a future non-HTTP caller could silently receive account-wide flock access.

The guard now returns Auth.Unauthorized before querying assignments. Existing HTTP and seeder paths already resolve an actor. The documentation changes keep two boundaries explicit: unresolved EF reads remain unrestricted, and a deliberately resolved system actor remains account-wide.

Review in this order:

  1. UserRoleAssignmentRepository.cs contains the behavior change.
  2. FlockScopeGuardTests.cs proves the refusal happens before database access.
  3. FlockScope, CurrentUserContext, middleware, and AGENTS.md record the unchanged boundaries.

How it was verified

  • Restored the original return Result.Success() branch. CheckAsync_WithUnresolvedActor_ReturnsUnauthorizedWithoutDatabaseAccess failed with expected Auth.Unauthorized and actual empty error.
  • Restored the fix. The full application suite passed: 291 tests.
  • dotnet build Cluckwork.sln --no-restore --disable-build-servers -v minimal succeeded with 0 warnings and 0 errors.

Checklist

  • The PR title is a conventional commit and is the intended release note.
  • Tests cover the change, and the new test failed first.
  • The new guard behavior was mutation-checked against its own assertion.
  • The write/read distinction and system-actor rule are recorded in AGENTS.md.
  • No credentials, hosting-provider names, packages, migrations, or user-visible behavior were added.

Summary by CodeRabbit

  • Bug Fixes

    • Unauthorized unresolved actors are now blocked from flock-scoped write operations instead of being granted access.
    • Flock-scoped reads remain unrestricted for unresolved actors by design.
  • Tests

    • Added coverage confirming unresolved actors are rejected without requiring database access.
  • Documentation

    • Updated authorization guidance and comments to clarify actor resolution and system-actor access behavior.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e03f29cc-00eb-4c1e-a25c-46aa73b4fb9d

📥 Commits

Reviewing files that changed from the base of the PR and between da49481 and 0f5c818.

📒 Files selected for processing (8)
  • AGENTS.md
  • src/Cluckwork.Api/Middleware/FlockScopeResolutionMiddleware.cs
  • src/Cluckwork.Application/Common/ICurrentUser.cs
  • src/Cluckwork.Infrastructure/Identity/CurrentUserContext.cs
  • src/Cluckwork.Infrastructure/Persistence/FlockScope.cs
  • src/Cluckwork.Infrastructure/Repositories/UserRoleAssignmentRepository.cs
  • tests/Cluckwork.Api.IntegrationTests/FlockScopeMiddlewareTests.cs
  • tests/Cluckwork.Application.Tests/FlockScope/FlockScopeGuardTests.cs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

FlockScopeGuard now returns Unauthorized for unresolved actors before reading assignments. A unit test verifies no database access. Comments and guidance now distinguish fail-closed writes from unrestricted unresolved reads and document system-actor scope.

Changes

Flock scope authorization

Layer / File(s) Summary
Reject unresolved actors on writes
src/Cluckwork.Infrastructure/Repositories/UserRoleAssignmentRepository.cs, tests/Cluckwork.Application.Tests/FlockScope/FlockScopeGuardTests.cs
FlockScopeGuard.CheckAsync returns AppError.Unauthorized() for unresolved actors before database access. The new test verifies this behavior with an unreachable database.
Document write and read scope rules
AGENTS.md, src/Cluckwork.Api/Middleware/FlockScopeResolutionMiddleware.cs, src/Cluckwork.Application/Common/ICurrentUser.cs, src/Cluckwork.Infrastructure/Identity/CurrentUserContext.cs, src/Cluckwork.Infrastructure/Persistence/FlockScope.cs, tests/Cluckwork.Api.IntegrationTests/FlockScopeMiddlewareTests.cs
Comments and guidance now describe fail-closed unresolved writes, unrestricted unresolved reads, assignment behavior, and system-actor access review requirements.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 0f5c8

The authorization change rejects unresolved write actors while preserving the documented read and system-actor boundaries. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 7 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main behavior change: unresolved flock-scope actors now fail closed. It uses a conventional commit format and includes the issue reference.
Description check ✅ Passed The description includes the required What and why, How it was verified, and Checklist sections. It explains the behavior change, links issue #787, documents mutation testing, reports test and build r…
Linked Issues check ✅ Passed Issue #787 requires fail-closed authorization for unresolved actors, explicit actor declaration by non-HTTP callers, preserved access for deliberately resolved system actors, unrestricted unresolved E…
Out of Scope Changes check ✅ Passed The guard change, the no-database-access test, and the related documentation updates all support Issue #787. The documentation clarifies the boundary between fail-closed flock-scoped writes and intent…
Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 7 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-787

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mforce

mforce commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mforce
mforce merged commit 16d0350 into main Sep 14, 2026
17 checks passed
@mforce
mforce deleted the fix/issue-787 branch September 14, 2026 14:29
mforce pushed a commit that referenced this pull request Sep 16, 2026
🤖 I have created a release *beep* *boop*
---


## [0.1.2](v0.1.1...v0.1.2)
(2026-09-16)


### Features

* **data:** standardize business record chronology
([#820](#820))
([6231b31](6231b31))
* **infra:** optional leader-lease endpoint for pooled deploys
([#869](#869))
([e9bc6a7](e9bc6a7))
* **sim:** seed a second farm for the README dashboard capture
([#867](#867))
([de407c6](de407c6))
* **web:** adopt MUI, themed from the farm palette tokens
([#674](#674))
([#860](#860))
([6c83c5c](6c83c5c))
* **web:** convert Daily entry to MUI, field-first on the phone
([#888](#888))
([b66f8b8](b66f8b8))
* **web:** convert the Dashboard and app shell to MUI
([#829](#829))
([#883](#883))
([2e94277](2e94277))
* **web:** retire the Slack-blue link colour for ink + a rule underline
([#884](#884))
([c08f9d8](c08f9d8))
* **web:** serve a per-request CSP nonce so Emotion's styles apply under
style-src 'self'
([#874](#874))
([ba4e6f3](ba4e6f3))
* **web:** visual language theme overrides for the MUI revamp
([#864](#864))
([#882](#882))
([0bb6b73](0bb6b73))
* **web:** whole-app MUI baseline, theme policy guard and the
[#740](#740) phone action rule
([#823](#823))
([#871](#871))
([af565e4](af565e4))


### Bug fixes

* **auth:** fail closed on unresolved flock-scope actors
([#787](#787))
([#868](#868))
([16d0350](16d0350))
* **auth:** make farm configuration owner-only
([#870](#870))
([42f9036](42f9036))
* **e2e:** repoint the canary at the markup two PRs replaced
([#844](#844))
([18b45dc](18b45dc))
* **i18n:** tl glossary uses the standard passive of ilagay
([#813](#813))
([20dec10](20dec10)),
closes [#738](#738)
* **sim:** stop the k6-baseline EXIT trap masking a clean run as failed
([#838](#838))
([f5ec96f](f5ec96f))
* **web:** declare the rule tokens the Dashboard reads, and guard
undeclared custom properties
([#885](#885))
([5bead1f](5bead1f))


### Performance

* **ci:** start the serialized integration collection first
([#861](#861))
([1dcc7f6](1dcc7f6)),
closes [#839](#839)


### Documentation

* **auth:** record the OAuth 2.1 decision for MCP authentication
([#801](#801))
([0510854](0510854))
* **designs:** MUI revamp design doc, component map, layout system, IA
([#862](#862))
([da49481](da49481))
* **readme:** recapture the daily entry, reports and sales screenshots
([#865](#865))
([f18e336](f18e336))
* **specs:** correct the sales_order_items column list in §10.5
([#812](#812))
([afe4a02](afe4a02)),
closes [#737](#737)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: cluckwork-lockfix[bot] <309265648+cluckwork-lockfix[bot]@users.noreply.github.com>
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.

FlockScopeGuard's fail-open branch grants account-wide flock access to an unresolved actor

1 participant