Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions apps/web/docker-compose.production.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ services:
- SEO_CRON_SECRET
# Server-side read-through RPC proxy (core/sdk-init.ts): reads go to
# vapi's /private-api/ssr/rpc over the overlay, with fallback to the node
# pool on any failure. Needs the same secret vapi was given above. The
# switch is an explicit value here, not a passthrough: production flips
# to 1 through a reviewed change to this line, after the alpha soak.
# pool on any failure. The secret is the switch for both services: set in
# the deploy job = on, blank = off on the next deploy. For an on-box kill
# without touching secrets: `docker service update --env-add
# SSR_RPC_PROXY=0 vision_web`.
- SSR_INTERNAL_SECRET
- SSR_RPC_PROXY=0
# Newsletter service (ecency/news), single instance on the EU box. EU reaches it
# on the swarm gateway address, US through the IP-allowlisted TLS relay on the EU
# origin; each deploy job supplies its own URL and refuses to deploy without them.
Expand Down
11 changes: 5 additions & 6 deletions apps/web/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,11 @@ services:
- SSR_MAX_INFLIGHT=16
# Server-side read-through RPC proxy (core/sdk-init.ts): reads go to
# vapi's /private-api/ssr/rpc over the overlay, with fallback to the node
# pool on any failure. Needs the same secret vapi was given above; the
# flag switches it on per deployment.
# The switch is on here, so the secret is required: a deploy without it
# would silently run without the proxy instead of exercising it.
- SSR_INTERNAL_SECRET=${SSR_INTERNAL_SECRET:?SSR_INTERNAL_SECRET is required while SSR_RPC_PROXY=1}
- SSR_RPC_PROXY=1
# pool on any failure. The secret is the switch for both services, and
# on alpha it is required (`:?`): `docker-compose config` fails the deploy
# without it, instead of staging quietly running without the proxy it is
# meant to exercise before production.
- SSR_INTERNAL_SECRET=${SSR_INTERNAL_SECRET:?SSR_INTERNAL_SECRET is required on alpha so staging always exercises the proxy}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
restart: always
ports:
- "3000:3000"
Expand Down
9 changes: 6 additions & 3 deletions apps/web/src/core/sdk-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,18 @@ if (isServer) {
// per-tag feeds, posts) are answered from one cache per host instead of
// being fetched by every renderer process on its own. An optimization, not
// a dependency: the SDK falls back to the node pool on any proxy failure.
// Switched on per deployment (SSR_RPC_PROXY=1) and only when both sides
// carry the shared secret; INTERNAL_API_HOST is the overlay route to vapi.
// On whenever the deployment hands this process the shared secret (vapi
// switches its side on from the same value) and the overlay route to vapi
// (INTERNAL_API_HOST). No separate switch: the secret is the switch, in one
// place, for both services. SSR_RPC_PROXY=0 is an explicit off for an
// on-box kill that leaves the secret alone.
// The timeout sits just above vapi's own lookup budget (1.5s), so a proxy
// that cannot answer in time is its 504, and the SDK's per-node timeout
// bounds it further; the prefetch's own abort signal bounds the whole call
// either way, so the proxy can never extend a render past the SSR cap.
const proxyHost = process.env.INTERNAL_API_HOST;
const proxySecret = process.env.SSR_INTERNAL_SECRET;
if (process.env.SSR_RPC_PROXY === "1" && proxyHost && proxySecret) {
if (process.env.SSR_RPC_PROXY !== "0" && proxyHost && proxySecret) {
ConfigManager.setServerRpcProxy({
url: `${proxyHost.replace(/\/+$/, "")}/private-api/ssr/rpc`,
headers: { "X-Ecency-Internal": proxySecret },
Expand Down
46 changes: 34 additions & 12 deletions apps/web/src/specs/core/sdk-init-proxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
import { afterEach, describe, expect, it, vi } from "vitest";

/**
* core/sdk-init.ts switches the server-side RPC proxy on at import time, and
* only when the deployment asked for it AND both halves of the wiring are
* present. A module with import-time side effects, so each case gets a fresh
* module registry and its own environment.
* core/sdk-init.ts switches the server-side RPC proxy on at import time
* whenever both halves of the wiring (shared secret, overlay host) are
* present; the secret is the switch, SSR_RPC_PROXY=0 the explicit off. A
* module with import-time side effects, so each case gets a fresh module
* registry and its own environment.
*/
const stats = {
served: 0,
Expand All @@ -26,12 +27,22 @@ const manager = {
vi.mock("@ecency/sdk", () => ({ ConfigManager: manager }));

const REPORT_MS = 5 * 60 * 1000;
const ON = { SSR_RPC_PROXY: "1", SSR_INTERNAL_SECRET: "s3cret", INTERNAL_API_HOST: "http://vapi:4000" };
const ON = { SSR_RPC_PROXY: undefined, SSR_INTERNAL_SECRET: "s3cret", INTERNAL_API_HOST: "http://vapi:4000" };
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const PROXY_VARS = ["SSR_RPC_PROXY", "SSR_INTERNAL_SECRET", "INTERNAL_API_HOST"] as const;

/**
* Hermetic: every proxy variable is set from the case, and a case that omits
* one UNSETS it (stubEnv with undefined), so nothing leaks in from the test
* process and "missing" and "blank" are different environments.
*/
async function load(env: Record<string, string | undefined>): Promise<void> {
vi.resetModules();
for (const k of PROXY_VARS) {
vi.stubEnv(k, env[k]);
}
for (const [k, v] of Object.entries(env)) {
vi.stubEnv(k, v ?? "");
if (!(PROXY_VARS as readonly string[]).includes(k)) vi.stubEnv(k, v);
}
await import("@/core/sdk-init");
}
Expand All @@ -51,8 +62,18 @@ afterEach(() => {
});

describe("sdk-init server rpc proxy", () => {
it("enables the proxy against the overlay host with the shared secret when switched on", async () => {
await load({ SSR_RPC_PROXY: "1", SSR_INTERNAL_SECRET: "s3cret", INTERNAL_API_HOST: "http://vapi:4000/" });
it("enables the proxy against the overlay host as soon as the shared secret is present", async () => {
await load({ SSR_INTERNAL_SECRET: "s3cret", INTERNAL_API_HOST: "http://vapi:4000/" });
Comment on lines +65 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Flaky env-dependent proxy test 🐞 Bug ☼ Reliability

The test that asserts the proxy enables “as soon as the shared secret is present” no longer stubs
SSR_RPC_PROXY, so an ambient SSR_RPC_PROXY=0 in the test runner environment will disable the proxy
and fail the test. This makes the spec non-hermetic and can cause CI/local flakiness unrelated to
the code under test.
Agent Prompt
### Issue description
`apps/web/src/specs/core/sdk-init-proxy.spec.ts` now has a test that calls `load()` without setting `SSR_RPC_PROXY`. Since `load()` only stubs env vars explicitly passed, `process.env.SSR_RPC_PROXY` can leak in from the outer test process. If it is set to `"0"`, `core/sdk-init.ts` will skip enabling the proxy and the test will fail.

### Issue Context
- `core/sdk-init.ts` enables the proxy when `SSR_RPC_PROXY !== "0"`.
- `load()` only calls `vi.stubEnv` for keys present in the `env` argument.

### Fix Focus Areas
- apps/web/src/specs/core/sdk-init-proxy.spec.ts[32-38]
- apps/web/src/specs/core/sdk-init-proxy.spec.ts[55-56]

### Suggested change
Make `load()` hermetic by always stubbing `SSR_RPC_PROXY` (and optionally the other related vars) to a known baseline before importing:
- e.g. at the start of `load()`: `vi.stubEnv("SSR_RPC_PROXY", "");`
- or change the first test to call `load(ON)`-style input (include `SSR_RPC_PROXY: undefined` explicitly) so it overrides any ambient value.
- alternatively delete the env var (`delete process.env.SSR_RPC_PROXY`) before import, if preferred for simulating “unset”.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in 113d24d: load() now stubs all three proxy variables for every case, and a variable the case omits is UNSET (vi.stubEnv(name, undefined)), so nothing leaks in from the test process and "secret missing" and "secret blank" are different environments. Verified by running the spec with SSR_RPC_PROXY=0 exported in the runner: 15/15 pass.

Comment thread
qodo-code-review[bot] marked this conversation as resolved.
expect(manager.setServerRpcProxy).toHaveBeenCalledWith({
url: "http://vapi:4000/private-api/ssr/rpc",
headers: { "X-Ecency-Internal": "s3cret" },
timeoutMs: 1600
});
});

it("a legacy SSR_RPC_PROXY=1 changes nothing", async () => {
await load({ ...ON, SSR_RPC_PROXY: "1" });
expect(manager.setServerRpcProxy).toHaveBeenCalledTimes(1);
expect(manager.setServerRpcProxy).toHaveBeenCalledWith({
url: "http://vapi:4000/private-api/ssr/rpc",
headers: { "X-Ecency-Internal": "s3cret" },
Expand All @@ -61,9 +82,10 @@ describe("sdk-init server rpc proxy", () => {
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it.each([
["the switch is off", { SSR_RPC_PROXY: undefined, SSR_INTERNAL_SECRET: "s3cret", INTERNAL_API_HOST: "http://vapi:4000" }],
["the secret is missing", { SSR_RPC_PROXY: "1", SSR_INTERNAL_SECRET: undefined, INTERNAL_API_HOST: "http://vapi:4000" }],
["the host is missing", { SSR_RPC_PROXY: "1", SSR_INTERNAL_SECRET: "s3cret", INTERNAL_API_HOST: undefined }]
["explicitly switched off", { ...ON, SSR_RPC_PROXY: "0" }],
["the secret is missing", { ...ON, SSR_INTERNAL_SECRET: undefined }],
["the secret is blank", { ...ON, SSR_INTERNAL_SECRET: "" }],
["the host is missing", { ...ON, INTERNAL_API_HOST: undefined }]
])("stays off when %s", async (_label, env) => {
await load(env);
expect(manager.setServerRpcProxy).not.toHaveBeenCalled();
Expand Down Expand Up @@ -100,7 +122,7 @@ describe("sdk-init server rpc proxy", () => {
it("does not start when the proxy is off", async () => {
vi.useFakeTimers();
const log = vi.spyOn(console, "log").mockImplementation(() => {});
await load({ ...ON, SSR_RPC_PROXY: undefined });
await load({ ...ON, SSR_INTERNAL_SECRET: undefined });
await vi.advanceTimersByTimeAsync(REPORT_MS * 2);
expect(manager.getServerRpcProxyStats).not.toHaveBeenCalled();
expect(log).not.toHaveBeenCalled();
Expand Down
24 changes: 13 additions & 11 deletions apps/web/src/specs/deploy/ssr-proxy-wiring.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import { describe, expect, it } from "vitest";

/**
* The server-side RPC proxy is decided at process start in core/sdk-init.ts
* from SSR_RPC_PROXY, SSR_INTERNAL_SECRET and INTERNAL_API_HOST, and vapi only
* switches its side on when it holds the same secret. A variable missing from
* any one of these places is silent: the SDK simply keeps going to the node
* pool. This pins the whole chain: both services in both stack files, the
* deploy jobs forwarding the secret, and the origin proxy hiding the path.
* from SSR_INTERNAL_SECRET and INTERNAL_API_HOST, and vapi only switches its
* side on when it holds the same secret: the secret is the switch for both.
* A variable missing from any one of these places is silent: the SDK simply
* keeps going to the node pool. This pins the whole chain: both services in
* both stack files, the deploy jobs forwarding the secret, the stack files
* not pinning the proxy off behind the secret's back, and the origin proxy
* hiding the path.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
*/
const root = join(__dirname, "..", "..", "..", "..", "..");
const read = (p: string): string => readFileSync(join(root, p), "utf8");
Expand All @@ -33,23 +35,23 @@ const envNames = (entries: string[]): string[] => entries.map((e) => e.split("="

describe("ssr rpc proxy deploy wiring", () => {
it.each(["apps/web/docker-compose.yml", "apps/web/docker-compose.production.yml"])(
"%s hands the secret to vapi and to web, and web carries the switch",
"%s hands the secret to vapi and to web, and does not pin the proxy off behind it",
(file) => {
const compose = read(file);
expect(envEntries(serviceBlock(compose, "vapi")), `${file}: vapi`).toContain("SSR_INTERNAL_SECRET");
const web = envEntries(serviceBlock(compose, "web"));
expect(envNames(web), `${file}: web`).toContain("SSR_INTERNAL_SECRET");
expect(web.some((e) => e === "SSR_RPC_PROXY=0" || e === "SSR_RPC_PROXY=1"), `${file}: web switch`).toBe(true);
// The secret is the switch. A literal SSR_RPC_PROXY anywhere in the stack
// file would make it lie about what the deploy job handed over.
expect(envNames(envEntries(compose)), `${file}: no separate switch`).not.toContain("SSR_RPC_PROXY");
}
);

it("alpha has the switch on and requires the secret; production carries an explicit value, never a bare passthrough", () => {
it("alpha requires the secret so staging always exercises the proxy; production passes it through", () => {
const alpha = envEntries(serviceBlock(read("apps/web/docker-compose.yml"), "web"));
expect(alpha).toContain("SSR_RPC_PROXY=1");
expect(alpha.some((e) => /^SSR_INTERNAL_SECRET=\$\{SSR_INTERNAL_SECRET:\?/.test(e))).toBe(true);
const prod = envEntries(serviceBlock(read("apps/web/docker-compose.production.yml"), "web"));
expect(prod).not.toContain("SSR_RPC_PROXY");
expect(prod.some((e) => /^SSR_RPC_PROXY=[01]$/.test(e))).toBe(true);
expect(prod).toContain("SSR_INTERNAL_SECRET");
});

it.each(["master.yml", "staging.yml"])(".github/workflows/%s forwards the secret end to end", (file) => {
Expand Down
Loading