feat(facs): enforce site-scoped LLMO routes in facsWrapper via secondary resource param - #3030
Open
ravverma wants to merge 9 commits into
Open
feat(facs): enforce site-scoped LLMO routes in facsWrapper via secondary resource param#3030ravverma wants to merge 9 commits into
ravverma wants to merge 9 commits into
Conversation
…ary resource param
Adds the LLMO secondary FACS resource (site -> brands) config + resolver and wires it into the facsWrapper chain, so site-scoped LLMO routes are authorized in the wrapper instead of per-controller. facs-capabilities.js: PRODUCTS_FACS_SECONDARY_RESOURCE.LLMO = { site, [siteId], llmoSiteToBrands }. support/facs-secondary-resolvers.js: hasCapabilityOnSiteBrands (org-scoped listBrandIdsForSite + single listResourceIdsWithCapability, parity with hasLlmoCapabilityForSite) + secondaryResolvers registry. index.js: pass secondaryResolvers to facsWrapper. edge-routing-auth.js: surgical revert of PR #2947's FACS-deferred branch (superseded by the wrapper); keeps paid-legacy / trial / unknown-tier paths.
Requires @adobe/spacecat-shared-http-utils with secondary-resolver support (spacecat-shared#1870); dependency bump + activation lands once that releases. Design: mysticat-architecture/platform/decisions/facs-wrapper-secondary-resource-param.md (#241).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…FACS paid orgs The surgical #2947 revert must not send FACS-enrolled paid orgs through the legacy IMS product-context check — they are authorized upstream by facsWrapper (secondary resource param). Gate the check on the JWT facs_enabled claim (not context.attributes.facs, which is only set when the wrapper DEFERS — no longer the case for site routes). Mirrors AccessControlUtil.hasLlmoCapabilityForSite's own gating. Adds a unit test for the FACS-enabled paid path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…can_configure Moves the CloudFront/Cloudflare/Akamai cdn-onboard routes from INTERNAL_ROUTES to PRODUCTS_ROUTES.LLMO (llmo/can_configure), making them customer-reachable under the ReBAC model — enforced by facsWrapper's secondary resource param (site -> brands). Swaps the controller gates from isLLMOAdministrator() to hasLlmoCapabilityForSite(site) (FACS capability, isLLMOAdministrator fallback for non-FACS orgs) via llmoForbiddenMessage(). Org routes (:spaceCatId onboard-site, semrush-onboarding) intentionally left in INTERNAL_ROUTES pending a team decision. required-capabilities.js (separate S2S layer) unchanged except two stale comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ravverma
marked this pull request as ready for review
August 12, 2026 05:50
Replaces the temporary gist tarball stopgap with the released 1.35.0 (secondary FACS resource param + resolver registry from spacecat-shared#1870). Activates the secondaryResolvers wiring end-to-end. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ravverma
requested review from
MysticatBot
and removed request for
MysticatBot
August 12, 2026 10:03
MysticatBot
approved these changes
Aug 12, 2026
MysticatBot
left a comment
MysticatBot
left a comment
There was a problem hiding this comment.
Hey @ravverma,
Verdict: Approve - well-structured authorization migration with correct fail-closed semantics throughout.
Complexity: HIGH - medium diff touching 3 signal domains (ReBAC/auth surface, API route classification, dependency bump).
Changes: Moves 28 LLMO CDN onboarding routes from INTERNAL_ROUTES to FACS-governed PRODUCTS_ROUTES with a new secondary resource resolver (site-to-brands cross-resource check), superseding the PR 2947 deferred-branch pattern (16 files).
Note: CI checks are currently failing (the pr-review check itself).
Non-blocking (3): minor issues and suggestions
- suggestion: Add warn-level logging on fail-closed paths in
hasCapabilityOnSiteBrands(missing site, missing org, empty brands) for production debugging -src/support/facs-secondary-resolvers.js:50-64. The resolver is silent on denial today;context.logis available. - suggestion: Add a startup assertion in
src/index.jsvalidating that everyresolverstring inPRODUCTS_FACS_SECONDARY_RESOURCEhas a matching key insecondaryResolvers- prevents silent misconfiguration after a rename. - nit: The CLAUDE.md middleware stack list (lines 90-104) does not include
facsWrapper, which is now the innermost enforcement wrapper withsecondaryResolversconfig -CLAUDE.md:90
|
Mysticat review failed: Claude CLI crashed (exit 1): stderr= stdout={"type":"result","subtype":"error_max_budget_usd","duration_ms":10,"duration_api_ms":1509888,"is_error":true,"num_turns":1,"stop_reason":null,"session_id":"52757ae7-8a98-4960-918b-8f2c9b5880ef","total_cost_usd":10.252599349999999,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_read_ |
…sWrapper Addresses non-blocking mysticatBot review feedback on #3030: - (#1) hasCapabilityOnSiteBrands was silent on denial; each fail-closed branch now logs (tag `facs-secondary`, distinct `reason`) for production debugging — `warn` for anomalies (no-postgrest, no-ims-org), `info` for ordinary data-shape denials (site-not-found, no-brands) to stay low-noise. The normal "evaluated, not authorized" outcome is intentionally not logged. - (#3) add `facsWrapper` (innermost enforcement wrapper) to the CLAUDE.md middleware-stack list. - Tests assert each fail-closed branch logs its reason. Skipped (#2) resolver-key startup assertion: already covered — the wrapper throws at construction on an unregistered resolver (facs-wrapper.js), caught at Lambda cold start and by the CI bundle smoke check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 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
Enforces site-scoped LLMO routes in
facsWrappervia the new secondary FACS resource param(site → brands), moving the check out of controllers so new LLMO site endpoints are covered
automatically.
routes/facs-capabilities.js:PRODUCTS_FACS_SECONDARY_RESOURCE.LLMO = { resourceType: 'site', aliases: ['siteId'], resolver: 'llmoSiteToBrands' }.support/facs-secondary-resolvers.js(new):hasCapabilityOnSiteBrands— org-scopedlistBrandIdsForSite+ a singlelistResourceIdsWithCapabilityquery (parity withAccessControlUtil.hasLlmoCapabilityForSite) + thesecondaryResolversregistry.index.js: passessecondaryResolverstofacsWrapper.support/edge-routing-auth.js: surgical revert of PR feat(llmo): FACS-capability auth for edge CDN routing; edge-optimize-config under can_configure #2947's FACS-deferred branch (superseded by the wrapper); keeps the paid-legacy / trial / unknown-tier paths.Why
Per
mac-state-layer.mdDecision 5, LLMO's ReBAC resource isbrand; site routes carrysiteIdandtoday defer to controllers (e.g.
hasLlmoCapabilityForSite), which does not scale. Design ADR:mysticat-architecture/platform/decisions/facs-wrapper-secondary-resource-param.md(adobe/mysticat-architecture#241, MysticatBot-approved).
Before this is ready for review
@adobe/spacecat-shared-http-utilsto that version (activates the wiring)Tests
New
test/support/facs-secondary-resolvers.test.js; updatedtest/routes/facs-capabilities.test.jsand
test/support/edge-routing-auth.test.js(dropped the superseded #2947 cases). Fullnpm testgreen;
type-check(base+strict) andlintclean.🤖 Generated with Claude Code