Skip to content

open-edition-honest-degradation c3: anonymous-deny ordering inverted on /ai/** (unauth /ai/models → 501, /ai/agents → 200 instead of 401) #7653

Closed

Description

@huangyiirene

The degradation itself is honest and PASSES — please do not "fix" that half: /ai/models, /conversations, /usage, /chat all 501 with the Cloud/EE remedy sentence (mounted-but-unimplemented, never 404/503); /ai/agents returns the declared envelope with the relocated payload; and /discovery's services.ai message is string-identical to the 501 body. The defect below is only the ordering of the anonymous-deny gate.

Symptom

Unauthenticated callers reach the /ai/** family instead of being denied.

  • Observed: unauth GET /api/v1/ai/models → 501 (leaking the Cloud/EE remedy sentence); unauth GET /api/v1/ai/agents → 200. Reproduced twice.
  • Expected: both → 401.
  • Control (proves anonymous-deny is otherwise live on the same boot): anonymous GET /api/v1/data/showcase_task → 401, correctly.

Disclosure impact is limited to two static strings, but the ordering contract this item exists to pin is inverted.

Root cause

runtime/src/domains/ai.ts handleAIRequest returns the /ai/agents empty-list courtesy ({ agents: [] }) and capabilityUnavailable(deps, 'ai') at the top of the handler (around ai.ts:68-79), while the shouldDenyAnonymous gate lives inside the per-route loop further down (ai.ts:128) — reachable only when the AI service is serveable. So on every open-edition deployment the whole /ai/** family answers unauthenticated callers before the anonymous gate is ever consulted. Still present on origin/main (the courtesy + capabilityUnavailable return precedes the shouldDenyAnonymous check).

Reproduction

On a stock boot, with no Authorization header:

curl http://<host>/api/v1/ai/models    # expected 401, actual 501
curl http://<host>/api/v1/ai/agents    # expected 401, actual 200
curl http://<host>/api/v1/data/showcase_task   # control: 401 (correct)

Source

Extracted from the QA run #7627 (framework 92f26f7, console 6314e87f).

Activity

  1. self-assigned this
    on Aug 12, 2026
  2. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Claim: PM loop round 2 — domain:cli seat.

    Scope boundary (the card is explicit and I am holding it)

    ⛔ The honest-degradation half PASSES and must not be "fixed" — /ai/models, /conversations, /usage, /chat answering 501 with the Cloud/EE remedy sentence (never 404/503), /ai/agents returning the declared envelope, and /discovery's services.ai message being string-identical to the 501 body are all correct today. A diff that changes any of them is a regression.

    The defect in scope is ordering only: handleAIRequest returns the /ai/agents courtesy and capabilityUnavailable(deps, 'ai') at the top of the handler, while shouldDenyAnonymous sits inside the per-route loop below — reachable only when the AI service is serveable. So on every open-edition deployment the whole /ai/** family answers unauthenticated callers before the anonymous gate is consulted.

    Pin the consequence, in the order the contract states

    The three-way assertion the card measured is the acceptance criterion: unauth /ai/models → 401 (not 501), unauth /ai/agents → 401 (not 200), and the control — anonymous /api/v1/data/showcase_task → 401 — still correct, proving the gate was live on the same boot and this is ordering rather than absence. Add the authenticated-caller cases too: with a valid principal the 501s and the /ai/agents envelope must come back unchanged, or "fixed the ordering" has silently become "broke the degradation".

    ⚠️ Reverse-verify: with the reordering reverted, the two 401 assertions must go red and the degradation assertions must stay green.


    Generated by Claude Code

  3. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor
    {
      "issue": 7653,
      "status": "complete",
      "branch": "claude/issue-7653-ai-anon-deny-ordering",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/7910",
      "premise_still_valid": true,
      "summary": {
        "premise_verification": "The card's mechanism held exactly on origin/main @ d91fad5, with zero line drift: the /ai/agents empty-list courtesy returns at ai.ts:68-70, capabilityUnavailable(deps,'ai') at ai.ts:79, and shouldDenyAnonymous sat inside the per-route loop at ai.ts:128 — reachable only when the AI service is serveable. Serveability decided whether the gate ran at all.",
        "fix": "The anonymous-deny decision is taken ONCE at the top of handleAIRequest and consulted at the exits — one shouldDenyAnonymous call, one refusal builder, no second copy of the rule. The route-level `auth: false` opt-out deliberately stays in the loop: it is a property of a REGISTERED route and can only be honoured where a route table exists, so with no serveable service the family default (auth required) stands. This is why the gate was not simply short-circuited at the top — a blanket 401 would satisfy every positive assertion and silently close routes a Cloud deployment legitimately opens (pinned by test C2).",
        "scope_note": "One extra exit in the same handler took the same treatment: the `!routes` 503 ('AI service routes not yet initialized') also preceded the gate, so a boot-race window disclosed 'AI is mounted here' to anonymous callers. Same defect class, same file, one-line consult — flagged in the PR body as slightly beyond the two paths the issue measured.",
        "degradation_untouched": "Confirmed in both directions. With an authenticated or internal SYSTEM principal: /ai/models, /conversations, /usage, /chat still answer 501 (never 404, never 503) with serviceUnavailableMessage('ai') verbatim; /ai/agents still returns the declared envelope with the payload RELOCATED under data.agents (asserted not-an-array, so a flatten regression is caught); the 501 body is asserted string-identical to what /discovery reports for the ai slot, both sides calling the one producer.",
        "sibling_survey": "All six dispatcher domains checked for the same inversion. meta.ts (:172), actions.ts (:129), packages.ts (:224) and automation.ts (gate :156 ahead of capabilityUnavailable :174) all gate correctly. security.ts is inverted the same way and was NOT fixed here — filed separately per the card."
      },
      "tests": {
        "added_file": "packages/runtime/src/domains/ai-anonymous-deny-ordering.test.ts (13 cases, 4 groups)",
        "envelope_assertions": "ADR-0112 compliant — every refusal asserts status === 401 === ANONYMOUS_DENY_STATUS AND error.code === 'UNAUTHENTICATED' === ANONYMOUS_DENY_CODE, plus error.httpStatus. The test's deps.error is the REAL builder (apiErrorResponse, as http-dispatcher.ts wires it), not a stub dropping the third argument — with a stub, details.code would never be promoted and every code assertion would be vacuous.",
        "reverse_verification": "With ONLY the reordering reverted and tests untouched: 5 failed | 8 passed (13). Every red is an anonymous-deny assertion (A: /ai/models 501→expected 401; A: /ai/agents 200→expected 401; A: resolved-but-sessionless shape; A: rest of the family; D: 503→expected 401). ALL degradation assertions (Group B, 4 cases) and ALL controls (Group C, 3 cases) STAYED GREEN. Explicitly confirmed: the degradation assertions did not move in either direction.",
        "control": "Group C is the unit-level form of the issue's /data control — with a SERVEABLE service this handler already denied anonymous before the change, which is what makes the defect ordering rather than absence. The QA boot's anonymous GET /api/v1/data/showcase_task → 401 is served by @objectstack/rest, outside the declared file surface, so it was not re-measured here.",
        "existing_tests_adjusted": "Three pre-existing degradation cases carried INCIDENTALLY anonymous fixtures and so measured the courtesy/501 through the very hole this closes: '/ai — a stub slot 501s per route…' (http-dispatcher.test.ts) and '/ai/agents returns an empty list (not 404)…' + '/ai routes 501 (service missing)…' (domain-handler-registry.test.ts). They now seed a principal — the convention the /notifications stub-slot case directly above one of them already used. The latter two ran through dispatch(), which re-resolves identity off the auth-less mock kernel and overwrites a seeded context, so they call handleAI directly (same bypass the /keys and /automation cases in that file use). Their end-to-end registry coverage was KEPT, not dropped, as a new case asserting /ai/** denies anonymous through the full registry path. No assertion was weakened or deleted.",
        "gates": {
          "derived": "node scripts/pm/dispatch-gates.mjs packages/runtime/src/domains/ai.ts → check:route-envelope, check:changeset-gate-self-tests",
          "check:route-envelope": "PASS — 9 route modules, dispatcher domains 16 audited, 0 newly ratcheted",
          "check:changeset-gate-self-tests": "PASS — 118 + 153 + 117 assertions",
          "check:nul-bytes": "PASS — 7232 files",
          "check-empty-changeset / check-adr-0087-registration / check-changeset-no-major": "PASS on the committed diff — 1 declaring, non-breaking, no major",
          "eslint": "PASS (exit 0) on all 4 touched files",
          "package_suite": "pnpm --filter @objectstack/runtime exec vitest run → 138 files passed, 2100/2100 tests passed",
          "build_closure": "turbo run build --filter='./packages/*' --filter='./packages/*/*' → 70/70 successful, run BEFORE the type-check ratchet as the script itself demands",
          "check:type-check-debt": "PASS — 33 ledger entries re-measured, 1789 raw errors, none above its recorded number. @objectstack/runtime's TEST_DEBT ceiling is 227; measured total exactly 227, UNCHANGED. Per-file contribution of the touched files: ai.ts 0, ai-anonymous-deny-ordering.test.ts 0, domain-handler-registry.test.ts 0. The 18 in http-dispatcher.test.ts are pre-existing stubbed() duck-typing / possibly-undefined errors (the one now at 3131 is the same one previously at 3124, shifted by added lines). NO debt ceiling was raised or lowered.",
          "ci": "23 checks; at report time 10 completed green (Check Changeset, ESLint pending, Check PR Size, Console Pin Freshness, ADR maintainer approval, Documentation Links, docs-drift flag, 'No other open PR may claim the same issue', Auto Label, filter), 0 failures, remainder in progress. PR subscription is active and the drive-to-green posture applies."
        }
      },
      "open_questions": [
        "RESOLVED BY ME, recorded for review: whether to hoist the gate unconditionally to the very top (the PM's sketch) or preserve the route-level `auth: false` opt-out. I preserved it. A full hoist would drop the opt-out for AI routes registered by cloud's service-ai — routes this repo cannot see — turning an ordering fix into a silent closure of legitimately public routes. Test C2 ('still SERVES anonymous on a route that declares auth: false') exists specifically to fail if anyone later collapses it. The result is one rule with two consult sites, not two copies of the rule; if the reviewer prefers a single site, it costs the opt-out.",
        "NOT CHANGED, flagged for a decision: with a serveable service and a route table present, an anonymous caller hitting an UNMATCHED /ai path still gets 404 routeNotFound rather than 401, so route existence remains distinguishable by an anonymous caller. That is a pre-existing property of the per-route auth design rather than the ordering defect, and it is unreachable on the open-edition boot this issue measures (everything 401s there). Left alone to avoid widening scope; say the word and it is a two-line follow-up.",
        "Reproduction was at unit level against the real handler, not a live HTTP boot — this container has no showcase app running. The QA run #7627 already measured the live boot twice, and the reverse-verification pins the same three-way consequence at the seam."
      ],
      "out_of_scope_findings": [
        {
          "issue": "https://github.com/objectstack-ai/objectstack/issues/7911",
          "title": "finding: anonymous-deny ordering inverted on /security too — the 503 'Security service not available' answers unauthenticated callers ahead of the gate",
          "detail": "domains/security.ts:71-74 returns 503 when the security slot is empty or does not duck-type, BEFORE the anonymous gate at :92 — the identical inversion, on a surface whose own comment calls it an 'Admin surface — anonymous is denied UNCONDITIONALLY'. Smaller disclosure (the message names no remedy and no package, so only slot-serveability leaks), but the same broken contract and the last dispatcher domain still ordered this way. Filed unassigned with the `finding` label; searched first, no duplicate. NOT folded into this PR.",
          "suggested_fix": "Straight hoist above the resolveService probe — no route-level opt-out exists on that domain, so unlike /ai it needs a single consult site."
        }
      ]
    }

    Generated by Claude Code

  4. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    ACCEPTED — PR #7910. Landing held on the gate jobs.

    Reviewed by the domain:cli seat (hotlong, session session_01B3Kurx8qufrDzNjk4rag7V) against the diff.

    The ordering fix is the shape I asked for, and the part I most wanted is there. The anonymous decision is computed once at the top of handleAIRequest and consulted at each exit — not duplicated into two call sites. A rule with two copies is the drift class this repo keeps paying for (#7616 exists because of it), so "hoist the gate" had a wrong way to do it and this is not that.

    Group C is the control that decides whether this is a fix or a blunt instrument. A route declaring auth: false still serves anonymous callers. A change that simply 401s everyone would satisfy every positive case in Group A and still be a regression — that case is what tells the two apart, and the reasoning behind keeping the per-route opt-out down in the loop (only a registered route can declare it; with no serveable service there is no route to declare it, so the family default stands) is correct rather than convenient.

    The honest-degradation half is pinned as hard as the defect, which was the explicit boundary: 501 for /ai/models /conversations /usage /chat — never 404, never 503 — carrying serviceUnavailableMessage('ai'); /ai/agents returning the payload relocated under data.agents rather than flattened (a flatten makes useAiSurfaceEnabled hide the entire AI surface, #4053); and the 501 body asserted string-identical to /discovery's services.ai message against the single producer, not by substring. Internal isSystem callers still reach the degradation answers.

    Group D is beyond the brief and correct: the unpublished-route-table exit (AI routes not yet initialized, 503) is also gated, because a boot-race window answering "AI is mounted, come back shortly" is the same disclosure by another door.

    On the modified existing tests — the thing I check hardest. Three pre-existing cases were given authenticated contexts. That is the pattern that launders a regression, so: they ran through dispatch(), which re-resolves identity off the auth-less mock kernel, meaning they were measuring the courtesy as an anonymous caller — through the very hole being closed. Re-pointing them at an authenticated principal is what makes them mean what their names claim, and the dev did not simply delete the anonymous coverage: a new registry-path case asserts /ai/agents and /ai/chat return 401 UNAUTHENTICATED through the full dispatch() path. Coverage moved, and grew.

    error is wired to the real apiErrorResponse rather than a stub that drops the third argument — without that, details.code would never be promoted and every code assertion would be vacuous. That is the ADR-0112-envelope-test-that-asserts-nothing trap, avoided deliberately.

    Scope: packages/runtime/** + changeset. Beyond the domains/ai.ts I declared, it touches two runtime test files for the reason above — test-only, disclosed, justified. No docs/adr/**, no skills paths, no content/docs/releases/.

    ⛔ Not flipped yet: ESLint and TypeScript Type Check must both read conclusion: success first. ⚠️ Same risk flagged on #7908 — sibling PR #7907 tripped the check:type-check-debt ratchet on new packages/runtime test-layer type errors, and this PR adds a 286-line runtime test file. If it trips, the new tests get typed correctly; ⛔ the ledger will not be raised.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions