Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@
"@adobe/spacecat-shared-http-utils": "1.31.0",
"@adobe/spacecat-shared-ims-client": "1.12.7",
"@adobe/spacecat-shared-launchdarkly-client": "1.3.0",
"@adobe/spacecat-shared-project-engine-client": "1.2.0",
"@adobe/spacecat-shared-project-engine-client": "1.3.0",
"@adobe/spacecat-shared-rum-api-client": "2.44.0",
"@adobe/spacecat-shared-scrape-client": "2.6.3",
"@adobe/spacecat-shared-slack-client": "1.6.7",
"@adobe/spacecat-shared-tier-client": "1.5.1",
"@adobe/spacecat-shared-tokowaka-client": "1.19.0",
"@adobe/spacecat-shared-user-manager-client": "1.1.0",
"@adobe/spacecat-shared-user-manager-client": "1.3.0",
"@adobe/spacecat-shared-utils": "1.123.0",
"@adobe/spacecat-shared-vault-secrets": "1.3.5",
"@aws-sdk/client-s3": "3.1045.0",
Expand Down
23 changes: 22 additions & 1 deletion src/controllers/serenity.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,31 @@ function mapError(e, log) {
* missing OR if the caller authenticated by some other mechanism. The
* upstream gateway only understands IMS user tokens; we refuse to forward
* anything else.
*
* SECURITY MODEL — this proxy is NOT the auth boundary; Semrush is. The bearer
* we forward is validated AGAIN by the real Semrush gateway on every upstream
* call (it rejects an invalid/expired/forged token with 401/403, which the
* transport surfaces as a SerenityTransportError). This local check is only a
* fail-fast + shape guard so we do not forward a token Semrush will obviously
* reject; it never substitutes for the upstream's own validation.
*
* Test-only escape hatch: when `SERENITY_ALLOW_NON_IMS_AUTH === 'true'` the
* IMS-type check is skipped so an authenticated NON-IMS caller (e.g. the
* locally-signed JWT the integration-test harness mints) can reach the
* handlers. This is sound because (a) production auth is unaffected — Semrush
* still validates the forwarded token end to end — and (b) the integration
* tests run against the Semrush vendor MOCKS, which intentionally do not
* validate the bearer, so the token's value never matters there, only that an
* authenticated identity is present. Mirrors `SERENITY_ALLOW_WORKSPACE_DELETE`
* in rest-transport.js: an explicit opt-in flag that NO deployed environment
* sets (it is never written to Vault `dx_mysticat/<env>/api-service`); it is
* for local + automated E2E only. The Authorization-header requirement still
* holds — a bearer must be present to forward upstream.
*/
function requireImsBearer(ctx) {
const authInfo = ctx?.attributes?.authInfo;
if (authInfo?.getType && authInfo.getType() !== 'ims') {
const allowNonIms = ctx?.env?.SERENITY_ALLOW_NON_IMS_AUTH === 'true';
if (!allowNonIms && authInfo?.getType && authInfo.getType() !== 'ims') {
throw new ErrorWithStatusCode(
'Serenity proxy requires IMS authentication',
401,
Expand Down
90 changes: 68 additions & 22 deletions src/support/serenity/rest-transport.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,53 +67,91 @@ export function redactUpstreamMessage(e) {
}

/**
* Resolves and validates the upstream base URL. The URL is required and must
* arrive via `env.SEMRUSH_PROJECTS_BASE_URL` — sourced from Vault
* (`dx_mysticat/<env>/api-service`) and injected through AWS Secrets Manager.
* No source default: the upstream host is operational config that must be
* settable per-environment without a code change.
* Validates a raw base-URL value and returns its canonical `protocol//host`
* origin — never the raw value. A misconfigured value like
* `https://host/path-prefix` or `https://user:pass@host` would otherwise
* silently bleed path/userinfo into every outbound request (and the userinfo
* form would leak credentials in each fetch's `Authorization`-adjacent
* metadata). Always returning the parsed origin closes both classes of
* injection. The https requirement is kept for both resolvers.
*
* Returns the canonical `protocol//host` origin — never the raw value. A
* misconfigured value like `https://host/path-prefix` or
* `https://user:pass@host` would otherwise silently bleed path/userinfo into
* every outbound request (and the userinfo form would leak credentials in
* each fetch's `Authorization`-adjacent metadata). Always returning the
* parsed origin closes both classes of injection.
* `varName` names the env var in every error message so a misconfiguration
* points the operator at the exact key to fix.
*
* Failure mapping (controller `mapError`):
* - Missing/invalid/non-https → throws ErrorWithStatusCode(503,
* 'configurationError'): operational failure, not a runtime bug.
*
* @param {string | undefined} raw - The raw env value.
* @param {string} varName - The env var name, used in error messages.
* @returns {string} The canonical `protocol//host` origin.
*/
function baseUrl(env) {
const raw = typeof env?.SEMRUSH_PROJECTS_BASE_URL === 'string'
? env.SEMRUSH_PROJECTS_BASE_URL.trim()
: env?.SEMRUSH_PROJECTS_BASE_URL;
if (!hasText(raw)) {
function normalizeBaseUrl(raw, varName) {
// Default a non-string (incl. undefined) to '' so the value is always a
// string for hasText/replace below — hasText is not a TS type guard, so the
// narrowing has to be structural (see this dir's CLAUDE.md).
const trimmed = typeof raw === 'string' ? raw.trim() : '';
if (!hasText(trimmed)) {
throw new ErrorWithStatusCode(
'SEMRUSH_PROJECTS_BASE_URL is not set. Configure it via Vault '
`${varName} is not set. Configure it via Vault `
+ '(dx_mysticat/<env>/api-service) or .env for local dev.',
503,
);
}
const candidate = raw.replace(/\/$/, '');
const candidate = trimmed.replace(/\/$/, '');
let parsed;
try {
parsed = new URL(candidate);
} catch {
throw new ErrorWithStatusCode(
`SEMRUSH_PROJECTS_BASE_URL is not a valid URL: ${candidate}`,
`${varName} is not a valid URL: ${candidate}`,
503,
);
}
if (parsed.protocol !== 'https:') {
throw new ErrorWithStatusCode(
`SEMRUSH_PROJECTS_BASE_URL must use https (got ${parsed.protocol})`,
`${varName} must use https (got ${parsed.protocol})`,
503,
);
}
return `${parsed.protocol}//${parsed.host}`;
}

/**
* Resolves the Project Engine gateway origin. Required; arrives via
* `env.SEMRUSH_PROJECTS_BASE_URL` — sourced from Vault
* (`dx_mysticat/<env>/api-service`) and injected through AWS Secrets Manager.
* No source default: the upstream host is operational config that must be
* settable per-environment without a code change.
*
* @param {object} env
* @returns {string} canonical `protocol//host` origin
*/
function baseUrl(env) {
return normalizeBaseUrl(env?.SEMRUSH_PROJECTS_BASE_URL, 'SEMRUSH_PROJECTS_BASE_URL');
}

/**
* Resolves the User Manager gateway origin. Reads `env.SEMRUSH_USERS_BASE_URL`,
* **falling back to `SEMRUSH_PROJECTS_BASE_URL` when unset** — Project Engine
* and User Manager are distinct Semrush services that share a single host in
* every deployed environment today, so the fallback keeps prod/stage/dev Vault
* config working untouched. Decoupling lets local + E2E setups point the User
* Manager calls at a SEPARATE (mock) host without a path-routing reverse proxy
* (LLMO / api-service#2656). The error message names whichever var was the
* effective source so a misconfiguration is unambiguous.
*
* @param {object} env
* @returns {string} canonical `protocol//host` origin
*/
function usersBaseUrl(env) {
const explicit = hasText(env?.SEMRUSH_USERS_BASE_URL);
return normalizeBaseUrl(
explicit ? env.SEMRUSH_USERS_BASE_URL : env?.SEMRUSH_PROJECTS_BASE_URL,
explicit ? 'SEMRUSH_USERS_BASE_URL' : 'SEMRUSH_PROJECTS_BASE_URL',
);
}

/**
* Wraps global fetch with the transport's 15s ceiling and the
* `Accept: application/json` header sent on every call — neither of which the
Expand Down Expand Up @@ -183,11 +221,17 @@ function unwrap(method, result) {
* separate user-manager gateway.
*
* @param {object} args
* @param {object} args.env - Environment (reads SEMRUSH_PROJECTS_BASE_URL override).
* @param {object} args.env - Environment (reads SEMRUSH_PROJECTS_BASE_URL and,
* for the User Manager gateway, SEMRUSH_USERS_BASE_URL — falling back to the
* projects host when the latter is unset).
* @param {string} args.imsToken - IMS user bearer token (without 'Bearer ' prefix).
*/
export function createSerenityTransport({ env, imsToken }) {
const root = baseUrl(env);
// User Manager has its OWN origin (SEMRUSH_USERS_BASE_URL), falling back to
// `root` when unset — see usersBaseUrl(). Lets the sub-workspace lifecycle
// calls hit a separate (mock) host independently of Project Engine.
const usersRoot = usersBaseUrl(env);

// Fail-closed guard for the destructive workspace delete. Deleting a
// sub-workspace must be IMPOSSIBLE in every deployed environment
Expand Down Expand Up @@ -225,8 +269,10 @@ export function createSerenityTransport({ env, imsToken }) {

// Typed User Manager client over the sub-workspace lifecycle gateway. Same
// shape as the project client; appends its own '/enterprise/users/api' prefix.
// Uses `usersRoot` (SEMRUSH_USERS_BASE_URL, or `root` by fallback) so the
// lifecycle gateway can be a separate host from Project Engine.
const users = createSerenityUserManagerApiClient({
baseUrl: root,
baseUrl: usersRoot,
authToken,
maxRetries: 0,
fetch: createTimeoutFetch(DEFAULT_TIMEOUT_MS),
Expand Down
22 changes: 22 additions & 0 deletions test/controllers/serenity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,28 @@ describe('SerenityController', () => {
expect(response.status).to.equal(401);
});

// Test-only escape hatch (SERENITY_ALLOW_NON_IMS_AUTH). The integration-test
// harness mints a non-IMS (JWT) token; with the flag set, the IMS-type gate
// is skipped so the handler runs (the Semrush mock ignores the forwarded
// bearer). The Authorization header is still required (asserted below).
it('lets a non-IMS caller through when SERENITY_ALLOW_NON_IMS_AUTH is set (reaches the handler, not 401)', async () => {
handlers.handleListPrompts.resolves({ items: [], total: 0 });
const controller = SerenityController({ env: {} }, fakeLog(), {});
const ctx = fakeContext({ authType: 'jwt', env: { SERENITY_ALLOW_NON_IMS_AUTH: 'true' } });
const response = await controller.listPrompts(ctx);
expect(response.status).to.equal(200);
expect(handlers.handleListPrompts).to.have.been.calledOnce;
});

it('still 401s a non-IMS caller with the flag set but NO Authorization header', async () => {
const controller = SerenityController({ env: {} }, fakeLog(), {});
const ctx = fakeContext({
authType: 'jwt', bearer: null, env: { SERENITY_ALLOW_NON_IMS_AUTH: 'true' },
});
const response = await controller.listPrompts(ctx);
expect(response.status).to.equal(401);
});

it('400s when :brandId is not a UUID (the new guard)', async () => {
const controller = SerenityController({ env: {} }, fakeLog(), {});
const ctx = fakeContext({ brandId: 'adobe-brand-name' });
Expand Down
18 changes: 18 additions & 0 deletions test/it/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,23 @@ export function buildEnv(publicKeyB64) {
POSTGREST_URL: `http://localhost:${process.env.IT_POSTGREST_PORT || '3300'}`,
POSTGREST_SCHEMA: 'public',
POSTGREST_API_KEY: POSTGREST_WRITER_JWT,

// ── Serenity E2E: Semrush vendor mocks ──────────────────────────────────
// NONE of the vars below require Vault / deployed-env config: SEMRUSH_USERS_BASE_URL
// falls back to SEMRUSH_PROJECTS_BASE_URL when unset, and the rest are IT-only.
// Point the two serenity transport gateways at the mock containers. The
// User Manager origin is split out via SEMRUSH_USERS_BASE_URL (api-service#2656)
// so the two mocks need no path-routing reverse proxy. Both serve self-signed
// HTTPS, so the dev server trusts them via NODE_TLS_REJECT_UNAUTHORIZED=0
// (scoped to this IT process — never a deployed setting).
SEMRUSH_PROJECTS_BASE_URL: `https://localhost:${process.env.IT_PE_MOCK_PORT || '8443'}`,
SEMRUSH_USERS_BASE_URL: `https://localhost:${process.env.IT_UM_MOCK_PORT || '8444'}`,
NODE_TLS_REJECT_UNAUTHORIZED: '0',
// Test-only escape hatch: accept the harness's non-IMS JWT on /serenity/*.
// Sound only against the mocks, which ignore the forwarded bearer. No
// deployed environment sets this (it is never written to Vault).
SERENITY_ALLOW_NON_IMS_AUTH: 'true',
// Lets the net-zero cleanup delete a sub-workspace it created in the mock.
SERENITY_ALLOW_WORKSPACE_DELETE: 'true',
};
}
48 changes: 48 additions & 0 deletions test/it/postgres/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,51 @@ services:
ports:
- "${IT_POSTGREST_PORT:-3300}:3000"
- "${IT_POSTGREST_ADMIN_PORT:-3301}:3001"

# ── Semrush vendor mocks (serenity E2E) ──────────────────────────────────
# Stateful Counterfact mocks of the Semrush Project Engine and User Manager
# gateways, published public to GHCR from adobe/spacecat-shared. Each serves
# ONLY its own path prefix over self-signed HTTPS on internal :8443 (Caddy);
# the dev server reaches them via localhost with NODE_TLS_REJECT_UNAUTHORIZED=0
# and the SEMRUSH_PROJECTS_BASE_URL / SEMRUSH_USERS_BASE_URL split (api-service#2656).
# Public images → no registry credentials needed in CI. Control routes
# (/<prefix>/__reset, /__seed, /__dump) are unauthenticated; reset between tests.
#
# Image tag == the installed client dependency version. The mock is published
# from the SAME package as the typed client (@adobe/spacecat-shared-*-client),
# so testing against a different mock version than the client we ship would be
# an unsound contract test. setup.js (startContainers) reads the installed
# client version and exports SERENITY_PE_MOCK_TAG / SERENITY_UM_MOCK_TAG.
# The `:?` form FAILS HARD when the tag is unset — there is deliberately NO
# default: a hardcoded fallback would silently run a different version than the
# client we ship (chaos). If a client is bumped to a version whose mock image
# is not yet published, the pull also fails loudly, forcing the mock to be
# released in lockstep. Always launch via `mise run`/the harness, never a bare
# `docker compose up` (which would have no tag set).
project-engine-mock:
image: ghcr.io/adobe/spacecat-shared-project-engine-client-mock:${SERENITY_PE_MOCK_TAG:?must be set to the installed @adobe/spacecat-shared-project-engine-client version (exported by test/it/postgres/setup.js)}
container_name: spacecat-it-pe-mock
environment:
MOCK_SEED: workspace-with-data
ports:
- "${IT_PE_MOCK_PORT:-8443}:8443"
healthcheck:
test: ["CMD", "curl", "-ksf", "https://localhost:8443/enterprise/projects/api/__dump"]
interval: 5s
timeout: 3s
retries: 20
start_period: 20s

user-manager-mock:
image: ghcr.io/adobe/spacecat-shared-user-manager-client-mock:${SERENITY_UM_MOCK_TAG:?must be set to the installed @adobe/spacecat-shared-user-manager-client version (exported by test/it/postgres/setup.js)}
container_name: spacecat-it-um-mock
environment:
MOCK_SEED: parent-with-child
ports:
- "${IT_UM_MOCK_PORT:-8444}:8443"
healthcheck:
test: ["CMD", "curl", "-ksf", "https://localhost:8443/enterprise/users/api/__dump"]
interval: 5s
timeout: 3s
retries: 20
start_period: 20s
6 changes: 3 additions & 3 deletions test/it/postgres/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { initAuth, createAllTokens } from '../shared/auth.js';
import { buildEnv } from '../env.js';
import { startServer, stopServer } from '../server.js';
import { createHttpClient } from '../shared/http-client.js';
import { startPostgres, stopPostgres } from './setup.js';
import { startContainers, stopContainers } from './setup.js';

/** Shared state populated during beforeAll, consumed by test files. */
export const ctx = {};
Expand All @@ -43,7 +43,7 @@ export const mochaHooks = {
const { publicKeyB64 } = await initAuth();
const tokens = await createAllTokens();

await startPostgres();
await startContainers();

const env = buildEnv(publicKeyB64);
const baseUrl = await startServer(env);
Expand All @@ -56,6 +56,6 @@ export const mochaHooks = {
async afterAll() {
this.timeout(HARNESS_HOOK_TIMEOUT_MS);
await stopServer();
await stopPostgres();
await stopContainers();
},
};
3 changes: 2 additions & 1 deletion test/it/postgres/serenity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
*/

import { ctx } from './harness.js';
import { resetPostgres } from './seed.js';
import serenityTests from '../shared/tests/serenity.js';

serenityTests(() => ctx.httpClient);
serenityTests(() => ctx.httpClient, resetPostgres);
Loading
Loading