History

Default addSpec credentialTargetScope to the source's scope (#747)

config-sync calls openapi.addSpec without ever passing credentialTargetScope. addSpecInternal forwarded the field unchanged to rebuildSource, where targetScopeForBinding rejected the missing field with "credentialTargetScope is required when adding direct OpenAPI credentials" — so any source with a header secret in executor.jsonc broke the daemon at startup. Fall back to config.scope, matching what refreshSource and editSource already do (refreshSource hardcodes scope; editSource uses input.credentialTargetScope ?? scope). Add a regression test that exercises addSpec with header secrets but no credentialTargetScope — the test fails on main and passes after the fix.

Rhys Sullivan committed 6 months ago

Commit 3afa88a1 parent baf3bd42 files · +56 −1Browse files

Unified · Split

2 files
  1. packages/plugins/openapi/src/sdk/plugin.test.ts +51 −0
  2. packages/plugins/openapi/src/sdk/plugin.ts +5 −1
packages/plugins/openapi/src/sdk/plugin.test.ts+51−0
@@ -549,6 +549,57 @@ layer(TestLayer)("OpenAPI Plugin", (it) => { · 548 unmodified lines
549549 }),
550550 );
551551
552+ it.effect("addSpec without credentialTargetScope defaults to the source's scope", () =>
553+ // Regression: config-sync calls addSpec without ever setting
554+ // credentialTargetScope. Before the fix, any source with a
555+ // header secret in executor.jsonc errored with
556+ // "credentialTargetScope is required when adding direct OpenAPI
557+ // credentials" the moment the daemon started.
558+ Effect.gen(function* () {
559+ const httpClient = yield* HttpClient.HttpClient;
560+ const clientLayer = Layer.succeed(HttpClient.HttpClient, httpClient);
561+
562+ const executor = yield* createExecutor(
563+ makeTestConfig({
564+ plugins: [
565+ openApiPlugin({ httpClientLayer: clientLayer }),
566+ memorySecretsPlugin(),
567+ ] as const,
568+ }),
569+ );
570+
571+ yield* executor.secrets.set(
572+ new SetSecretInput({
573+ id: SecretId.make("config-sync-token"),
574+ scope: ScopeId.make(TEST_SCOPE),
575+ name: "Config-sync token",
576+ value: "secret-from-jsonc",
577+ }),
578+ );
579+
580+ yield* executor.openapi.addSpec({
581+ spec: specJson,
582+ scope: TEST_SCOPE,
583+ namespace: "default_target_scope",
584+ baseUrl: "",
585+ headers: {
586+ Authorization: { secretId: "config-sync-token", prefix: "Bearer " },
587+ },
588+ });
589+
590+ const bindings = yield* executor.openapi.listSourceBindings(
591+ "default_target_scope",
592+ TEST_SCOPE,
593+ );
594+ expect(bindings).toHaveLength(1);
595+ expect(bindings[0]).toMatchObject({
596+ scopeId: ScopeId.make(TEST_SCOPE),
597+ slot: "header:authorization",
598+ value: { kind: "secret", secretId: SecretId.make("config-sync-token") },
599+ });
600+ }),
601+ );
602+
552603 it.effect("fails clearly when a secret is missing", () =>
553604 Effect.gen(function* () {
554605 const httpClient = yield* HttpClient.HttpClient;
packages/plugins/openapi/src/sdk/plugin.ts+5−1
@@ -1004,7 +1004,11 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { · 1003 unmodified lines
10041004 queryParams: config.queryParams,
10051005 specFetchCredentials: config.specFetchCredentials,
10061006 oauth2: config.oauth2,
1007− credentialTargetScope: config.credentialTargetScope,
1007+ // Default to the source's own scope. refreshSource and editSource
1008+ // do the same; without this, config-sync's addSpec — which never
1009+ // passes the field — fails with "credentialTargetScope is
1010+ // required" the moment the jsonc declares any header secret.
1011+ credentialTargetScope: config.credentialTargetScope ?? config.scope,
10081012 });
10091013 });
10101014

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t