test(tenancy): allow only login to call the cross-farm FindBySlugAsync

[mforce]

TL;DR: a new guard, FindBySlugCallerTests, fails CI when any member except login's IdentityProvider.ResolveFarmCodeAsync references IAccountRepository.FindBySlugAsync. The review round also fixes two weaknesses in the shared source walk that every source guard inherits. No production code changes; login is unchanged.

Closes #1053

Design choice: caller guard, not a move

FindBySlugAsync ignores the tenant filter and returns any farm's whole Account. It sits on Farm's seam (IAccountRepository), which every peer module may take, so the ledger guards stay green on a new caller (mutant O5 in the #1043 Opus review).

• Moving it behind IFarmDirectory would have login name the directory, which FarmDirectoryCallerTests keeps off every request path.
• Moving it into an Access login port would add an Access read of a Farm table, with a new compatibility-exception row and tenant-bypass entry, to close a gap that a guard closes on its own.

How the guard works

• It walks every .cs under src/ and finds each IdentifierNameSyntax named FindBySlugAsync. Each reference is keyed by its enclosing symbol through GuardScanner.EnclosingSymbolOf (Replace TenantBypass line-number anchors with stable query identities #632). Only Cluckwork.Infrastructure.Identity.IdentityProvider.ResolveFarmCodeAsync(string farmCode, CancellationToken ct) may reference it. Declarations are not references.
• Positive control: the list of references, with duplicates kept, must contain login's symbol.
• Known limit, the same as FarmDirectoryCallerTests: the guard does not follow calls (login forwards only (Id, IsActive)), and it cannot see reflection by string name.

Shared-walk fixes (review F2, F3)

• F2: parse symbols come from the build. ModuleLedgerScanner.ParseOptions was a hand list. It omitted the SDK's compatibility symbols NETCOREAPP1_0_OR_GREATER … NETCOREAPP3_1_OR_GREATER. The test csproj now embeds $(DefineConstants) as assembly metadata, from a target that runs after AddImplicitDefineConstants. ParseOptions reads that list and throws if it is missing.
• Why the test(arch): match build symbols and reject inactive source #1056 tripwire did not stop A2: it did. SourcePreprocessorTests.RealSourceTree_HasNoInactiveCode goes red on A2 in Debug and Release and names CreateProductHandler.cs:21. The review's 21-test set did not include that class. The tripwire works; the gap was the incomplete symbol list. A branch the compiler builds was reported as inactive, and the guards parsed it as dead code.
• F3: the walk matches the compiler's inputs. GuardScanner.EnumerateSourceFiles skipped every bin/, obj/ and node_modules/ directory. The SDK skips only a project's own bin/ and obj/. The walk now does the same, so a .cs file under src/<project>/node_modules/ is walked. No src/ file count changes today.

Change map

File	What	Why
tests/.../TenantBypass/FindBySlugCallerTests.cs	New guard: real-tree fact keyed by symbol, 7 syntax cases, 1 negative case	Fails CI on any new caller (#1053, F1)
tests/.../Cluckwork.Application.Tests.csproj	EmbedDefineConstants target	Embeds the compiler's real symbol list (F2)
tests/.../Architecture/ModuleLedgerScanner.cs	ParseOptions reads the embedded list; fails closed if it is missing	Removes the hand list (F2)
tests/.../Architecture/SourcePreprocessorTests.cs	ActiveFrameworkRegion_IsAccepted becomes a theory with NETCOREAPP3_1_OR_GREATER	Pins the A2 symbol as active (F2)
tests/.../TenantBypass/GuardScanner.cs	Skip bin/obj only directly under a project directory; stop skipping node_modules	Matches the SDK's compile inputs (F3)
tests/.../TenantBypass/Data/tenant-bypass-allowlist.json	Rewrote the FindBySlugAsync justification	The old text ("operator CLI and provisioning") was stale
src/Cluckwork.Application/Features/Accounts/IAccountRepository.cs	Two comment lines	Point the next caller to the guard
src/AGENTS.md, docs/decisions/843-*.md, docs/decisions/846-*.md	One sentence each	Record the guard and the derived symbol list

Mutation table (real tree, each mutant reverted after its run)

#	Mutant	Config	Result	Member named
O5	CreateProductHandler calls FindBySlugAsync("another-farm", ct)	Debug	red	CreateProductHandler.HandleAsync
M2	Method-group alias into a Func	Debug	red	CreateProductHandler.HandleAsync
M3	Extension helper in a new file	Debug	red	AccountLookupExtensions.OtherFarmAsync
M4	RequestServices.GetRequiredService<IAccountRepository>().FindBySlugAsync(…) in an endpoint filter	Debug	red	ReportConcurrencyLimitFilter.InvokeAsync
M5	O5 under #if NET10_0	Debug	red	CreateProductHandler.HandleAsync
M6	Login's call removed	Debug	red (positive control)	—
A1	Second caller in another IdentityProvider method	Debug	red	IdentityProvider.OtherFarmNameAsync
A1b	Login forwards to a sibling helper in the same file	Debug	red	IdentityProvider.FindAsync
A2	O5 under #if NETCOREAPP3_1_OR_GREATER	Debug	red	CreateProductHandler.HandleAsync
A2	same	Release	red	CreateProductHandler.HandleAsync
A3	Compiled wrapper under src/Cluckwork.Application/node_modules/, called from the handler	Debug	red	HiddenLookup.ReadAsync
F2c	EmbedDefineConstants target deleted	Debug	red, 18/18 fail: "no DefineConstants metadata"	—
—	Head	Debug + Release	green	—

Tests (measured locally, Cluckwork.Application.Tests)

• --list-tests: 703 at main (cd1de3a8), 713 at head.
• At head, each class run on its own in both Debug and Release, all green: SourcePreprocessor 9, TenantBypass 56, ModuleLedger 40, PeerContractRealTree 1, AdapterReachRealTree 2, AdapterTier 32, CompatibilityExceptionRealTree 2, CouplingMatrixRealTree 2, InsightsReadOnly 25, EggLotLockSql 2, SeamSurface 41, Documentation 20.
• The two SchemaDocsTests ImagePin tests pass. Not run locally: the full suite and the integration suite.

[mforce]

Verdict: request changes. Two P2 gaps undermine the new caller guard. There is no newly introduced production vulnerability: this diff changes tests, comments and one registry justification. The findings concern code changes that the guard promises to reject.

Reviewed c2e91f75512bbaee58121bc2cce702d3c771dbf7 against 1100445eafd13680113caf32a64ab914d4d8c9b3, the #1060 stack base. The supplied diff matches git diff HEAD^ HEAD byte-for-byte. PR head advanced to b4b5c2ba29c63ff9875f3bb085fd10f419278295 before posting; this review and its test results apply to the assigned c2e91f75 head only. No delegation, product fixes or pushes.

F1, P2: exempt the login symbol, not its entire file. FindBySlugCallerTests.cs:19–22,35–42 exempts every reference in the 1,958-line IdentityProvider.cs, then proves only that the file contains some reference. A new caller in any other method passes, and removing login's reference would still pass if that other reference remained. This conflicts with #1053's login-only boundary and AGENTS.md's #632 symbol-key rule.

I added this helper in IdentityProvider.cs, then used its result as GetUserAsync's display name, with the original display name as the null fallback:

private async Task<string?> OtherFarmNameAsync(CancellationToken ct) =>
    (await accounts.FindBySlugAsync("another-farm", ct))?.Name;

The existing /me path would return another farm's name when that code exists. The mutant builds and all 21 targeted guard tests pass in Release, with no ledger or allow-list edit. This is a direct new reference outside login, not merely the documented inability to follow calls. Key references by Cluckwork.Infrastructure.Identity.IdentityProvider.ResolveFarmCodeAsync(string farmCode, CancellationToken ct) using the existing GuardScanner.EnclosingSymbolOf. Require the expected reference to exist without collapsing duplicates. Prove a second method in the same file fails, and that a reference elsewhere cannot substitute for the login positive control.

F2, P2: the parser omits framework symbols the compiler defines. FindBySlugCallerTests.cs:60–62 uses ModuleLedgerScanner.cs:41–48, whose symbol list omits NETCOREAPP3_1_OR_GREATER and the other .NET Core compatibility symbols. Wrapping the original O5 call in this conditional makes the guard green:

#if NETCOREAPP3_1_OR_GREATER
    await accounts.FindBySlugAsync("another-farm", ct);
#endif

MSBuild's AddImplicitDefineConstants target confirms this symbol is active for the repository's .NET 10 Release build. The mutant builds and all 21 targeted guard tests pass in both Debug and Release. The existing check for custom DefineConstants does not cover SDK-generated symbols. The incomplete shared parser predates this PR, but this new guard inherits its false negative. Complete the shared framework-symbol set and add this real-tree mutation to the proof. No new call-graph machinery is needed.

F3, P3: the shared source walk skips C# that the project compiles. FindBySlugCallerTests.cs:30 inherits GuardScanner.cs:1384–1389, which skips node_modules, bin and obj. I placed a HiddenLookup.ReadAsync wrapper under src/Cluckwork.Application/node_modules/, called it from CreateProductHandler, and built the Application tests. The C# SDK includes that file by default; the wrapper calls FindBySlugAsync. All 21 targeted guards pass. The ignored file would need to be explicitly force-added to ship, so this is a lower-priority, pre-existing source-discovery weakness. Align the compiler's inputs with the guard's exclusions, or explicitly narrow the completeness claim. A normal .g.cs outside skipped directories is caught.

The 21-test set was FindBySlugCallerTests (8), FarmDirectoryCallerTests (2), TenantBypassRealTreeTests (4), ModuleLedgerRealTreeTests (2), PeerContractRealTreeTests (1), AdapterReachRealTreeTests (2), and CompatibilityExceptionRealTreeTests (2). Each class ran separately with DOTNET_USE_POLLING_FILE_WATCHER=1. These are targeted results, not a claim that the full CI or HTTP integration suite passed the mutants.

Independent mutation results

Mutant	Result
Author O5: direct call in CreateProductHandler	New guard red; names the handler
Author M2: method-group delegate	New guard red; names the handler
Author M3: extension helper in a new file	New guard red; names the extension file
Author M4: endpoint-filter service resolution	New guard red; names the filter
Author M5: O5 under #if NET10_0	New guard red
Author M6: remove login's call	Positive control red
A1: new helper inside the exempt file	All 21 guards green, including Release
A2: O5 under #if NETCOREAPP3_1_OR_GREATER	All 21 guards green in Debug and Release
A3: compiled wrapper in skipped node_modules	All 21 guards green
A4: nameof(IAccountRepository.FindBySlugAsync)	New guard red
A5: GetMethod("FindBySlugAsync").Invoke(...)	New guard green; documented string-reflection limit
A6: generic helper constrained to IAccountRepository	New guard red
A7: caller in a generated .g.cs partial class	New guard red; names the generated file
A8: another repository method with the same filter-free read	New guard green, existing tenant-bypass guard red on the new method

The six author mutations were reproduced from the author's script against the review checkout and reverted individually. A5 is an acknowledged limitation, not an additional finding. Generic/interface dispatch, method groups and ordinary partial/generated source do not evade this identifier scan. A fresh raw query still requires a tenant-bypass classification.

Security review, applied manually. I traced farm-code input through IAccessModule and IdentityProvider to the full-Account IgnoreQueryFilters read, checked the existing /me response path for F1, and inspected alternative directory/repository reads. The current login forwarder returns only FarmSignIn(Id, IsActive). No executable query, authentication rule, tenant filter, credential handling or serialization changes in this PR. There is no HIGH/MEDIUM runtime vulnerability introduced by the diff. F1–F3 are confirmed guard weaknesses requiring a subsequent source change to expose data; I did not present those mutations as deployed exploits.

Registry check. The JSON edit changes exactly one justification; its symbol and file keys are unchanged. That allow-list row has no token-hash field. The separate token-hash registry, filter-free-set-sites.tsv, is byte-identical to the base, as is AccountRepository.cs. No query token or stored hash was repinned. The TSV's SHA-256 is c052e74ff809fe797da75ddcdcafd216bf60a2890b99a872837c1cd205ca718c.

Thermo-nuclear quality review, P3 and slice-scoped. The new guard is 63 lines, reuses the repository's parser and source walker, adds no dependency and introduces no product branching. No file crosses the 1,000-line threshold in this diff. The useful structural correction is F1: reuse the existing symbol extractor instead of a file exemption. A semantic call-graph analyzer or moving the query into Access would cost more and is unnecessary for the two P2 fixes. No separate maintainability blocker.

Ponytail review, complexity only: Lean already. Ship. There is no worthwhile deletion finding; the correctness verdict remains request changes.

Restored-head verification: 62/62 tests passed in Release across 11 classes, including all 55 TenantBypass cases. git status --porcelain is empty and git diff HEAD^ HEAD --check passes. All mutants were reverted. The full solution, integration suite and HTTP scenario were not run.

Detailed review notes, exact mutation patches/scripts, compiler-symbol output and per-class logs are retained locally in /home/mforce/.cluckwork-slices/reviews/1061-c2e91f75-astra-evidence/. The report is /home/mforce/.cluckwork-slices/reviews/1061-c2e91f75-astra.md.

[mforce]

Reply to Astra's review of c2e91f75. Fixed in b5502ee9bb835571ed34582552ed2b7fbc07a4fe.

F1 (P2), fixed. The exemption is now one enclosing symbol, Cluckwork.Infrastructure.Identity.IdentityProvider.ResolveFarmCodeAsync(string farmCode, CancellationToken ct), computed with GuardScanner.EnclosingSymbolOf. The positive control checks for that symbol in the reference list, with duplicates kept.

• A1 (your OtherFarmNameAsync helper): red, names IdentityProvider.OtherFarmNameAsync.
• A1b (login forwards to a sibling FindAsync in the same file, so another reference replaces login's): red, names IdentityProvider.FindAsync.

F2 (P2), fixed at the source. ModuleLedgerScanner.ParseOptions no longer has a hand list. The test csproj embeds $(DefineConstants) as AssemblyMetadata, from a target that runs after AddImplicitDefineConstants. The embedded list is TRACE;DEBUG;NET;NET10_0;NETCOREAPP;NET5_0_OR_GREATER…NET10_0_OR_GREATER;NETCOREAPP1_0_OR_GREATER…NETCOREAPP3_1_OR_GREATER. If the metadata is missing, the guards throw. Deleting the target made 18 of 18 tests fail with "no DefineConstants metadata".

• A2 under #if NETCOREAPP3_1_OR_GREATER: red in Debug and in Release, names CreateProductHandler.HandleAsync.
• Why the test(arch): match build symbols and reject inactive source #1056 tripwire did not stop A2: it did. With the old head, SourcePreprocessorTests.RealSourceTree_HasNoInactiveCode was red on A2 in both configurations and named CreateProductHandler.cs:21. Your 21-test set did not include that class. The tripwire works. The gap was the incomplete list: code the compiler builds was parsed as dead code. ActiveFrameworkRegion_IsAccepted is now a theory that includes NETCOREAPP3_1_OR_GREATER.

F3 (P3), aligned. GuardScanner.EnumerateSourceFiles now skips bin/ and obj/ only when they sit directly under a directory that holds a .csproj, as the SDK does. It no longer skips node_modules/. A3 (a wrapper under src/Cluckwork.Application/node_modules/): red, names HiddenLookup.ReadAsync. No src/ file count changes today.

Verification at b5502ee9. I ran each class on its own, in Debug and in Release, and all are green: SourcePreprocessor 9, TenantBypass 56, ModuleLedger 40, PeerContractRealTree 1, AdapterReachRealTree 2, AdapterTier 32, CompatibilityExceptionRealTree 2, CouplingMatrixRealTree 2, InsightsReadOnly 25, EggLotLockSql 2, SeamSurface 41, Documentation 20. The two ImagePin tests are also green. The earlier mutants O5 and M2–M6 are still red. Application.Tests lists 703 tests at main and 713 at head. I did not run the full suite or the integration suite locally. The PR body has the updated change map and mutation table.

[mforce]

Verdict: approve the fix round with one nonblocking P3 follow-up. No PRODUCT defect or blocking guard defect remains. F1 is fixed, F2 now uses the compiler symbols, and the original F3 node_modules mutation is caught. The shared walk retains a narrower directory blind spot described below.

Reviewed b5502ee9bb835571ed34582552ed2b7fbc07a4fe, including the fix delta from b4b5c2ba and the full PR diff against cd1de3a8. The full supplied PR diff matches Git. No delegation, product fixes or pushes.

Correction to my first review. The worker is right about SourcePreprocessorTests. I checked out the exact old head, c2e91f75, and repeated A2 in Debug and Release. FindBySlugCallerTests passed, but SourcePreprocessorTests.RealSourceTree_HasNoInactiveCode failed in both configurations, naming CreateProductHandler.cs:21. My 21-test set omitted that class. A2 demonstrated an incomplete parser and an incorrect dead-code rejection, not a way through the complete CI guard set. I withdraw that implication from the earlier F2 finding.

F1 is closed. The allowance now uses GuardScanner.EnclosingSymbolOf and names only IdentityProvider.ResolveFarmCodeAsync. A1, adding OtherFarmNameAsync in the same file, fails and names that helper. A1b, moving the real call into a sibling FindAsync and forwarding login through it, also fails. Both fail in Debug and Release. The reference list preserves duplicates; another method can no longer substitute for login's positive control. This is the existing symbol helper applied at the right boundary.

F2 is closed; the complementary inactive-code check remains necessary. At ModuleLedgerScanner.cs:42–48, the symbols come from the test assembly's metadata. The csproj target at line 29 records them after AddImplicitDefineConstants. A2 now fails the caller guard in both configurations, while the inactive-code check correctly accepts that compiler-active region. Removing the metadata target makes all 18 tests across the caller and preprocessor classes fail in each configuration with no DefineConstants metadata.

The metadata is a snapshot of the test project's build, not a per-source-project evaluation. I evaluated all five src projects plus Application.Tests using MSBuild: all six use net10.0 and identical symbol sets within each configuration. The generated assembly metadata contains the SDK compatibility symbols and changes from DEBUG to RELEASE with the build. I found no stale-metadata or current TFM/configuration mismatch.

I also tested divergent source settings rather than assuming they could not occur:

Mutation	Debug	Release
Application alone defines ASTRA_SOURCE_ONLY; O5 is inside #if ASTRA_SOURCE_ONLY	Caller scan misses it, but SourcePreprocessorTests and AdapterTierRealTreeTests reject it	Same
Application alone sets DefineConstants to TRACE; O5 is inside #if !DEBUG	Inactive-code check rejects it	Caller guard rejects it
Delete EmbedDefineConstants	All 18 caller/preprocessor tests fail	All 18 fail

These results support fail-closed handling of compiler/parser drift for scanned source. The metadata alone does not prove future projects have matching settings. I did not install or build a different TFM; a symbol unknown to the test parser is covered by the inactive-code check, as the source-only-symbol mutation demonstrates.

F3's original mutation is closed; one P3 blind spot remains. A wrapper under src/Cluckwork.Application/node_modules/ now fails the caller guard. So does a wrapper under src/Cluckwork.Application/Probe/bin/ when Probe has no project file. Both results hold in Debug and Release. Ordinary project-root bin/obj outputs remain excluded, and I found no current-tree false positive.

However, GuardScanner.cs:1386–1389 treats any directory containing a .csproj as an output-exclusion boundary. Add src/Cluckwork.Application/Probe/Nested.csproj containing <Project Sdk="Microsoft.NET.Sdk" />. The same Probe/bin/HiddenLookup.cs is now skipped, although the outer Application project still compiles it. Calling its ReadAsync(accounts, ct) from CreateProductHandler reaches FindBySlugAsync without changing a registry. The mutant builds and all 32 targeted checks pass in both configurations, including SourcePreprocessorTests and AdapterTierRealTreeTests.

This requires additional source and a nested project marker; no such caller exists at head. It is a residual form of the pre-existing directory blind spot, not a new regression or a reason to block this improvement. A follow-up should derive output exclusions from actual build-project boundaries, or prove that omitted files cannot belong to an enclosing project's compile inputs. The comment that the walk mirrors SDK exclusions is broader than this case supports.

The 32-test mutant set comprised FindBySlugCallerTests (9), SourcePreprocessorTests (9), ModuleLedgerRealTreeTests (2), PeerContractRealTreeTests (1), AdapterReachRealTreeTests (2), AdapterTierRealTreeTests (3), TenantBypassRealTreeTests (4), and CompatibilityExceptionRealTreeTests (2). This is targeted guard evidence, not a claim that the full CI or HTTP suite passed that mutant.

Shared-scanner and quality review. I applied the thermo-nuclear and ponytail checks to the fix delta. Reusing the existing symbol extractor and MSBuild assembly-metadata support keeps the fix small. No new package, product abstraction, or file-size threshold crossing. The changes broaden source discovery and remove the hand-maintained framework list. No new structural or over-engineering finding; the P3 exclusion-boundary issue is the only remaining guard concern. Ponytail, complexity only: Lean already. Ship.

AccountRepository.cs and the token-hash registry filter-free-set-sites.tsv remain byte-identical to the first reviewed head. No product query or stored hash changed.

Restored-head verification: 340/340 targeted tests passed in Debug and 340/340 in Release across 24 classes per configuration, each run separately with DOTNET_USE_POLLING_FILE_WATCHER=1. Coverage includes the tenant-bypass, module, peer, adapter, compatibility and coupling guards, plus direct shared-parser consumers. All mutations are reverted, the assigned head is restored, git status --porcelain is empty, and the fix diff passes --check. The full solution and integration/HTTP suites were not run.

Exact mutation scripts, patches, per-class logs, evaluated compiler properties and the old-head reproduction are retained in /home/mforce/.cluckwork-slices/reviews/1061-b5502ee9-astra-evidence/. Report: /home/mforce/.cluckwork-slices/reviews/1061-b5502ee9-astra.md.