Skip to content

Commit 159042a

Browse files
committed
feat(spec): ConnectorProviderContext.resolvePackagePath, a host-provided package anchor
ConnectorProviderContext gains one optional, host-provided member, resolvePackagePath(relativePath), beside loadPackageFile. It resolves a relative path against the declaring stack/package root, returns the root for '.', and refuses an empty, absolute or root-escaping path. service-automation's materializer hands it to every provider factory, anchored at packageRoot (default process.cwd()), and both members now share one confinement check (resolveInsidePackageRoot) so they cannot disagree. loadPackageFile's behaviour and error strings are unchanged. Claude-Session: https://claude.ai/code/session_01VZqqwTj2wsihZEbfT6yyYN Co-authored-by: Claude <noreply@anthropic.com>
1 parent da159f7 commit 159042a

5 files changed

Lines changed: 281 additions & 28 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-automation": minor
4+
---
5+
6+
feat(spec): `ConnectorProviderContext.resolvePackagePath` — a connector provider factory can resolve a path against the declaring app's root
7+
8+
Clause-②: yes (widening)
9+
10+
- **What is new.** `ConnectorProviderContext` gains one optional, host-provided member, `resolvePackagePath(relativePath): Promise<string>`, beside `loadPackageFile`. It resolves a relative path against the root of the stack or package that declared the connector entry and returns the absolute path. `'.'` returns the root itself. It refuses (throws on) an empty path, an absolute path, and a path that escapes the root after resolution, such as `../x` or `a/../../x`. That is the same rule `loadPackageFile` follows. It reads nothing and does not check that the path exists.
11+
- **Who hands it.** The connector materializer in `@objectstack/service-automation` hands it to every provider factory. It is anchored at the plugin's `packageRoot` option, the same root `loadPackageFile` reads under, and falls back to `process.cwd()` the same way. The two members now share one confinement check, so they cannot disagree about what is inside the root. `loadPackageFile`'s behaviour and error messages are unchanged.
12+
- **Who reads it.** Nothing yet. `@objectstack/connector-mcp` is next: a declarative stdio transport will use it as the launched process's working directory, so a relative command or argument resolves against the app's root instead of the directory the server was started from.
13+
- **What does not change.** Which commands a declarative stdio transport may launch is untouched. A factory that does not read the member behaves as before.
14+
- **Nothing to migrate.** A host that builds its own `ConnectorProviderContext` may leave the member out. A factory must then keep its existing behaviour, or fail with a clear message if it needs the root.

‎packages/services/service-automation/src/connector-materialization.test.ts‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { ConnectorUpstreamUnavailableError } from '@objectstack/spec/integration
2424
import {
2525
AutomationServicePlugin,
2626
createPackageFileLoader,
27+
createPackagePathResolver,
2728
DECLARATIVE_RETRY_BASE_MS,
2829
type CredentialResolver,
2930
} from './plugin.js';
@@ -542,6 +543,127 @@ describe('#3016 — file-ref materialization policy (fatal at boot, soft on relo
542543
});
543544
});
544545

546+
// ── #22434 — package path refs (resolvePackagePath) ─────────────────────────
547+
//
548+
// The sibling of `loadPackageFile` for a factory that needs a LOCATION — a
549+
// launched process's working directory — rather than file contents. Same root,
550+
// same confinement rule (one shared check), same `process.cwd()` default. It
551+
// reads nothing, so a resolved path need not exist.
552+
553+
/** The rejection a promise settles with, or a fail-loud marker if it resolved. */
554+
async function rejectionOf(p: Promise<unknown>): Promise<Error> {
555+
return p.then(
556+
(value) => { throw new Error(`expected a refusal, got ${JSON.stringify(value)}`); },
557+
(err: unknown) => err as Error,
558+
);
559+
}
560+
561+
describe('#22434 — package path resolver (createPackagePathResolver)', () => {
562+
const root = path.resolve(fixtureRoot);
563+
const resolvePath = createPackagePathResolver(fixtureRoot);
564+
565+
it('resolves a relative path against the package root, and `.` to the root itself', async () => {
566+
await expect(resolvePath('.')).resolves.toBe(root);
567+
await expect(resolvePath('./specs/billing.json')).resolves.toBe(path.join(root, 'specs', 'billing.json'));
568+
await expect(resolvePath('specs')).resolves.toBe(path.join(root, 'specs'));
569+
// Judged on the RESOLVED path: a traversal that lands back inside passes.
570+
await expect(resolvePath('./specs/../specs')).resolves.toBe(path.join(root, 'specs'));
571+
// A location, not a read: a path that does not exist yet still resolves.
572+
await expect(resolvePath('./scripts/not-there.mjs')).resolves.toBe(path.join(root, 'scripts', 'not-there.mjs'));
573+
});
574+
575+
it('refuses a path that escapes the root, naming the ref and the root it is confined to', async () => {
576+
for (const ref of ['..', '../outside', 'specs/../../outside']) {
577+
const err = await rejectionOf(resolvePath(ref));
578+
expect(err).toBeInstanceOf(Error);
579+
expect(err).not.toBeInstanceOf(TypeError);
580+
expect(err.message).toContain(`'${ref}' escapes the stack/package root`);
581+
expect(err.message).toContain(`confined to '${root}'`);
582+
}
583+
});
584+
585+
it('refuses an absolute path (posix and windows-drive) and an empty ref', async () => {
586+
for (const ref of [path.join(root, 'specs'), '/etc', 'C:\\evil']) {
587+
const err = await rejectionOf(resolvePath(ref));
588+
expect(err).not.toBeInstanceOf(TypeError);
589+
expect(err.message).toContain(`'${ref}' is absolute`);
590+
}
591+
for (const ref of ['', ' ']) {
592+
expect((await rejectionOf(resolvePath(ref))).message).toContain('non-empty relative path');
593+
}
594+
});
595+
596+
it('shares the loader\'s rule: a ref is refused for confinement by both members or by neither', async () => {
597+
const load = createPackageFileLoader(fixtureRoot);
598+
const confinement = /non-empty relative path|is absolute|escapes the stack\/package root/;
599+
const refs = ['.', 'specs', './specs/billing.json', './specs/../specs/billing.json', '', '..', '../x',
600+
'specs/../../x', '/etc/hostname', 'C:/evil', 'D:\\evil'];
601+
for (const ref of refs) {
602+
const loaderRefused = await load(ref).then(() => false, (e: Error) => confinement.test(e.message));
603+
const resolverRefused = await resolvePath(ref).then(() => false, (e: Error) => confinement.test(e.message));
604+
expect(resolverRefused, `ref '${ref}'`).toBe(loaderRefused);
605+
}
606+
});
607+
608+
it('defaults to process.cwd() when no packageRoot is given, as the loader does', async () => {
609+
await expect(createPackagePathResolver()('.')).resolves.toBe(path.resolve(process.cwd()));
610+
});
611+
});
612+
613+
describe('#22434 — the materializer hands resolvePackagePath anchored to packageRoot', () => {
614+
/** A provider whose factory resolves the refs in `providerConfig.paths` before materializing. */
615+
function makePathResolvingProvider() {
616+
const resolved: string[] = [];
617+
const factory: ConnectorProviderFactory = async (ctx) => {
618+
for (const ref of (ctx.providerConfig.paths as string[]) ?? []) {
619+
resolved.push(await ctx.resolvePackagePath!(ref));
620+
}
621+
return {
622+
def: { name: ctx.name, label: ctx.label, type: 'api', authentication: { type: 'none' }, actions: [{ key: 'ping', label: 'Ping' }] } as unknown as Connector,
623+
handlers: { ping: async () => ({ ok: true }) },
624+
};
625+
};
626+
return { factory, resolved };
627+
}
628+
629+
it('a factory resolving `.` and a relative path gets locations under packageRoot', async () => {
630+
const { factory, resolved } = makePathResolvingProvider();
631+
const kernel = await boot(
632+
[providerConnector('tools', { providerConfig: { paths: ['.', './scripts/fixture.mjs'] } })],
633+
{ providerFactory: factory, packageRoot: fixtureRoot },
634+
);
635+
expect(automationOf(kernel).getRegisteredConnectors()).toContain('tools');
636+
expect(resolved).toEqual([path.resolve(fixtureRoot), path.join(path.resolve(fixtureRoot), 'scripts', 'fixture.mjs')]);
637+
await kernel.shutdown();
638+
});
639+
640+
it('fails boot loudly when a factory resolves a path that escapes the package root', async () => {
641+
const { factory, resolved } = makePathResolvingProvider();
642+
await expect(
643+
boot([providerConnector('tools', { providerConfig: { paths: ['../outside'] } })], {
644+
providerFactory: factory,
645+
packageRoot: fixtureRoot,
646+
}),
647+
).rejects.toThrow(/failed to materialize connector instance 'tools'.*'\.\.\/outside' escapes the stack\/package root/s);
648+
expect(resolved).toEqual([]);
649+
});
650+
651+
it('a factory that never reads it materializes exactly as before', async () => {
652+
const { factory, calls } = makeFakeProvider();
653+
const kernel = await boot([providerConnector('billing')], { providerFactory: factory, packageRoot: fixtureRoot });
654+
const engine = automationOf(kernel);
655+
expect(engine.getRegisteredConnectors()).toContain('billing');
656+
// The member is handed (the factory simply ignores it) …
657+
expect(typeof calls[0]?.resolvePackagePath).toBe('function');
658+
// … and the connector it materialized is listed exactly as it always was.
659+
const desc = engine.getConnectorDescriptors().find((d) => d.name === 'billing');
660+
expect(desc?.actions.map((a) => a.key)).toEqual(['ping']);
661+
expect(desc?.origin).toBe('declarative');
662+
expect(calls).toHaveLength(1);
663+
await kernel.shutdown();
664+
});
665+
});
666+
545667
// ── #3017 — upstream unavailable: degrade + retry, never a dead boot ─────────
546668
//
547669
// A provider factory that throws the CONNECTOR_UPSTREAM_UNAVAILABLE marker

‎packages/services/service-automation/src/plugin.ts‎

Lines changed: 72 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -183,47 +183,75 @@ export interface AutomationServicePluginOptions {
183183
* against (#3016, e.g. `providerConfig.spec: './billing-openapi.json'` for
184184
* `provider: 'openapi'`). The CLI passes the directory containing
185185
* `objectstack.config.ts`; embedders pass their stack root. Defaults to
186-
* `process.cwd()`. Reads are confined to this root (see
187-
* {@link createPackageFileLoader}).
186+
* `process.cwd()`. Reads and resolved paths are confined to this root (see
187+
* {@link createPackageFileLoader} and {@link createPackagePathResolver}).
188188
*/
189189
packageRoot?: string;
190190
}
191191

192+
/**
193+
* The ONE confinement rule behind both package-anchored members of
194+
* {@link ConnectorProviderContext} — `loadPackageFile` (#3016) and
195+
* `resolvePackagePath` (#22434) — so the two can never disagree about what
196+
* "inside the stack/package root" means. Resolves `relativePath` against
197+
* `packageRoot` (default `process.cwd()`, read at call time) and returns the
198+
* absolute result. Rejects an empty ref, an absolute ref (posix or Windows
199+
* drive-letter), and any ref that escapes the root after resolution (`../…`,
200+
* `a/../../…`). The check is on the normalized path, not on `realpath`: a
201+
* symlink inside the root is judged by where it sits, not by where it points,
202+
* for both members alike.
203+
*
204+
* `confined` only words the escape refusal for the member that raised it
205+
* (`reads` for the loader, whose text is pinned verbatim by the platform
206+
* checklist, `paths` for the resolver); the rule itself is identical.
207+
*
208+
* `node:path` is imported lazily so merely constructing either capability
209+
* never touches it — hosts without a filesystem only fail if a factory
210+
* actually dereferences a package ref.
211+
*/
212+
async function resolveInsidePackageRoot(
213+
packageRoot: string | undefined,
214+
relativePath: string,
215+
confined: 'reads' | 'paths',
216+
): Promise<string> {
217+
if (typeof relativePath !== 'string' || relativePath.trim().length === 0) {
218+
throw new Error('package file ref must be a non-empty relative path.');
219+
}
220+
const path = await import('node:path');
221+
// Windows drive-letter absolutes ('C:\…') are not `isAbsolute` on posix —
222+
// reject them explicitly so the guard is platform-independent.
223+
if (path.isAbsolute(relativePath) || /^[a-zA-Z]:[\\/]/.test(relativePath)) {
224+
throw new Error(
225+
`package file ref '${relativePath}' is absolute — file refs must be relative to the declaring stack/package root.`,
226+
);
227+
}
228+
const root = path.resolve(packageRoot ?? process.cwd());
229+
const resolved = path.resolve(root, relativePath);
230+
const rel = path.relative(root, resolved);
231+
if (rel.startsWith('..') || path.isAbsolute(rel)) {
232+
throw new Error(
233+
`package file ref '${relativePath}' escapes the stack/package root — ${confined} are confined to '${root}'.`,
234+
);
235+
}
236+
return resolved;
237+
}
238+
192239
/**
193240
* Build the `loadPackageFile` capability handed to provider factories via
194241
* {@link ConnectorProviderContext} (#3016): read a UTF-8 text file resolved
195-
* against `packageRoot`, **confined to that root**. Rejects absolute paths and
196-
* any path that escapes the root after resolution (`../…`, `a/../../…`), so a
197-
* declarative entry can never read outside the stack/package that declared it.
198-
* A missing/unreadable file throws — the materializer's reconcile policy makes
199-
* that fatal at boot and a skipped entry on reload, like every other ADR-0097
200-
* materialization failure.
242+
* against `packageRoot`, **confined to that root** by
243+
* {@link resolveInsidePackageRoot}, so a declarative entry can never read
244+
* outside the stack/package that declared it. A missing/unreadable file throws
245+
* — the materializer's reconcile policy makes that fatal at boot and a skipped
246+
* entry on reload, like every other ADR-0097 materialization failure.
201247
*
202248
* Node builtins are imported lazily inside the returned closure so merely
203249
* constructing the capability never touches `node:fs`/`node:path` — hosts
204250
* without a filesystem only fail if a factory actually dereferences a file ref.
205251
*/
206252
export function createPackageFileLoader(packageRoot?: string): (relativePath: string) => Promise<string> {
207253
return async (relativePath: string) => {
208-
if (typeof relativePath !== 'string' || relativePath.trim().length === 0) {
209-
throw new Error('package file ref must be a non-empty relative path.');
210-
}
211-
const path = await import('node:path');
212-
// Windows drive-letter absolutes ('C:\…') are not `isAbsolute` on posix —
213-
// reject them explicitly so the guard is platform-independent.
214-
if (path.isAbsolute(relativePath) || /^[a-zA-Z]:[\\/]/.test(relativePath)) {
215-
throw new Error(
216-
`package file ref '${relativePath}' is absolute — file refs must be relative to the declaring stack/package root.`,
217-
);
218-
}
219-
const root = path.resolve(packageRoot ?? process.cwd());
220-
const resolved = path.resolve(root, relativePath);
221-
const rel = path.relative(root, resolved);
222-
if (rel.startsWith('..') || path.isAbsolute(rel)) {
223-
throw new Error(
224-
`package file ref '${relativePath}' escapes the stack/package root — reads are confined to '${root}'.`,
225-
);
226-
}
254+
const resolved = await resolveInsidePackageRoot(packageRoot, relativePath, 'reads');
227255
const { readFile } = await import('node:fs/promises');
228256
try {
229257
return await readFile(resolved, 'utf8');
@@ -235,6 +263,20 @@ export function createPackageFileLoader(packageRoot?: string): (relativePath: st
235263
};
236264
}
237265

266+
/**
267+
* Build the `resolvePackagePath` capability handed to provider factories via
268+
* {@link ConnectorProviderContext} (#22434): the absolute path of a ref
269+
* resolved against `packageRoot` — `'.'` is the root itself — **confined to
270+
* that root** by the same {@link resolveInsidePackageRoot} rule the loader
271+
* uses, with the same `process.cwd()` default. It reads nothing and does not
272+
* check existence; a factory that needs a location (a launched process's
273+
* working directory) gets the declaring app's root instead of the server's
274+
* current directory.
275+
*/
276+
export function createPackagePathResolver(packageRoot?: string): (relativePath: string) => Promise<string> {
277+
return (relativePath: string) => resolveInsidePackageRoot(packageRoot, relativePath, 'paths');
278+
}
279+
238280
/**
239281
* Resolves a declarative connector `credentialRef` to its plaintext secret, or
240282
* `undefined`/empty when unknown (which the materializer turns into a hard boot
@@ -1975,6 +2017,9 @@ export class AutomationServicePlugin implements Plugin {
19752017
// openapi's `providerConfig.spec: './billing-openapi.json'`),
19762018
// confined to the stack/package root.
19772019
loadPackageFile: createPackageFileLoader(this.options.packageRoot),
2020+
// #22434 — the same root as a location (e.g. a launched
2021+
// process's working directory), under the same confinement.
2022+
resolvePackagePath: createPackagePathResolver(this.options.packageRoot),
19782023
};
19792024

19802025
let materialization;

‎packages/spec/src/integration/connector-provider.test.ts‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@
55
// credentialRef-based instance auth (no inline secrets), and the authoring rules
66
// enforced by DeclarativeConnectorEntrySchema.
77

8-
import { describe, it, expect } from 'vitest';
8+
import { describe, it, expect, expectTypeOf } from 'vitest';
99
import {
1010
ConnectorSchema,
1111
DeclarativeConnectorEntrySchema,
1212
ConnectorInstanceAuthSchema,
1313
} from './connector.zod';
14+
import type { ConnectorProviderContext } from './connector-provider';
1415

1516
describe('ADR-0097 connector schema evolution', () => {
1617
describe('ConnectorSchema — new fields', () => {
@@ -167,3 +168,53 @@ describe('ADR-0097 connector schema evolution', () => {
167168
});
168169
});
169170
});
171+
172+
// ---------------------------------------------------------------------------
173+
// [#22434] `ConnectorProviderContext.resolvePackagePath` — the host-provided
174+
// package anchor, beside `loadPackageFile`.
175+
//
176+
// The pins here hold what the card decided at the CONTRACT layer: the member is
177+
// OPTIONAL — a host that does not hand it, and every context built before it
178+
// existed, still conforms, so its absence leaves a factory's behaviour as it
179+
// was — and it has exactly the signature of its sibling `loadPackageFile`. The
180+
// behaviour pins (resolves against the package root, `'.'` is the root, an
181+
// escape is refused) live with the host that implements it:
182+
// `packages/services/service-automation/src/connector-materialization.test.ts`.
183+
//
184+
// The optionality pin is a type alias, not `expectTypeOf`: `toEqualTypeOf<…
185+
// | undefined>` cannot tell an optional member from a REQUIRED one typed
186+
// `… | undefined`, while `{} extends Pick<…>` can. Exported for TS6196;
187+
// `check:test-typecheck` compiles this file.
188+
// ---------------------------------------------------------------------------
189+
190+
type Eq<A, B> = (<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
191+
type Assert<T extends true> = T;
192+
193+
/** Optional: a host may omit it. A required member turns this alias red. */
194+
export type ResolvePackagePathIsOptional = Assert<
195+
{} extends Pick<ConnectorProviderContext, 'resolvePackagePath'> ? true : false
196+
>;
197+
198+
/** One member family: the same `(relativePath) => Promise<string>` shape as the loader. */
199+
export type ResolvePackagePathMatchesLoader = Assert<
200+
Eq<ConnectorProviderContext['resolvePackagePath'], ConnectorProviderContext['loadPackageFile']>
201+
>;
202+
203+
describe('ConnectorProviderContext.resolvePackagePath — the host-provided package anchor', () => {
204+
it('is optional: a context without it still conforms, and a factory can tell it is absent', async () => {
205+
expectTypeOf<ConnectorProviderContext['resolvePackagePath']>().toEqualTypeOf<
206+
((relativePath: string) => Promise<string>) | undefined
207+
>();
208+
209+
// The shape every host built before this member existed. This literal
210+
// failing to compile is the regression.
211+
const withoutAnchor: ConnectorProviderContext = { name: 'tools', label: 'Tools', type: 'api', providerConfig: {} };
212+
expect(withoutAnchor.resolvePackagePath).toBeUndefined();
213+
214+
const withAnchor: ConnectorProviderContext = {
215+
...withoutAnchor,
216+
resolvePackagePath: async (relativePath) => `/srv/app/${relativePath}`,
217+
};
218+
await expect(withAnchor.resolvePackagePath?.('scripts')).resolves.toBe('/srv/app/scripts');
219+
});
220+
});

0 commit comments

Comments
 (0)