11.28.0: unify the 401/403 policy and fix S_SELF/S_CREATOR ownership across the permission layers

[DKoenig9]

Rebased onto 11.27.7 (#560). The commit history was rebuilt on the current develop; the branch now targets 11.27.7 → 11.28.0. It also absorbs the former fix/s-self-input-path — the ownership fix ships here, in one release, so consumers audit their @Restricted fields once.

What this does

Two related security/consistency fixes to the permission checks, plus one separable test-infra fix.

1. One 401/403 policy across all five permission layers

Permission errors for authenticated users were thrown as 401. Frontends treat 401 as "session expired" and auto-logout (the @lenne.tech/nuxt-extensions interceptor patches $fetch/fetch globally), so a mere permission error kicked logged-in users out of the whole app.

accessDeniedException(user, message?) — a factory, not a class — derives the status from the requester's auth state and returns the native ForbiddenException / UnauthorizedException, so instanceof and @Catch(...) filters in consumer projects keep working and the REST wire body is unchanged. (A custom HttpException subclass, which an earlier draft used, breaks both — that was the review's headline finding.)

Layer	Was	Now
check() / checkRestricted() / prepareInput()	401 for authenticated	403 authenticated, 401 anonymous
CoreTenantMemberModel.securityCheck()	401 for an authenticated non-member (the bug)	403
S_NO_ONE	401 (guards) / 403 (tenant) / mixed (check)	403 for everyone — authenticating can never unlock it
CoreTenantGuard, no user	403 (inverted)	401

Messages are the translatable ErrorCode.ACCESS_DENIED / ErrorCode.UNAUTHORIZED the guards already throw (the old raw strings had no #LTNS_ marker, so the frontend couldn't translate them, and they leaked internal class/field names — those go to the debug log now).

2. Ownership (S_SELF / S_CREATOR) read from the persisted object, not the payload

checkRestricted() decided S_SELF/S_CREATOR from data. On the input path data is the caller DTO, so an authenticated attacker could unlock an owner-restricted field on someone else's record by asserting ownership in the body — the service writes to the target it was called with, not to the ids in the payload:

PATCH /users/<victim>  { "id": "<attacker>", "createdBy": "<attacker>", "<restricted>": ... }

Three reads were affected — S_SELF, S_CREATOR, and the isCreatorOfParent trust that propagates into nested objects — all now read from config.dbObject on input, matching check(), which always did. Output is unchanged (data IS the persisted object there); create() has no dbObject, so ownership can't be established — matching check().

This is latent hardening, not a live-exploit fix: MapAndValidatePipe strips id/createdBy from payloads, so the branch usually couldn't fire and the fields were effectively admin-only. But it must be audited before adopting — a field that looked owner-restricted may become writable once it starts working. An instance-wide scan of all consumer projects found none currently exploitable; the migration guide has the audit steps and the S_CREATOR trap (in invite flows the inviter, not the user, is the "creator").

Breaking changes

Five, all in migration-guides/11.27.7-to-11.28.0.md with before/after and search recipes:

1. Authenticated permission errors → 403
2. S_NO_ONE → 403 for everyone
3. Permission-error messages → ErrorCodes
4. CoreTenantGuard → 401 when unauthenticated
5. S_SELF/S_CREATOR on input fields now enforce ownership — audit required

Also

• core-better-auth.controller re-throws instanceof HttpException instead of an enumeration that masked everything it forgot as a 500.
• FRAMEWORK-API.md gained an Errors & Status Codes section (extracted from source, so a new exception can't stay invisible).
• versioning.md now documents the spectaql.yml version lockstep — a recurring release miss.

Tests

The unit spec asserts the contract that actually broke — instanceof + wire format — plus all five layers, S_NO_ONE across paths, and the ownership fix in both directions (attack → 403, owner allowed). A test-expectation bug (a 401 asserted where an authenticated attacker correctly gets 403) surfaced during the rebuild and was fixed. Full pnpm run check green, including #560's check:swc-tdz (the new accessDeniedException import introduces no cycle).

Second commit is separable

fix(tests): stop wiping the shared jwks keyset fixes a pre-existing bug (confirmed on develop): one e2e file dropped the shared BetterAuth signing keyset in a beforeAll, stranding JWTs that other parallel files had already minted. Kept here only because the pipeline can't go green without it; cherry-pick it separately if preferred.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adjusts authorization error semantics so authenticated users who lack permissions receive 403 Forbidden (instead of 401), aligning service-layer rights checks with the existing RolesGuard behavior and preventing frontend “auto-logout on 401” flows from triggering on mere permission failures.

Changes:

• Switch permission-denied paths in check(), checkRestricted(), and prepareInput() to throw ForbiddenException when user/currentUser.id is present, keeping UnauthorizedException for unauthenticated callers.
• Update the affected e2e assertion from 401 → 403 and document the new status-code semantics.
• Bump package version to 11.28.0 and add a migration guide for consumer updates.

Reviewed changes

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

Show a summary per file

File	Description
tests/server.e2e-spec.ts	Updates e2e assertion to expect 403 for authenticated permission failure.
src/core/common/helpers/service.helper.ts	Throws 403 (ForbiddenException) when authenticated users attempt disallowed role assignment.
src/core/common/helpers/input.helper.ts	Throws 403 for authenticated “Missing rights/No access” cases, retains 401 for unauthenticated.
src/core/common/decorators/restricted.decorator.ts	Throws 403 for authenticated @Restricted violations, retains 401 for unauthenticated.
package.json	Version bump to 11.28.0.
migration-guides/11.27.6-to-11.28.0.md	Adds migration documentation for 401 → 403 change.
FRAMEWORK-API.md	Regenerated header with new date/version.
docs/REQUEST-LIFECYCLE.md	Documents the 401 vs 403 semantics for rights checks starting v11.28.0.

[DKoenig9]

Fixed in 5c3eba3: the Behavior-Changes row now names the exception-type switch explicitly and lists all consumer-visible effects (REST body "error": "Forbidden" instead of "Unauthorized", GraphQL originalError.error, and instanceof UnauthorizedException matches in exception filters/interceptors). The "Who is affected" section gained a matching entry for server-side exception filters and extended test guidance.

[DKoenig9]

Review follow-up addressed (from the internal Linear review, DEV-2489):

• 1358987 — the auth-state ternary (user?.id ? ForbiddenException : UnauthorizedException) was duplicated 6× across 3 files; it is now centralized in the new AccessDeniedException (src/core/common/exceptions/, matching the existing exceptions convention). It extends HttpException, picks 403/401 in the constructor, and builds the body via HttpException.createBody — a new unit spec asserts exact body parity with the native Forbidden/Unauthorized exceptions for both auth states, so responses cannot drift from the RolesGuard. Placed under common/ (not auth/exceptions/) so common does not depend on a module; exported via src/index.ts.
• 5c3eba3 — migration-guide clarification from the Copilot review comment (exception type changes, not just the status code).

Suites after the refactor: 1380/1380 e2e + 694/694 unit, lint/format/build green. The consuming GEQ project mirrors this state 1:1 in its vendored core.

[DKoenig9]

Multi-agent review of the full PR (incl. 909f153 + cc3fea5) — findings fixed in df33984

Per the nest-server change process, the complete diff (develop..HEAD, all 5 commits) went through another /lt-dev:review-style pass — three parallel reviewers (backend conventions, security semantics, test quality) plus a full pnpm run check.

Security verdict: SHIP. The 401/403 decision is derived from a non-spoofable signal (user.id from guard-validated request state) at every call site, fails closed to 401, and the change strictly reduces information disclosure (class/field names + rejected role lists moved to debug logs). The better-auth controller catch-broadening closes a real masking bug (rights-403s no longer collapse to 500).

Findings that were fixed in df33984:

1. checkRestricted() did not implement the documented S_NO_ONE → 403 always invariant (backend + security, found independently). The docs shipped in this PR (REQUEST-LIFECYCLE.md, role-system.md, migration guide BC2) declare 403-for-everyone for S_NO_ONE, but both checkRestricted throw sites routed through accessDeniedException → anonymous requesters got 401. Fixed: both sites now special-case S_NO_ONE (same rationale comment as the guards), with the flatten logic extracted so the detection honors processType-scoped restrictions.
2. Two of the five layers had zero test execution (test review): BetterAuthRolesGuard (only registered in IAM-only setups — no story file exercised it) and prepareInput's roles processing. New tests/unit/access-denied-call-sites.spec.ts (15 tests) pins all previously unexercised layers in both directions (401 anonymous / 403 authenticated / S_NO_ONE), including that rejected role names no longer leak to the client.
3. Migration guide gaps: BC3 now lists the remaining changed messages (Missing roles of current user, Access to tenant membership denied, Authentication required, tenant-guard Access denied) with matching Step-3 greps (exact toEqual('Access denied') assertions break; toContain survives); the BC2 table names checkRestricted() explicitly.
4. Minors: service.helper rejected-roles log now goes through Logger.debug (respects log level) instead of a bare console.debug; the anonymous-401 e2e asserts the ErrorCode its comment promised; unit spec imports vitest explicitly.

Validation: full pnpm run check green on the final state — 2095 tests / 74 files, lint + format clean, build + server-start ok, pnpm audit 0 findings.

Documented follow-ups (deliberately NOT in this PR):

• The AI module services are an un-unified "sixth layer": core-ai-prompt.service.ts:42/144 throw 403 with a raw string for missing authentication (should be 401 per this PR's policy), and core-ai-prompt.service.ts:161 / core-ai-slot.service.ts:354/479 return raw-string 403s (one interpolating internal error messages). Normally shadowed by guards, reachable via internal/MCP call paths → separate PR migrating them to accessDeniedException + ErrorCode.
• RolesGuard.checkGraphQL's S_NO_ONE branch has no direct test (only the REST canActivate copy is exercised) — would need a @Roles(S_NO_ONE) test resolver.

From my side the PR is ready to merge into develop.