Skip to content

Commit 45a33e7

Browse files
committed
chore(auth): sweep the archaeological requireAuth comments — and one dead code block (#3963)
Follow-up cleanup after retiring the opt-out. Most were stale comments asserting behaviour that no longer exists ("no-op when requireAuth is off", "when requireAuth is set", "requireAuth=true deployments"), updated to the unconditional anonymous-deny posture across rest-server, the runtime domains, plugin-security, plugin-approvals, and metadata-protocol. One was NOT just a comment: plugin-hono-server still computed `const requireAuth = restConfig?.api?.requireAuth ?? true` and logged a boot warning off it — dead plumbing, since denyAnonymous stopped reading requireAuth. Removed. hono suite green (116). Also merges origin/main (rest.test.ts discovery-route conflict — the route is control-plane/allowlisted, so it takes the serviceExists constructor from main and drops the unnecessary auth injection). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzLE9cw4gZKNyPN2ZP4iTt
1 parent 86cb4fb commit 45a33e7

6 files changed

Lines changed: 34 additions & 47 deletions

File tree

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2664,8 +2664,8 @@ export class ObjectStackProtocolImplementation implements
26642664
// it was not swept into the implicit-filter bucket either: a caller's
26652665
// `context` survived this spread and, because the assignment below is
26662666
// conditional, became the operation's execution context whenever no
2667-
// server context resolved (an anonymous request on a deployment that
2668-
// set `requireAuth: false`).
2667+
// server context resolved (before #3963 an anonymous request on a
2668+
// `requireAuth: false` deployment; the invariant holds regardless).
26692669
//
26702670
// What rides on it is total: plugin-security's middleware opens with
26712671
// `if (opCtx.context?.isSystem) return next()` — the entire RLS / FLS /

‎packages/plugins/plugin-approvals/src/approval-service.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -639,9 +639,10 @@ export class ApprovalService implements IApprovalService {
639639
* (having also put them on the context). Those are the only two callers that
640640
* hold a trustworthy actor with no session behind them.
641641
*
642-
* A caller with NO identity at all cannot act. That case is reachable: the
643-
* REST anonymous-deny only fires when `api.requireAuth` is set, so without it
644-
* an anonymous request previously decided approvals outright by naming one.
642+
* A caller with NO identity at all cannot act. Belt-and-suspenders: the REST
643+
* anonymous-deny now denies every anonymous request (#3963), but this service
644+
* must not rely on a caller upstream — an anonymous actor could otherwise
645+
* decide approvals outright by naming one.
645646
*/
646647
private async resolveActor(
647648
actorId: string | undefined,

‎packages/plugins/plugin-hono-server/src/hono-plugin.ts‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -813,22 +813,9 @@ export class HonoServerPlugin implements Plugin {
813813
// failing. Gating here makes the deny decision a property of THIS entry
814814
// point too, so security no longer depends on who registered first.
815815
//
816-
// Secure-by-default: `requireAuth` mirrors `rest-server.ts`'s `?? true`
817-
// (ADR-0056 D2). A deployment that intentionally serves data publicly
818-
// sets `restConfig.api.requireAuth = false` (a boot warning is logged, as
819-
// in the REST plugin). No-op in that case — the previously-public surface
820-
// is unchanged. An authenticated / system caller always passes.
821-
//
822-
// `requireAuth` is not in the typed `api` shape (rest-server.ts reads it
823-
// via the same `as any` cast), so widen locally.
824-
const requireAuth =
825-
(this.options.restConfig?.api as { requireAuth?: boolean } | undefined)?.requireAuth ?? true;
826-
if (!requireAuth) {
827-
ctx.logger.warn(
828-
'Hono standard /data endpoints: requireAuth is OFF — anonymous callers can read/write object data. ' +
829-
'This is a deliberate opt-out; set restConfig.requireAuth=true to deny anonymous access (ADR-0056 D2, #2567).',
830-
);
831-
}
816+
// [#3963] Anonymous access to object data is denied unconditionally —
817+
// the `requireAuth` opt-out is retired, so there is no posture to read
818+
// or warn about here. An authenticated / system caller always passes.
832819
// Returns a 401 Response when the caller is anonymous, else null
833820
// (caller proceeds). Delegates the decision to the
834821
// shared `shouldDenyAnonymous` (#2567) so every HTTP seam stays in

‎packages/plugins/plugin-security/src/security-plugin.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ export class SecurityPlugin implements Plugin {
771771
// form's declared target (set by the rest-server form-submit route). It
772772
// authorizes ONLY create + the immediate read-back on THAT object — never
773773
// anything else, and never the anonymous fall-open. This lets public forms
774-
// work under secure-by-default (requireAuth) WITHOUT a deployment-configured
774+
// work under secure-by-default (anonymous-deny) WITHOUT a deployment-configured
775775
// `guest_portal`, scoped to exactly the declared object (the field
776776
// allow-list is enforced at the route; the context is request-scoped).
777777
const formGrant = opCtx.context?.publicFormGrant;

‎packages/rest/src/rest-server.ts‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,8 +1222,9 @@ export class RestServer {
12221222
}
12231223

12241224
/**
1225-
* Reject anonymous requests with HTTP 401 when `api.requireAuth` is set.
1226-
* Returns `true` if the response was sent and the caller should stop
1225+
* Reject anonymous requests with HTTP 401 — unconditionally (#3963: the
1226+
* `api.requireAuth` opt-out is retired). Returns `true` if the response was
1227+
* sent and the caller should stop
12271228
* processing. Returns `false` to continue.
12281229
*
12291230
* The check is intentionally narrow: only `context?.userId` counts as
@@ -1232,7 +1233,7 @@ export class RestServer {
12321233
*/
12331234
private enforceAuth(req: any, res: any, context: any): boolean {
12341235
// ADR-0069 — authentication-policy gate (password expiry, enforced MFA).
1235-
// Independent of `requireAuth`: a gated session (carrying `authGate`) is
1236+
// Independent of the anonymous-deny: a gated session (carrying `authGate`) is
12361237
// blocked from protected resources, while the core allow-list keeps auth
12371238
// + remediation reachable. Runs before the anonymous check.
12381239
const gate = context?.authGate;
@@ -2544,23 +2545,22 @@ export class RestServer {
25442545
}
25452546

25462547
/**
2547-
* Register the metadata routes behind the SAME `requireAuth` gate the
2548+
* Register the metadata routes behind the SAME anonymous-deny gate the
25482549
* `/data` routes use.
25492550
*
25502551
* `registerMetadataEndpoints` builds ~17 `/meta/*` routes but — unlike the
25512552
* `/data` handlers — never calls {@link enforceAuth}: its handlers assumed
2552-
* the `requireAuth` gate rejected anonymous callers "upstream", yet nothing
2553-
* upstream covers `/meta`, so on a `requireAuth` deployment an anonymous
2554-
* caller could read object / field schemas. On a tenant-less runtime host
2555-
* those are SYSTEM-object schemas and the host is publicly reachable — a
2556-
* real leak.
2553+
* the anonymous-deny rejected anonymous callers "upstream", yet nothing
2554+
* upstream covers `/meta`, so an anonymous caller could read object / field
2555+
* schemas. On a tenant-less runtime host those are SYSTEM-object schemas and
2556+
* the host is publicly reachable — a real leak.
25572557
*
25582558
* Rather than add the gate to every handler (and have the next new route
25592559
* forget it — the exact failure mode that caused this), wrap the route
25602560
* registrar for the duration of registration so every meta route, present
2561-
* and future, inherits it. The check is a no-op when `requireAuth` is off
2562-
* (demo / single-tenant), so the previously-public metadata surface there
2563-
* is unchanged; an authenticated user passes exactly as on `/data`.
2561+
* and future, inherits it. An authenticated user passes exactly as on
2562+
* `/data`; the one exception is the declaration-derived public-book read
2563+
* (#3963), handled just below.
25642564
*/
25652565
private registerMetadataEndpoints(basePath: string): void {
25662566
const realRouteManager = this.routeManager;
@@ -2579,7 +2579,7 @@ export class RestServer {
25792579
const context = await this.resolveExecCtx(environmentId, req).catch(() => undefined);
25802580
// [#3963] `audience: 'public'` is a DECLARED capability, so it
25812581
// must not depend on a deployment flipping its whole data plane
2582-
// open (`requireAuth: false`). An anonymous read of the
2582+
// open. An anonymous read of the
25832583
// book/doc surface skips the anonymous-deny and is authorized
25842584
// instead by the ADR-0046 §6.7 audience gate inside the handler
25852585
// — the same declaration-derived shape ADR-0056 Option A chose
@@ -2756,7 +2756,7 @@ export class RestServer {
27562756
// privileged apps (Studio, Setup, etc.) and gated nav
27572757
// items are stripped before reaching the client. We
27582758
// intentionally leave anonymous responses untouched —
2759-
// the existing `requireAuth` gate (when enabled) blocks
2759+
// the anonymous-deny gate blocks
27602760
// them upstream; when disabled, the demo / public
27612761
// surface keeps its prior behaviour.
27622762
//
@@ -5091,14 +5091,14 @@ export class RestServer {
50915091
* GET {basePath}/forms/:slug → resolved form spec
50925092
* POST {basePath}/forms/:slug/submit → INSERT record (no auth required)
50935093
*
5094-
* Both routes bypass `enforceAuth` even when `requireAuth=true` on the
5094+
* Both routes bypass `enforceAuth` even though anonymous-deny is on for the
50955095
* deployment (e.g. ObjectOS multi-tenant). Security is delegated to the
50965096
* `guest_portal` permission set carried on the execution context — the
50975097
* SecurityPlugin enforces INSERT-only access to the target object. If
50985098
* the deployment hasn't registered a `guest_portal` profile, the
50995099
* security middleware falls open with `permissions: []` (no userId),
51005100
* matching the existing anonymous-access semantics; deployers must
5101-
* keep `requireAuth=true` deployments paired with a `guest_portal`
5101+
* keep secure-by-default deployments paired with a `guest_portal`
51025102
* profile (the CRM example does this) to enforce the INSERT-only
51035103
* contract.
51045104
*
@@ -5370,7 +5370,7 @@ export class RestServer {
53705370
// form — a narrow create grant scoped to exactly this form's target
53715371
// object. The SecurityPlugin honors `publicFormGrant` (create + the
53725372
// immediate read-back, that object ONLY), so public forms work under
5373-
// secure-by-default (requireAuth) WITHOUT a deployment-configured
5373+
// secure-by-default (anonymous-deny) WITHOUT a deployment-configured
53745374
// `guest_portal`. `guest_portal` + `anonymous` are kept for back-compat
53755375
// with object hooks (guest detection via falsy `ctx.user?.id`).
53765376
const context: any = {
@@ -5728,8 +5728,8 @@ export class RestServer {
57285728
const context = await this.resolveExecCtx(environmentId, req);
57295729
if (this.enforceAuth(req, res, context)) return;
57305730
if (!context?.userId) {
5731-
// Even on requireAuth=false deployments the explain surface
5732-
// stays authenticated-only — it is an admin diagnosis tool.
5731+
// The explain surface stays authenticated-only — it is an
5732+
// admin diagnosis tool. (Anonymous is already 401ed above.)
57335733
return res.status(401).json({
57345734
code: 'UNAUTHORIZED',
57355735
message: 'The access-explanation endpoint requires an authenticated caller.',
@@ -7125,7 +7125,7 @@ export class RestServer {
71257125
// (ADR-0049) was enforced on A while B was written. Zod also
71267126
// strips unknown keys, which keeps a body `context` from
71277127
// becoming the execution context on a deployment where none
7128-
// resolves (anonymous-reachable `requireAuth: false`).
7128+
// resolves (e.g. an anonymous public-book read, #3963).
71297129
const { UpdateManyDataRequestSchema } = await import('@objectstack/spec/api');
71307130
const updateManyInput = { ...(req.body ?? {}), object: req.params.object };
71317131
const parsedUpdate = (UpdateManyDataRequestSchema as any).safeParse(updateManyInput);

‎packages/runtime/src/domains/security.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@ export async function handleSecurityRequest(
5050
}
5151

5252
const ec = context.executionContext;
53-
// Admin surface — anonymous is denied UNCONDITIONALLY (`requireAuth:
54-
// true` hardcoded), independent of the deployment posture: even a
55-
// `requireAuth: false` demo must not let anonymous callers list or
56-
// confirm audience bindings. Shares the decision + body with every
57-
// other HTTP seam (#2567).
53+
// Admin surface — anonymous is denied UNCONDITIONALLY (#2567, #3963):
54+
// even before the opt-out was retired this seam never honoured it, so an
55+
// anonymous caller could never list or confirm audience bindings. Shares
56+
// the decision + body with every other HTTP seam.
5857
if (shouldDenyAnonymous({ userId: ec?.userId, isSystem: ec?.isSystem })) {
5958
return {
6059
handled: true,

0 commit comments

Comments
 (0)