Skip to content

Commit a76bc4e

Browse files
authored
fix(source-dalux): route a different-node Dalux download link through the relay (#3370)
* fix(source-dalux): route a different-node Dalux download link through the relay (#3308) download()'s downloadLink/revision-content fallback can hand back a URL on the customer's own field node (e.g. node2), not the canonical node1 origin every other request is built against. That URL never matched the relay's declared upstream, so it went out as a direct cross-origin fetch and Dalux's CORS-free response got blocked by the browser — reproducing the "Failed to fetch" CORS error reported in #3308. getBinary now recognises a same-shaped Dalux field-node URL on a different node and reroutes it onto the canonical origin with that node stamped as daluxNode, the same way a node preference is already stamped on same-origin requests. A genuinely different host (an opaque signed link) is still left byte-for-byte untouched. * fix(source-dalux): match the relay base path at a boundary, not by bare prefix `canonicalFieldNodeUrl` gated on `parsed.pathname.startsWith(base.pathname)`. With a base of `/service/api` that also admits the SIBLING path `/service/api-v2/...` — a different API surface, which would then be rewritten onto our own origin and sent through the relay as if it were ours. Now matches the base path exactly or at a `/` boundary, with the base's trailing slash normalised so `/service/api/` behaves the same as `/service/api`. Reported by CodeRabbit on this PR and confirmed against the code before fixing. RED proven: with the source reverted, the new sibling-path test fails while the sixteen existing tests still pass. The companion test asserts the exact base path and a real child are still accepted, so this is a boundary check rather than an over-tightening. packages/source-dalux: 165 passed, 12 skipped.
1 parent 8904273 commit a76bc4e

4 files changed

Lines changed: 185 additions & 8 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@ifc-lite/source-dalux': patch
3+
---
4+
5+
Fix "Failed to fetch" downloading a Dalux Box file for an account on a node other than node1.
6+
7+
`download()` falls back to a `downloadLink`/revision-content URL Dalux hands back when the file carries no known revision id. For an account on, say, node2, that URL points straight at `node2.field.dalux.com` — not the canonical `node1.field.dalux.com` origin every request in this provider is otherwise built against. The client only ever routed URLs matching that canonical origin through the app's same-origin relay, so this one went out as a direct cross-origin fetch to Dalux, which sends no CORS headers on any node, and the browser blocked it.
8+
9+
`getBinary` now recognises a Dalux field-node-shaped URL that lands on a *different* node than `baseUrl` and reroutes it back onto the canonical origin with that node stamped as the relay's `daluxNode` parameter, the same way an explicit node preference is already stamped onto same-origin requests. A genuinely different host (an opaque signed CDN link) is still left byte-for-byte untouched.

packages/source-dalux/src/http-client.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
44

55
import type { PluginContext } from '@ifc-lite/plugin-api';
6+
import { canonicalFieldNodeUrl } from './node-url.js';
67

78
export interface DaluxCredentials {
89
readonly baseUrl: string;
@@ -171,13 +172,12 @@ export class BrowserDaluxApiClient {
171172
// Missing it here would send file downloads to the default node while
172173
// listings went to the user's own — the failure would look like "the file
173174
// is gone" rather than "wrong host".
174-
// Only re-serialise when we actually added the selector. `new URL(x)
175-
// .toString()` is NOT identity: it strips a default port, normalises `.`
176-
// and `..` path segments and can re-case percent escapes, any of which
177-
// changes a URL whose signature was computed over the original string.
178-
// Round-tripping unconditionally would have re-broken the exact links the
179-
// stamping guard above exists to protect.
180-
const url = this.nodeSelectorFor(rawUrl) ?? rawUrl;
175+
// Only re-serialise when we added the selector or rerouted onto the
176+
// canonical origin (`canonicalFieldNodeUrl`, #3308) — `new URL(x)
177+
// .toString()` is NOT identity, so doing it unconditionally would
178+
// re-break the signed links the guard above protects.
179+
const url =
180+
this.nodeSelectorFor(rawUrl) ?? canonicalFieldNodeUrl(rawUrl, this.credentials.baseUrl) ?? rawUrl;
181181
this.debug('binary GET request', { url });
182182
const response = await this.ctx.fetch(url, {
183183
headers: {

packages/source-dalux/src/node-url.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,57 @@ export function parseDaluxNode(raw: string | undefined | null): string | undefin
4646
}
4747
return match[1] === 'node1' ? undefined : match[1];
4848
}
49+
50+
/** Matches a Dalux field-node hostname exactly, e.g. `node2.field.dalux.com`. */
51+
const DALUX_NODE_HOST_PATTERN = /^(node[1-9][0-9]{0,2})\.field\.dalux\.com$/;
52+
53+
/**
54+
* Returns the node name if `hostname` is a Dalux field node, or `undefined`
55+
* otherwise.
56+
*
57+
* Unlike {@link parseDaluxNode} this never throws: it is used to
58+
* opportunistically recognise a Dalux-shaped URL the API itself handed back
59+
* (a `downloadLink`), not to validate user input, so an unrecognised host is
60+
* simply "not a Dalux field node" rather than an error.
61+
*/
62+
export function daluxFieldNode(hostname: string): string | undefined {
63+
return DALUX_NODE_HOST_PATTERN.exec(hostname)?.[1];
64+
}
65+
66+
/**
67+
* Reroutes a `rawUrl` on a *different* Dalux field node than `baseUrl` back
68+
* onto `baseUrl`'s origin, stamping that node as `daluxNode` — or
69+
* `undefined` if `rawUrl` isn't field-node-shaped.
70+
*
71+
* A `downloadLink`/revision-content value from a non-node1 account points
72+
* straight at that node using our own `/service/api` shape — not always the
73+
* opaque, differently-hosted link `BrowserDaluxApiClient.nodeSelectorFor`
74+
* describes. Untouched it never matches the relay's upstream, so the
75+
* browser fetches Dalux directly and CORS blocks it (#3308). The node comes
76+
* from `rawUrl` itself, which is authoritative; re-serialising is safe
77+
* because the relay rebuilds the request from the forwarded path/query
78+
* regardless of `rawUrl`'s exact bytes.
79+
*/
80+
export function canonicalFieldNodeUrl(rawUrl: string, baseUrl: string): string | undefined {
81+
let parsed: URL;
82+
try {
83+
parsed = new URL(rawUrl);
84+
} catch {
85+
return undefined;
86+
}
87+
const base = new URL(baseUrl);
88+
if (parsed.origin === base.origin) return undefined; // caller's own-origin path
89+
// Boundary-checked, not a bare `startsWith`: with a base path of
90+
// `/service/api`, a bare prefix test also admits the SIBLING path
91+
// `/service/api-v2/...`, which is a different API surface and would be
92+
// rerouted through the relay as if it were ours. Match the path exactly, or
93+
// at a `/` boundary.
94+
const basePath = base.pathname.endsWith('/') ? base.pathname.slice(0, -1) : base.pathname;
95+
if (parsed.pathname !== basePath && !parsed.pathname.startsWith(`${basePath}/`)) return undefined;
96+
const node = daluxFieldNode(parsed.hostname);
97+
if (!node) return undefined;
98+
parsed.protocol = base.protocol;
99+
parsed.host = base.host;
100+
parsed.searchParams.set('daluxNode', node);
101+
return parsed.toString();
102+
}

packages/source-dalux/test/node-url.test.ts

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
*/
1313

1414
import { describe, expect, it } from 'vitest';
15-
import { parseDaluxNode } from '../src/node-url.js';
15+
import { canonicalFieldNodeUrl, daluxFieldNode, parseDaluxNode } from '../src/node-url.js';
1616
import { BrowserDaluxApiClient } from '../src/http-client.js';
1717

1818
describe('parseDaluxNode', () => {
@@ -129,4 +129,118 @@ describe('node stamping is scoped to the relay', () => {
129129
await client.getBinary(lookalike);
130130
expect(seen[0]).toBe(lookalike);
131131
});
132+
133+
it('reroutes a downloadLink pointing at a different Dalux field node through the relay origin (#3308)', async () => {
134+
// A customer whose account lives on node2 (not node1) sees Dalux hand
135+
// back a `downloadLink`/revision-content URL built on THEIR node, using
136+
// the same `/service/api` REST shape our own requests use — this is not
137+
// the opaque-CDN-link case the tests above cover. `baseUrl` is always the
138+
// canonical node1 origin (see `provider.ts#createClient`), so this URL's
139+
// origin never matches it. Left alone, it would never be rewritten onto
140+
// the same-origin relay by the host (`applyRelay` in `host-fetch.ts`
141+
// only rewrites URLs starting with the declared relay upstream, node1),
142+
// and the browser would attempt a direct cross-origin fetch to Dalux —
143+
// which fails, because Dalux sends no CORS headers from any node. The
144+
// fix must land it back on the node1 origin with the real node stamped
145+
// as `daluxNode`, exactly like the node-preference case above.
146+
const seen: string[] = [];
147+
const ctx = {
148+
fetch: async (url: string) => {
149+
seen.push(url);
150+
return new Response(new ArrayBuffer(2), { status: 200 });
151+
},
152+
log: { debug() {}, error() {}, info() {}, warn() {} },
153+
} as unknown as ConstructorParameters<typeof BrowserDaluxApiClient>[1];
154+
155+
// No `node` preference set — the account's node preference is
156+
// irrelevant here; what matters is the node baked into the URL itself.
157+
const client = new BrowserDaluxApiClient(
158+
{ baseUrl: 'https://node1.field.dalux.com/service/api', apiKey: 'k' },
159+
ctx,
160+
);
161+
162+
const otherNodeLink =
163+
'https://node2.field.dalux.com/service/api/2.0/projects/p1/file_areas/fa1/files/f1/revisions/r1/content?Signature=abc';
164+
await client.getBinary(otherNodeLink);
165+
166+
expect(seen[0]).not.toBe(otherNodeLink);
167+
const fetched = new URL(seen[0]);
168+
expect(fetched.origin).toBe('https://node1.field.dalux.com');
169+
expect(fetched.pathname).toBe('/service/api/2.0/projects/p1/file_areas/fa1/files/f1/revisions/r1/content');
170+
expect(fetched.searchParams.get('daluxNode')).toBe('node2');
171+
expect(fetched.searchParams.get('Signature')).toBe('abc');
172+
});
173+
});
174+
175+
describe('daluxFieldNode', () => {
176+
it('recognises a Dalux field-node hostname', () => {
177+
expect(daluxFieldNode('node1.field.dalux.com')).toBe('node1');
178+
expect(daluxFieldNode('node2.field.dalux.com')).toBe('node2');
179+
expect(daluxFieldNode('node10.field.dalux.com')).toBe('node10');
180+
});
181+
182+
it('rejects anything that is not exactly a Dalux field node', () => {
183+
for (const host of [
184+
'cdn.dalux.com',
185+
'node1.field.dalux.com.evil.com',
186+
'node0.field.dalux.com',
187+
'field.dalux.com',
188+
'evil.com',
189+
]) {
190+
expect(daluxFieldNode(host), host).toBeUndefined();
191+
}
192+
});
193+
});
194+
195+
describe('canonicalFieldNodeUrl', () => {
196+
const baseUrl = 'https://node1.field.dalux.com/service/api';
197+
198+
it('rewrites a different-node URL onto baseUrl, stamping the real node', () => {
199+
const result = canonicalFieldNodeUrl(
200+
'https://node3.field.dalux.com/service/api/2.0/x/content?a=b',
201+
baseUrl,
202+
);
203+
expect(result).toBeDefined();
204+
const url = new URL(result!);
205+
expect(url.origin).toBe('https://node1.field.dalux.com');
206+
expect(url.pathname).toBe('/service/api/2.0/x/content');
207+
expect(url.searchParams.get('daluxNode')).toBe('node3');
208+
expect(url.searchParams.get('a')).toBe('b');
209+
});
210+
211+
it('returns undefined for a URL already on baseUrl’s origin', () => {
212+
expect(canonicalFieldNodeUrl('https://node1.field.dalux.com/service/api/2.0/x', baseUrl)).toBeUndefined();
213+
});
214+
215+
it('returns undefined for a host that is not a Dalux field node', () => {
216+
expect(canonicalFieldNodeUrl('https://cdn.dalux.com/files/abc?Signature=x', baseUrl)).toBeUndefined();
217+
});
218+
219+
it('returns undefined when the path does not share the base path', () => {
220+
expect(canonicalFieldNodeUrl('https://node2.field.dalux.com/other/2.0/x', baseUrl)).toBeUndefined();
221+
});
222+
223+
it('returns undefined for an unparseable URL', () => {
224+
expect(canonicalFieldNodeUrl('not a url', baseUrl)).toBeUndefined();
225+
});
226+
227+
// A bare `startsWith(base.pathname)` also admits a SIBLING path: with a base
228+
// of `/service/api`, `/service/api-v2/...` shares the prefix without being
229+
// under it. That is a different API surface, and rerouting it through the
230+
// relay would send a request we do not own to our own origin.
231+
it('returns undefined for a sibling path that merely shares the base prefix', () => {
232+
expect(
233+
canonicalFieldNodeUrl('https://node2.field.dalux.com/service/api-v2/2.0/x', baseUrl),
234+
).toBeUndefined();
235+
expect(
236+
canonicalFieldNodeUrl('https://node2.field.dalux.com/service/apiary/x', baseUrl),
237+
).toBeUndefined();
238+
});
239+
240+
it('still accepts the base path exactly, and a real child of it', () => {
241+
expect(canonicalFieldNodeUrl('https://node2.field.dalux.com/service/api', baseUrl)).toBeDefined();
242+
expect(
243+
canonicalFieldNodeUrl('https://node2.field.dalux.com/service/api/2.0/x', baseUrl),
244+
).toBeDefined();
245+
});
132246
});

0 commit comments

Comments
 (0)