feat: guard edge deploy against out-of-scope suggestions for subpath sites by dhavkuma · Pull Request #2595 · adobe/spacecat-api-service

dhavkuma · 2026-06-11T17:03:03Z

Summary

Adds isSuggestionInScope() inside deploySuggestionToEdge in src/controllers/suggestions.js
For subpath sites (e.g. nba.com/kings), suggestions whose URL or allowedRegexPatterns fall outside the site's registered base path are rejected with a 400 entry in the 207 response — preventing cross-tenant CDN deployments (e.g. a nba.com/kings site cannot accidentally deploy suggestions scoped to nba.com/wolves)
Root-level sites (pathname === '/') pass all suggestions through unchanged, so there is no regression for existing deployments

Logic

Suggestion type	Guard checks
Regular	`new URL(suggestion.url).pathname.startsWith(siteBasePath)`
Domain-wide / path	Every entry in `allowedRegexPatterns` starts with `siteBasePath`
Root-level site	Always passes through

Related PRs

Audit-worker subpath scoping: feat(prerender): generate subpath-scoped domain wide suggestion for subpath sites (LLMO-5145) spacecat-audit-worker#2605
UI subpath filter: adobe/project-elmo-ui (feat/subpath-deploy-guards-ui)

Test plan

4 new unit tests added to test/controllers/suggestions.test.js under subpath scope guard describe block
All existing deploySuggestionToEdge tests still pass
Full test suite passes (npm test)
Test: subpath site rejects regular suggestion with URL on sibling subpath → 207 with "outside the scope" message
Test: subpath site rejects domain-wide suggestion with wrong allowedRegexPatterns → 207 with "outside the scope" message
Test: subpath site accepts in-scope suggestion → passes scope guard, proceeds normally
Test: root-level site — all suggestions pass through unchanged

🤖 Generated with Claude Code

…sites Adds isSuggestionInScope() to deploySuggestionToEdge so that suggestions whose URL or allowedRegexPatterns fall outside the site's registered base path are rejected with a 400 in the 207 response, preventing cross-tenant CDN deployments (e.g. a nba.com/kings site cannot deploy wolves suggestions). Root-level sites (pathname '/') pass all suggestions through unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

codecov · 2026-06-11T17:11:38Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

github-actions · 2026-06-11T17:41:31Z

This PR will trigger a minor release when merged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

`startsWith("/kings")` incorrectly passes `/kingston/page`. Require the path to equal siteBasePath exactly or start with `siteBasePath + "/"` — applies to both the URL pathname check and the allowedRegexPatterns check. Adds a regression test for the prefix-collision edge case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dhavkuma · 2026-06-11T18:23:20Z

Fixed in ae2ee15.

Both startsWith(siteBasePath) checks now require a path-segment boundary — the path must either equal siteBasePath exactly or start with siteBasePath + "/". This prevents /kings from matching /kingston/page.

Added a regression test covering the prefix-collision case (/kings site, URL with /kingston/page path → rejected).

MysticatBot

Hey @dhavkuma,

Verdict: Request changes - two observability/security gaps in the scope guard warrant attention before merge.
Changes: Adds a subpath scope guard to deploySuggestionToEdge that rejects suggestions targeting URLs outside the site's registered base path, preventing cross-tenant CDN deployments (2 files).

Must fix before merge

[Important] Missing observability when site base URL is unparseable - scope guard silently disabled - src/controllers/suggestions.js:1800 (details inline)
[Important] allowedRegexPatterns string prefix check may be bypassable via regex metacharacters - src/controllers/suggestions.js:1814 (details inline)

Non-blocking (3): minor issues and suggestions

nit: isSuggestionInScope re-parses site.getBaseURL() on every suggestion in the batch; the siteBasePath is constant and could be hoisted above the forEach - src/controllers/suggestions.js:1795
suggestion: Add a test with mixed in-scope/out-of-scope allowedRegexPatterns (e.g. ['/kings/page', '/wolves/page']) to exercise the every() boundary condition - test/controllers/suggestions.test.js
suggestion: Add a test for isPathSuggestion type (not domain-wide) with out-of-scope patterns to cover both branches of the || - test/controllers/suggestions.test.js

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 14s | Cost: $2.70 | Commit: 1e7b0efc1dcb2cdf2fd1b544fc88ec6e9f2f161e
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

MysticatBot · 2026-06-16T06:25:49Z

+      let siteBasePath;
+      try {
+        siteBasePath = new URL(siteBaseURL).pathname;
+      } catch {


issue (blocking): This catch returns true without logging. If site.getBaseURL() returns malformed data, the scope guard is entirely disabled for that site with zero observability. This is a tenant isolation guard - silent degradation means operators cannot detect when it is ineffective for a given site.

Add a warning log before the pass-through:

} catch { context.log.warn(`[edge-deploy] site ${apexBaseUrl} has unparseable baseURL, skipping scope guard`); return true; }

MysticatBot · 2026-06-16T06:25:49Z

+          return true;
+        }
+        const scopePrefix = `${siteBasePath}/`;
+        return patterns.every((p) => p === siteBasePath || p.startsWith(scopePrefix));


issue (blocking): This checks p.startsWith(scopePrefix) treating allowedRegexPatterns values as literal path strings. If these are actually evaluated as regexes downstream (as the field name implies), a pattern like /kings/.*|/wolves/.* passes the prefix check (starts with /kings/) but when evaluated as a regex also matches /wolves/ paths - bypassing tenant isolation.

If patterns can contain regex metacharacters in practice, consider rejecting patterns containing scope-extending operators (|, (, lookahead syntax). If patterns are always simple path prefixes despite the field name, add a brief comment documenting that assumption so future maintainers know the string check is intentional.

…silent failures Addresses PR #2595 review feedback: - Log a warning instead of silently disabling the scope guard when a site's baseURL is unparseable, so operators can detect ineffective tenant isolation. - allowedRegexPatterns entries are compiled as regexes and matched against the full URL downstream (buildUrlMatcher), so a plain startsWith() check on the pattern text was bypassable via regex alternation (e.g. `/kings/.*|/wolves/.*`). Patterns are now only trusted as in-scope when they exactly match the site's base path or use the `/*` prefix-match convention; anything else is rejected (fail closed). - Hoist the per-batch siteBasePath resolution out of the per-suggestion closure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Suggestion URLs were scope-checked by pathname only, letting a suggestion for https://attacker.example/kings/page pass on a site scoped to https://example.com/kings. Route the URL-side check through isWithinSiteScope (host+port aware) and extract the pattern-side check into src/utils/path-scope.js.

…tils Bump @adobe/spacecat-shared-utils to 1.124.0, which now ships the hardened subpath scope guard, and drop the local src/utils/path-scope.js copy in favor of the shared implementation.

… feat/subpath-deploy-guards

npm install --package-lock-only had left the lockfile's root dependency entry for @adobe/spacecat-shared-http-utils on a caret range while package.json pins it exactly; reconcile after merging main.

MysticatBot

Hey @dhavkuma,

⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.

Verdict: Approve - the scope guard is well-implemented, both prior findings are addressed, and no blocking issues remain.
Complexity: HIGH - medium diff; API surface + dependency signal.
Changes: Adds a subpath scope guard to deploySuggestionToEdge that rejects suggestions targeting URLs outside the site's registered base path, preventing cross-tenant CDN deployments (4 files).

Non-blocking (4): minor issues and suggestions

nit: The inner try/catch for URL parsing could use URL.canParse(url) (Node 20+) to avoid the try/catch/eslint-disable dance - src/controllers/suggestions.js:1849
suggestion: Add one test where the URL comes from a getSuggestionUrl fallback field (e.g. data.recommendations[0].pageUrl) and is out of scope, to prove the full URL-resolution chain feeds into the guard - test/controllers/suggestions.test.js
suggestion: Tighten acceptance-test assertions to verify body.suggestions[0] is defined before asserting on .message, preventing false passes from undefined - test/controllers/suggestions.test.js
suggestion: Add a structured log field (the rejected URL or pattern) to the scope-guard warning at line 1876 to aid incident investigation - src/controllers/suggestions.js:1876

Previously flagged, now resolved

Unparseable site base URL now logs a warning before passing suggestions through
allowedRegexPatterns bypass via regex metacharacters now delegates to isPathPatternWithinSiteScope from shared-utils, which fails closed on any pattern containing metacharacters

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 32s | Cost: $7.42 | Commit: c586411338c239473ba5119ca6a0682144d8b811
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

Hoist siteBaseURL to where apexBaseUrl is first derived and reuse it for the batch-level siteBasePath resolution, instead of calling site.getBaseURL() twice.

MysticatBot

Hey @dhavkuma,

⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.

Verdict: Approve - the single new commit is a correct, trivial refactor with no behavioral change; no blocking issues remain.
Complexity: HIGH - medium diff; API surface + dependency signal.
Changes: Adds a subpath scope guard to deploySuggestionToEdge that rejects suggestions targeting URLs outside the site's registered base path, preventing cross-tenant CDN deployments (4 files).

Non-blocking (1): minor issues and suggestions

suggestion: Consider adding a structured log field (the rejected URL or pattern) to the scope-guard warning at line 1876 to aid incident triage - src/controllers/suggestions.js:1876

Previously flagged, now resolved

Duplicate site.getBaseURL() call now hoisted to a single siteBaseURL declaration reused by both apexBaseUrl and siteBasePath

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 5m 32s | Cost: $1.48 | Commit: b9d9ca1fb5e1f01ee978e9e8ca9f93b805ec8ff7
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

isSuggestionInScope now delegates entirely to isWithinSiteScope / isPathPatternWithinSiteScope, which handle root-level sites and fail closed on anything they cannot confirm to be in scope. Removes the dead siteBasePath pre-parse block and the redundant inline guards. Behavior changes (fail open -> fail closed): - unparseable site baseURL now rejects the batch instead of skipping the guard - a suggestion URL that resolves outside a subpath site's base path is rejected Preserved (fail open): a suggestion with no resolvable URL is let through (nothing to scope-check) so no-URL deploy + auto-cover keeps working on all sites, including root. Tests updated to assert the new fail-closed behavior; fixed the incomplete headingsOpportunity mock (missing getData) that previously only worked because root sites short-circuited before getSuggestionUrl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

MysticatBot

Hey @dhavkuma,

⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.

Verdict: Approve - the simplification commit is a correct, security-improving refactor with no blocking issues.
Complexity: HIGH - medium diff; API surface + dependency signal.
Changes: Simplifies the edge-deploy scope guard by removing local URL parsing and delegating entirely to shared-utils, changing behavior to fail-closed on unparseable inputs (2 files).

Non-blocking (2): minor issues and suggestions

suggestion: Add a structured log field (the rejected URL or pattern) to the scope-guard warning so operators can distinguish "out of scope" from "could not parse" during incident triage - src/controllers/suggestions.js:1849
suggestion: Consider a defensive try/catch around the isSuggestionInScope call site so that a future shared-utils release changing error semantics cannot abort the entire batch-processing loop - src/controllers/suggestions.js:1848

Previously flagged, now resolved

Local URL parsing and root-level site early-return logic removed; shared-utils now owns all scope decisions, eliminating duplicated parsing and strengthening the fail-closed posture

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 4s | Cost: $3.98 | Commit: 3bf7d62e89f6b7123a5f48e526b205bb3864b710
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

…uards # Conflicts: # package-lock.json

Regression coverage for edge-deploy's subpath scope guard: proves domain-wide, path-level, and regular suggestions still classify and deploy correctly together (and in isolation from each other) on a /kings-style subpath site, across both the synchronous deployToEdge flow and the respond-async geo-experiment flow, which share the same isSuggestionInScope classification. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

MysticatBot

Hey @dhavkuma,

⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.

Verdict: Approve - no blocking issues found; the new merge and test commits strengthen coverage without introducing concerns.
Complexity: HIGH - large diff; API surface + dependency signal.
Changes: Adds a merge from main and comprehensive tests for mixed domain-wide + path-level suggestion scoping on subpath sites (4 files).

Non-blocking (4): minor issues and suggestions

suggestion: The "rejects a path suggestion" test uses isDomainWide: false without setting pathType/pathPattern, so it may exercise the URL branch rather than the isPathSuggestion branch. Add pathType: 'prefix' to confirm it tests the intended path - test/controllers/suggestions.test.js:240
suggestion: Add a test for an explicitly empty allowedRegexPatterns: [] on a subpath site to document the pass-through behavior - test/controllers/suggestions.test.js
suggestion: Assert statusCode on each individual suggestion in the sync regression "deploys all 3 together" test, matching the async test's assertion pattern - test/controllers/suggestions.test.js:632
suggestion: Assert tokowakaClientStub.deployToEdge.called is false in the async experiment flow test to guard against fallthrough to sync deploy - test/controllers/suggestions.test.js:456

Previously flagged, now resolved

Duplicate site.getBaseURL() call hoisted to single siteBaseURL declaration

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 28s | Cost: $3.75 | Commit: e392aa251b903ed92d5e0f23feeaec1def504881
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

# [1.621.0](v1.620.0...v1.621.0) (2026-07-03) ### Features * guard edge deploy against out-of-scope suggestions for subpath sites ([#2595](#2595)) ([f084db1](f084db1))

solaris007 · 2026-07-03T07:55:01Z

🎉 This PR is included in version 1.621.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

dhavkuma marked this pull request as draft June 11, 2026 17:04

dhavkuma temporarily deployed to dev-branches June 11, 2026 17:16 — with GitHub Actions Inactive

dhavkuma requested a review from MysticatBot June 11, 2026 17:37

dhavkuma changed the base branch from main to fix/subpath-staging-domain-validation June 11, 2026 17:37

dhavkuma changed the base branch from fix/subpath-staging-domain-validation to main June 11, 2026 17:38

Merge branch 'main' into feat/subpath-deploy-guards

a9013ff

dhavkuma marked this pull request as ready for review June 11, 2026 17:41

dhavkuma temporarily deployed to dev-branches June 11, 2026 17:54 — with GitHub Actions Inactive

dhavkuma and others added 2 commits June 11, 2026 23:41

test: cover isSuggestionInScope fallback paths for edge deploy guard

6dc0a2c

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dhavkuma temporarily deployed to dev-branches June 11, 2026 18:36 — with GitHub Actions Inactive

Merge branch 'main' into feat/subpath-deploy-guards

1e7b0ef

dhavkuma requested review from MysticatBot and removed request for MysticatBot June 16, 2026 06:16

MysticatBot requested changes Jun 16, 2026

View reviewed changes

MysticatBot added the ai-reviewed Reviewed by AI label Jun 16, 2026

Merge branch 'main' into feat/subpath-deploy-guards

607af71

dhavkuma temporarily deployed to dev-branches July 1, 2026 13:49 — with GitHub Actions Inactive

dhavkuma and others added 3 commits July 1, 2026 19:40

Merge branch 'main' into feat/subpath-deploy-guards

0951cea

dhavkuma temporarily deployed to dev-branches July 1, 2026 18:23 — with GitHub Actions Inactive

dhavkuma added 2 commits July 2, 2026 01:07

refactor: consume isPathPatternWithinSiteScope from spacecat-shared-u…

183b46a

…tils Bump @adobe/spacecat-shared-utils to 1.124.0, which now ships the hardened subpath scope guard, and drop the local src/utils/path-scope.js copy in favor of the shared implementation.

Merge remote-tracking branch 'origin/feat/subpath-deploy-guards' into…

c586411

… feat/subpath-deploy-guards

dhavkuma requested a review from MysticatBot July 1, 2026 19:43

chore: sync package-lock.json http-utils version pin

313d111

npm install --package-lock-only had left the lockfile's root dependency entry for @adobe/spacecat-shared-http-utils on a caret range while package.json pins it exactly; reconcile after merging main.

MysticatBot approved these changes Jul 1, 2026

View reviewed changes

MysticatBot added the complexity:high AI-assessed PR complexity: HIGH label Jul 1, 2026

dhavkuma temporarily deployed to dev-branches July 1, 2026 20:07 — with GitHub Actions Inactive

refactor: dedupe site.getBaseURL() call in deploySuggestionToEdge

b9d9ca1

Hoist siteBaseURL to where apexBaseUrl is first derived and reuse it for the batch-level siteBasePath resolution, instead of calling site.getBaseURL() twice.

dhavkuma requested a review from MysticatBot July 2, 2026 05:10

MysticatBot approved these changes Jul 2, 2026

View reviewed changes

dhavkuma temporarily deployed to dev-branches July 2, 2026 05:21 — with GitHub Actions Inactive

dhavkuma temporarily deployed to dev-branches July 2, 2026 11:37 — with GitHub Actions Inactive

dhavkuma requested a review from MysticatBot July 2, 2026 11:46

MysticatBot approved these changes Jul 2, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into feat/subpath-deploy-g…

fc70f06

…uards # Conflicts: # package-lock.json

dhavkuma temporarily deployed to dev-branches July 3, 2026 06:28 — with GitHub Actions Inactive

dhavkuma requested a review from MysticatBot July 3, 2026 07:23

Merge branch 'main' into feat/subpath-deploy-guards

dac37ff

MysticatBot approved these changes Jul 3, 2026

View reviewed changes

dhavkuma temporarily deployed to dev-branches July 3, 2026 07:37 — with GitHub Actions Inactive

dhavkuma merged commit f084db1 into main Jul 3, 2026
20 checks passed

dhavkuma deleted the feat/subpath-deploy-guards branch July 3, 2026 07:40

solaris007 pushed a commit that referenced this pull request Jul 3, 2026

chore(release): 1.621.0 [skip ci]

43ccb99

# [1.621.0](v1.620.0...v1.621.0) (2026-07-03) ### Features * guard edge deploy against out-of-scope suggestions for subpath sites ([#2595](#2595)) ([f084db1](f084db1))

solaris007 added the released label Jul 3, 2026

dhavkuma mentioned this pull request Jul 3, 2026

chore(deps): bump spacecat-shared-utils to 1.124.1 (fail-closed site scope) #2746

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: guard edge deploy against out-of-scope suggestions for subpath sites#2595

feat: guard edge deploy against out-of-scope suggestions for subpath sites#2595
dhavkuma merged 17 commits into
mainfrom
feat/subpath-deploy-guards

dhavkuma commented Jun 11, 2026

Uh oh!

codecov Bot commented Jun 11, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 11, 2026

Uh oh!

dhavkuma commented Jun 11, 2026

Uh oh!

MysticatBot left a comment •

edited

Loading

Uh oh!

MysticatBot Jun 16, 2026

Uh oh!

MysticatBot Jun 16, 2026

Uh oh!

MysticatBot left a comment •

edited

Loading

Uh oh!

MysticatBot left a comment •

edited

Loading

Uh oh!

MysticatBot left a comment •

edited

Loading

Uh oh!

MysticatBot left a comment •

edited

Loading

Uh oh!

Uh oh!

solaris007 commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dhavkuma commented Jun 11, 2026

Summary

Logic

Related PRs

Test plan

Uh oh!

codecov Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Jun 11, 2026

Uh oh!

dhavkuma commented Jun 11, 2026

Uh oh!

MysticatBot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Must fix before merge

Uh oh!

MysticatBot Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

MysticatBot Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

MysticatBot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Previously flagged, now resolved

Uh oh!

MysticatBot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Previously flagged, now resolved

Uh oh!

MysticatBot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Previously flagged, now resolved

Uh oh!

MysticatBot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Previously flagged, now resolved

Uh oh!

Uh oh!

solaris007 commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jun 11, 2026 •

edited

Loading

MysticatBot left a comment •

edited

Loading

MysticatBot left a comment •

edited

Loading

MysticatBot left a comment •

edited

Loading

MysticatBot left a comment •

edited

Loading

MysticatBot left a comment •

edited

Loading