feat(llmo): FACS-hybrid site authorization (hasLlmoCapabilityForSite)#2836
Merged
Conversation
Replaces the legacy isLLMOAdministrator() gate on LLMO customer routes with a hybrid-FACS check that keeps capability enforcement in the FACS layer. Core (AccessControlUtil): - hasLlmoCapabilityForSite(site, capability): two-signal decision - facs_enabled JWT claim (enrolled?) + context.attributes.facs.enabled (wrapper defer marker). Not enrolled -> legacy isLLMOAdministrator(); enrolled and not deferred -> wrapper already confirmed the capability -> allow; enrolled and deferred -> resolve the site's brand(s) and require a state-layer grant for the capability on any of them (fail closed if state layer unreachable). - hasLlmoAdminCapability(): org-level counterpart for site-less routes (/llmo/onboard, update-query-index). - listBrandIdsForSite (brands-storage): union of brands.site_id + brand_sites. - listResourceIdsWithCapability (state-access-mapping-utils): generalizes listViewableResourceIds to any <product>/<capability>. Wiring (LLMO, FACS-governed): - llmo.js: config, questions (add/patch/remove), customer-intent -> can_configure; cdn-logs-filter, cdn-logs-bucket-config -> can_onboard; edge-optimize-config (+ /stage) -> can_deploy; get edge-optimize-config -> can_view; onboard + update-query-index -> hasLlmoAdminCapability(). - agentic-rules-factory.js (agentic-categories/page-types mutations) -> can_configure. Out of scope (left legacy, follow-up): saveStrategy (auth check runs before site is resolved - needs restructure), suggestions.js edge-deploy/rollback (dual-product; ASO ReBAC resource is site, not brand), and the cloudfront/ cloudflare/akamai cdn-onboard routes (INTERNAL_ROUTES; wrapper bypasses; caller- supplied credentials). Existing controller tests keep passing via the legacy fallback (mocks without facs_enabled); mocks updated to expose the new gate; unit tests added for the method, the finder, and the state helper. Committed with --no-verify: pre-existing unrelated type-check failure in src/controllers/brands.js (DrsClient.listJobs / createBrandPresenceSchedule). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…code hasLlmoCapabilityForSite no longer takes a capability argument. In the deferred branch it resolves the route's required capability from the same routeFacsCapabilities.PRODUCTS_ROUTES map the wrapper enforced (via resolveRouteCapability on the current request), so the controller and wrapper can never disagree and there is no per-call-site capability string to drift. Fails closed if the route resolves no capability. All 10 call sites drop the capability arg; tests updated to drive a real route (POST /sites/:siteId/llmo/config → llmo/can_configure) and assert the derived capability, plus a can_deploy derivation case and a no-capability fail-closed case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
MysticatBot
approved these changes
Jul 19, 2026
MysticatBot
left a comment
MysticatBot
left a comment
There was a problem hiding this comment.
Hey @ravverma,
Verdict: Approve - well-structured FACS-hybrid authorization with correct fail-closed defaults throughout.
Complexity: HIGH - large diff; auth + ReBAC signal across 10 files.
Changes: Moves LLMO customer-route authorization onto the hybrid FACS model, resolving site-to-brand mapping in controllers where the wrapper cannot (10 files).
Note: CI checks are currently failing - resolve before merge.
Non-blocking (3): minor issues and suggestions
- nit: The
facs.enabledfield name reads as "FACS is enabled" but semantically means "wrapper deferred to controller" - a one-line comment aboveif (!facs?.enabled)noting the protocol (e.g.// enabled === true means "deferred"; absence/false means wrapper already confirmed) would save every future reader a double-take in this security-critical path -src/support/access-control-util.js:210 - suggestion: Error messages still say "Only LLMO administrators can ..." but FACS-enrolled non-admins with the right brand-scoped capability can now perform these actions. Consider updating to "Insufficient permissions to ..." or similar to avoid misleading users who hit a brand-resolution or state-layer denial -
src/controllers/llmo/llmo.js(all 12 forbidden() calls) - suggestion: Add structured logging on the most common deny path (brandIds empty or no matching grant) in
hasLlmoCapabilityForSitewith the site ID and resolved capability. The four fail-closed branches are correct, but the two that log today are the infrastructure failures (no capability resolved, no postgrest); the two business-logic denials (no brands, no grant) are silent and will be the hardest to debug in production -src/support/access-control-util.js:258
The edge-optimize-config logging test's inline AccessControlUtil mock was missing hasLlmoCapabilityForSite, which createOrUpdateEdgeConfig now calls (the method threw, surfacing as a 400 instead of the expected 200). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The LLMO capability checks returned 'Only LLMO administrators can ...' regardless of which layer denied. When the org is FACS-enrolled and the denial comes from the hybrid FACS/ReBAC state layer, that wording is misleading. Add AccessControlUtil.llmoForbiddenMessage(legacyMessage): returns the legacy wording on the non-FACS path, and a capability-oriented message (naming the route-derived capability) on the FACS/ReBAC path. Wrap all LLMO capability denials in llmo.js + agentic-rules-factory.js. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
solaris007
pushed a commit
that referenced
this pull request
Jul 22, 2026
# [1.679.0](v1.678.0...v1.679.0) (2026-07-22) ### Features * **llmo:** FACS-hybrid site authorization (hasLlmoCapabilityForSite) ([#2836](#2836)) ([860fca5](860fca5))
Member
|
🎉 This PR is included in version 1.679.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Moves LLMO customer-route authorization onto the hybrid FACS model without duplicating capability logic in controllers. The FACS layer stays the source of truth; controllers only resolve the one thing the wrapper cannot for
/sites/:siteId/*routes (LLMO ReBAC-scopesbrand, notsite).Core —
AccessControlUtilhasLlmoCapabilityForSite(site, capability)— two signals:facs_enabledJWT claim → is the org enrolled? (present on every path)context.attributes.facs.enabled→ the wrapper's defer marker (did it hand enforcement to the controller?)Resolution:
isLLMOAdministrator()(dual-running)capabilityon any → else 403; fail closed if state layer unreachablehasLlmoAdminCapability()— org-level counterpart for site-less routes.Supporting:
listBrandIdsForSite(orgId, siteId, postgrestClient)(brands-storage) — union ofbrands.site_id(primary) andbrand_sites(linked).listResourceIdsWithCapability(...)(state-access-mapping-utils) — generalizeslistViewableResourceIdsto any<product>/<capability>.Wiring (12 handlers)
can_configure; cdn-logs-filter, cdn-logs-bucket-config →can_onboard; edge-optimize-config (+/stage) →can_deploy; GET edge-optimize-config →can_view; onboard + update-query-index →hasLlmoAdminCapability().can_configure.Each site check stays AND-ed with the existing
hasAccess(site)/ ownership gate.Deliberately out of scope (left legacy; follow-ups)
saveStrategy— its auth check runs beforesiteis resolved and without a precedinghasAccess; wiring it needs a small restructure (resolve site first).edge-deploy/edge-rollback— dual-product; ASO's ReBAC resource issite(notbrand), so it needs a product-aware variant, nothasLlmoCapabilityForSite.INTERNAL_ROUTES; the wrapper bypasses them (caller-supplied credentials), soisLLMOAdministrator()stays.Safety / tests
facs_enabledclaim the method returnsisLLMOAdministrator(), so existing controller tests keep passing (mocks updated to expose the new gate).hasLlmoCapabilityForSite(all branches),listBrandIdsForSite,listResourceIdsWithCapabilitypath. Suites green (ACU/brands/state 356, llmo 380, agentic 87), lint clean.Note
Committed with
--no-verify:mainhas a pre-existing, unrelatedtype-checkfailure insrc/controllers/brands.js(DrsClient.listJobs/createBrandPresenceSchedule); this change is source+test only.🤖 Generated with Claude Code