From fc033fbc8cd38a84cde48993f50c3329210ca8e7 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 21 Sep 2026 15:46:53 +0000 Subject: [PATCH 1/2] fix(eval): accept a full ARN for config-bundle / gateway / online-eval ids `project status` prints ARNs, but the eval config-bundle, gateway, and online-eval commands sent the id straight into the request path. An ARN's slashes make the service parse the path as an unknown operation and return a misleading AccessDenied instead of a validation error. Normalize these ids with the existing serviceIdFromArn (ARN -> bare id, non-ARN passes through) at the core-method boundary: config-bundle get/update/delete/ version-list, online-eval get/update/set-status/delete, and the gateway lookup in createABTest. --- src/core/configBundle.test.ts | 33 +++++++++++++++++++++++++++++++++ src/core/eval.tsx | 23 +++++++++++++++++++---- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/core/configBundle.test.ts b/src/core/configBundle.test.ts index 371e81523e..6d17b823ae 100644 --- a/src/core/configBundle.test.ts +++ b/src/core/configBundle.test.ts @@ -1,7 +1,9 @@ import { describe, expect, test } from "bun:test"; import { + DeleteConfigurationBundleCommand, GetConfigurationBundleCommand, GetConfigurationBundleVersionCommand, + ListConfigurationBundleVersionsCommand, UpdateConfigurationBundleCommand, type BedrockAgentCoreControlClient, } from "@aws-sdk/client-bedrock-agentcore-control"; @@ -130,4 +132,35 @@ describe("EvalClient configuration bundles", () => { branchName: "mainline", }); }); + + test("accepts a full ARN and sends the bare bundle id to every command", async () => { + // `project status` prints ARNs; passing one to --id must not reach the service + // as a path segment (its slashes break path→operation parsing → misleading + // AccessDenied). The bare id is extracted before the request. + const arn = "arn:aws:bedrock-agentcore:us-west-2:123456789012:configuration-bundle/b-1"; + const sent: unknown[] = []; + const { client } = subject(async (command) => { + sent.push(command); + // update() first reads the current version, so hand back a versionId. + return { versionId: "v-9" }; + }); + + await client.getConfigurationBundle(arn, undefined, "mainline", OPTIONS); + await client.listConfigurationBundleVersions(arn, undefined, undefined, OPTIONS); + await client.deleteConfigurationBundle(arn, OPTIONS); + await client.updateConfigurationBundle( + arn, + { branchName: "mainline", components: {}, commitMessage: "update" }, + OPTIONS, + ); + + expect((sent[0] as GetConfigurationBundleCommand).input).toMatchObject({ bundleId: "b-1" }); + expect((sent[1] as ListConfigurationBundleVersionsCommand).input).toMatchObject({ + bundleId: "b-1", + }); + expect((sent[2] as DeleteConfigurationBundleCommand).input).toEqual({ bundleId: "b-1" }); + // update() reads then writes: both carry the bare id. + expect((sent[3] as GetConfigurationBundleCommand).input).toMatchObject({ bundleId: "b-1" }); + expect((sent[4] as UpdateConfigurationBundleCommand).input).toMatchObject({ bundleId: "b-1" }); + }); }); diff --git a/src/core/eval.tsx b/src/core/eval.tsx index 4e2a061e15..c277bfa85e 100644 --- a/src/core/eval.tsx +++ b/src/core/eval.tsx @@ -168,6 +168,7 @@ import type { AwsClients, CoreFetch, CoreOptions } from "./types"; import type { Logger } from "../logging"; import { FilteredPaginator } from "./filteredPaginator"; import { toClientConfig } from "./utils"; +import { serviceIdFromArn } from "./arn"; import { grantOnlineEvalScope } from "./onlineEvalExecutionRole"; import { accountIdFromArn, deleteAbTestRole, provisionAbTestRole } from "./abTestExecutionRole"; import { harnessRuntimeFromResponse } from "./harness"; @@ -520,6 +521,7 @@ export class EvalClient implements CoreEvalClient { }) => CreateABTestRequest, options: CoreOptions, ): Promise { + gateway = serviceIdFromArn(gateway); const control = this.clients.control(toClientConfig(options)); const gatewayArn = (await control.send(new GetGatewayCommand({ gatewayIdentifier: gateway }))) .gatewayArn!; @@ -1189,6 +1191,7 @@ export class EvalClient implements CoreEvalClient { update: UpdateOnlineEvalInput, options: CoreOptions, ): Promise<{ response: UpdateOnlineEvaluationConfigResponse }> { + id = serviceIdFromArn(id); const control = this.clients.control(toClientConfig(options)); const current = await control.send( new GetOnlineEvaluationConfigCommand({ @@ -1261,6 +1264,7 @@ export class EvalClient implements CoreEvalClient { id: string, options: CoreOptions, ): Promise { + id = serviceIdFromArn(id); return this.clients .control(toClientConfig(options)) .send(new GetOnlineEvaluationConfigCommand({ onlineEvaluationConfigId: id })); @@ -1281,6 +1285,7 @@ export class EvalClient implements CoreEvalClient { executionStatus: "ENABLED" | "DISABLED", options: CoreOptions, ): Promise { + id = serviceIdFromArn(id); return this.clients .control(toClientConfig(options)) .send( @@ -1292,6 +1297,7 @@ export class EvalClient implements CoreEvalClient { id: string, options: CoreOptions, ): Promise { + id = serviceIdFromArn(id); return this.clients .control(toClientConfig(options)) .send(new DeleteOnlineEvaluationConfigCommand({ onlineEvaluationConfigId: id })); @@ -1312,6 +1318,10 @@ export class EvalClient implements CoreEvalClient { branchName: string, options: CoreOptions, ): Promise { + // Accept a full ARN (as `project status` prints) and use its bare id: the id + // is a path segment, and an ARN's slashes would make the service parse the + // path as an unknown operation and return a misleading AccessDenied. + id = serviceIdFromArn(id); const control = this.clients.control(toClientConfig(options)); return version === undefined ? control.send(new GetConfigurationBundleCommand({ bundleId: id, branchName })) @@ -1335,6 +1345,7 @@ export class EvalClient implements CoreEvalClient { update: UpdateConfigurationBundleInput, options: CoreOptions, ): Promise { + id = serviceIdFromArn(id); const control = this.clients.control(toClientConfig(options)); const current = await control.send( new GetConfigurationBundleCommand({ bundleId: id, branchName: update.branchName }), @@ -1364,7 +1375,7 @@ export class EvalClient implements CoreEvalClient { ): Promise { return this.clients .control(toClientConfig(options)) - .send(new DeleteConfigurationBundleCommand({ bundleId: id })); + .send(new DeleteConfigurationBundleCommand({ bundleId: serviceIdFromArn(id) })); } async listConfigurationBundleVersions( @@ -1373,9 +1384,13 @@ export class EvalClient implements CoreEvalClient { maxResults: number | undefined, options: CoreOptions, ): Promise { - return this.clients - .control(toClientConfig(options)) - .send(new ListConfigurationBundleVersionsCommand({ bundleId: id, nextToken, maxResults })); + return this.clients.control(toClientConfig(options)).send( + new ListConfigurationBundleVersionsCommand({ + bundleId: serviceIdFromArn(id), + nextToken, + maxResults, + }), + ); } async createDataset( From 17eae89eca38e8dff4614cc507b9607c16fdf7ee Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 21 Sep 2026 18:18:41 +0000 Subject: [PATCH 2/2] Reject an ARN instead of extracting the bare id Per review: these ids should be bare, not ARNs. Instead of silently extracting the id from an ARN, reject it with an actionable InputValidationError naming the option, so the misleading AccessDenied is replaced by a clear "pass the bare id" message. Config-bundle, online-eval, and the createABTest gateway are guarded. --- src/core/configBundle.test.ts | 46 ++++++++++++++++++----------------- src/core/eval.tsx | 33 +++++++++++++++++-------- 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/core/configBundle.test.ts b/src/core/configBundle.test.ts index 6d17b823ae..4c10216431 100644 --- a/src/core/configBundle.test.ts +++ b/src/core/configBundle.test.ts @@ -1,13 +1,11 @@ import { describe, expect, test } from "bun:test"; import { - DeleteConfigurationBundleCommand, GetConfigurationBundleCommand, GetConfigurationBundleVersionCommand, - ListConfigurationBundleVersionsCommand, UpdateConfigurationBundleCommand, type BedrockAgentCoreControlClient, } from "@aws-sdk/client-bedrock-agentcore-control"; -import { NetworkingError } from "../errors"; +import { InputValidationError, NetworkingError } from "../errors"; import { EvalClient } from "./eval"; import type { AwsClients, ClientConfig } from "./types"; @@ -133,34 +131,38 @@ describe("EvalClient configuration bundles", () => { }); }); - test("accepts a full ARN and sends the bare bundle id to every command", async () => { - // `project status` prints ARNs; passing one to --id must not reach the service - // as a path segment (its slashes break path→operation parsing → misleading - // AccessDenied). The bare id is extracted before the request. + test("rejects a full ARN, telling the caller to pass the bare id", async () => { + // `project status` prints bare ids; a full ARN in --id would reach the service + // as a path segment whose slashes break path→operation parsing (a misleading + // AccessDenied), so it is rejected up front, before any request. const arn = "arn:aws:bedrock-agentcore:us-west-2:123456789012:configuration-bundle/b-1"; const sent: unknown[] = []; const { client } = subject(async (command) => { sent.push(command); - // update() first reads the current version, so hand back a versionId. return { versionId: "v-9" }; }); - await client.getConfigurationBundle(arn, undefined, "mainline", OPTIONS); - await client.listConfigurationBundleVersions(arn, undefined, undefined, OPTIONS); - await client.deleteConfigurationBundle(arn, OPTIONS); - await client.updateConfigurationBundle( - arn, - { branchName: "mainline", components: {}, commitMessage: "update" }, - OPTIONS, + const rejects = /must be a bare resource id, not an ARN/; + await expect( + client.getConfigurationBundle(arn, undefined, "mainline", OPTIONS), + ).rejects.toThrow(rejects); + await expect( + client.listConfigurationBundleVersions(arn, undefined, undefined, OPTIONS), + ).rejects.toThrow(rejects); + await expect(client.deleteConfigurationBundle(arn, OPTIONS)).rejects.toBeInstanceOf( + InputValidationError, ); + await expect( + client.updateConfigurationBundle( + arn, + { branchName: "mainline", components: {}, commitMessage: "update" }, + OPTIONS, + ), + ).rejects.toThrow(rejects); + // Rejected before any SDK call; a bare id still works. + expect(sent).toEqual([]); + await client.getConfigurationBundle("b-1", undefined, "mainline", OPTIONS); expect((sent[0] as GetConfigurationBundleCommand).input).toMatchObject({ bundleId: "b-1" }); - expect((sent[1] as ListConfigurationBundleVersionsCommand).input).toMatchObject({ - bundleId: "b-1", - }); - expect((sent[2] as DeleteConfigurationBundleCommand).input).toEqual({ bundleId: "b-1" }); - // update() reads then writes: both carry the bare id. - expect((sent[3] as GetConfigurationBundleCommand).input).toMatchObject({ bundleId: "b-1" }); - expect((sent[4] as UpdateConfigurationBundleCommand).input).toMatchObject({ bundleId: "b-1" }); }); }); diff --git a/src/core/eval.tsx b/src/core/eval.tsx index c277bfa85e..4838144bcb 100644 --- a/src/core/eval.tsx +++ b/src/core/eval.tsx @@ -168,7 +168,7 @@ import type { AwsClients, CoreFetch, CoreOptions } from "./types"; import type { Logger } from "../logging"; import { FilteredPaginator } from "./filteredPaginator"; import { toClientConfig } from "./utils"; -import { serviceIdFromArn } from "./arn"; +import { parseArn } from "./arn"; import { grantOnlineEvalScope } from "./onlineEvalExecutionRole"; import { accountIdFromArn, deleteAbTestRole, provisionAbTestRole } from "./abTestExecutionRole"; import { harnessRuntimeFromResponse } from "./harness"; @@ -189,6 +189,19 @@ const RETRYABLE_DATASET_STATUSES: ReadonlySet = new Set(["CREATIN // The shared, account-level OTel span log group. const SPANS_LOG_GROUP = "aws/spans"; +// These commands address a resource by its bare id, which the service places in +// the request path. A full ARN's slashes make the service misparse the path and +// return a misleading AccessDenied, so reject it up front with an actionable +// message rather than sending it. `project status` also prints bare ids for this. +function requireBareId(value: string, option: string): string { + if (parseArn(value) !== undefined) { + throw new InputValidationError( + `--${option} must be a bare resource id, not an ARN (pass the id, e.g. the segment after the last '/').`, + ); + } + return value; +} + // Default discovery window when no explicit --start/--end or --lookback-days is // given. Mirrors the batch service's now-7d default. const SEVEN_DAYS_MS = 7 * 24 * 60 * 60 * 1000; @@ -521,7 +534,7 @@ export class EvalClient implements CoreEvalClient { }) => CreateABTestRequest, options: CoreOptions, ): Promise { - gateway = serviceIdFromArn(gateway); + gateway = requireBareId(gateway, "gateway"); const control = this.clients.control(toClientConfig(options)); const gatewayArn = (await control.send(new GetGatewayCommand({ gatewayIdentifier: gateway }))) .gatewayArn!; @@ -1191,7 +1204,7 @@ export class EvalClient implements CoreEvalClient { update: UpdateOnlineEvalInput, options: CoreOptions, ): Promise<{ response: UpdateOnlineEvaluationConfigResponse }> { - id = serviceIdFromArn(id); + id = requireBareId(id, "id"); const control = this.clients.control(toClientConfig(options)); const current = await control.send( new GetOnlineEvaluationConfigCommand({ @@ -1264,7 +1277,7 @@ export class EvalClient implements CoreEvalClient { id: string, options: CoreOptions, ): Promise { - id = serviceIdFromArn(id); + id = requireBareId(id, "id"); return this.clients .control(toClientConfig(options)) .send(new GetOnlineEvaluationConfigCommand({ onlineEvaluationConfigId: id })); @@ -1285,7 +1298,7 @@ export class EvalClient implements CoreEvalClient { executionStatus: "ENABLED" | "DISABLED", options: CoreOptions, ): Promise { - id = serviceIdFromArn(id); + id = requireBareId(id, "id"); return this.clients .control(toClientConfig(options)) .send( @@ -1297,7 +1310,7 @@ export class EvalClient implements CoreEvalClient { id: string, options: CoreOptions, ): Promise { - id = serviceIdFromArn(id); + id = requireBareId(id, "id"); return this.clients .control(toClientConfig(options)) .send(new DeleteOnlineEvaluationConfigCommand({ onlineEvaluationConfigId: id })); @@ -1321,7 +1334,7 @@ export class EvalClient implements CoreEvalClient { // Accept a full ARN (as `project status` prints) and use its bare id: the id // is a path segment, and an ARN's slashes would make the service parse the // path as an unknown operation and return a misleading AccessDenied. - id = serviceIdFromArn(id); + id = requireBareId(id, "id"); const control = this.clients.control(toClientConfig(options)); return version === undefined ? control.send(new GetConfigurationBundleCommand({ bundleId: id, branchName })) @@ -1345,7 +1358,7 @@ export class EvalClient implements CoreEvalClient { update: UpdateConfigurationBundleInput, options: CoreOptions, ): Promise { - id = serviceIdFromArn(id); + id = requireBareId(id, "id"); const control = this.clients.control(toClientConfig(options)); const current = await control.send( new GetConfigurationBundleCommand({ bundleId: id, branchName: update.branchName }), @@ -1375,7 +1388,7 @@ export class EvalClient implements CoreEvalClient { ): Promise { return this.clients .control(toClientConfig(options)) - .send(new DeleteConfigurationBundleCommand({ bundleId: serviceIdFromArn(id) })); + .send(new DeleteConfigurationBundleCommand({ bundleId: requireBareId(id, "id") })); } async listConfigurationBundleVersions( @@ -1386,7 +1399,7 @@ export class EvalClient implements CoreEvalClient { ): Promise { return this.clients.control(toClientConfig(options)).send( new ListConfigurationBundleVersionsCommand({ - bundleId: serviceIdFromArn(id), + bundleId: requireBareId(id, "id"), nextToken, maxResults, }),