From 2fce262fbfddd2921aa5e919f26af7bdfdd9c02c Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Wed, 19 Aug 2026 16:11:03 -0400 Subject: [PATCH 1/9] feat(project): reshape credential schemas for project add credentials OAuth credentials now model the two authoring paths: guided custom OAuth (clientId, discoveryUrl, scopes) and vendored providers (free-form vendor + complete providerConfig). providerConfig rejects nested secret material so secrets can only travel via .env.local or Secrets Manager references, which both credential types record as secretRef/clientSecretRef. The unused usage field is dropped. This shape is the shared contract with @aws/agentcore-cdk (mirror change lands on that repo's refactor branch). --- src/projectSchemas/credential.test.ts | 132 ++++++++++++++++++++++++++ src/projectSchemas/credential.ts | 86 +++++++++++++++-- 2 files changed, 209 insertions(+), 9 deletions(-) create mode 100644 src/projectSchemas/credential.test.ts diff --git a/src/projectSchemas/credential.test.ts b/src/projectSchemas/credential.test.ts new file mode 100644 index 000000000..0de8b4f37 --- /dev/null +++ b/src/projectSchemas/credential.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from "bun:test"; +import { CredentialSchema } from "./credential"; + +const DISCOVERY_URL = "https://idp.example.com/.well-known/openid-configuration"; +const SECRET_REF = { + secretId: "arn:aws:secretsmanager:us-west-2:123456789012:secret:s", + jsonKey: "value", +}; + +describe("credential schema", () => { + it.each<[string, Record]>([ + ["an api-key credential", { authorizerType: "ApiKeyCredentialProvider", name: "svc-key" }], + [ + "an api-key credential with an external secret reference", + { authorizerType: "ApiKeyCredentialProvider", name: "svc-key", secretRef: SECRET_REF }, + ], + [ + "a guided custom OAuth credential", + { + authorizerType: "OAuthCredentialProvider", + name: "idp", + vendor: "CustomOauth2", + clientId: "client-1", + discoveryUrl: DISCOVERY_URL, + scopes: ["openid"], + managed: true, + }, + ], + [ + "a custom OAuth credential with a complete provider config", + { + authorizerType: "OAuthCredentialProvider", + name: "idp", + vendor: "CustomOauth2", + providerConfig: { + customOauth2ProviderConfig: { + clientId: "client-1", + oauthDiscovery: { discoveryUrl: DISCOVERY_URL }, + }, + }, + }, + ], + [ + "a vendored OAuth credential with an external client secret reference", + { + authorizerType: "OAuthCredentialProvider", + name: "github", + vendor: "GithubOauth2", + providerConfig: { githubOauth2ProviderConfig: { clientId: "client-1" } }, + clientSecretRef: SECRET_REF, + }, + ], + ])("accepts %s and retains its fields", (_label, value) => { + const result = CredentialSchema.safeParse(value); + expect(result.success).toBe(true); + expect(result.data).toMatchObject(value); + }); + + it("defaults the OAuth vendor to CustomOauth2", () => { + const result = CredentialSchema.safeParse({ + authorizerType: "OAuthCredentialProvider", + name: "idp", + clientId: "client-1", + discoveryUrl: DISCOVERY_URL, + }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ vendor: "CustomOauth2" }); + }); + + it.each<[string, Record, RegExp]>([ + [ + "a vendored OAuth credential without a provider config", + { authorizerType: "OAuthCredentialProvider", name: "github", vendor: "GithubOauth2" }, + /providerConfig/, + ], + [ + "a guided custom OAuth credential without a discovery URL", + { authorizerType: "OAuthCredentialProvider", name: "idp", clientId: "client-1" }, + /discoveryUrl/, + ], + [ + "a provider config combined with guided fields", + { + authorizerType: "OAuthCredentialProvider", + name: "idp", + discoveryUrl: DISCOVERY_URL, + providerConfig: { customOauth2ProviderConfig: {} }, + }, + /mutually exclusive/, + ], + [ + "a provider config with a nested clientSecret", + { + authorizerType: "OAuthCredentialProvider", + name: "github", + vendor: "GithubOauth2", + providerConfig: { + githubOauth2ProviderConfig: { clientId: "client-1", clientSecret: "sssh" }, + }, + }, + /secret material.*clientSecret/, + ], + [ + "a provider config with a nested apiKey", + { + authorizerType: "OAuthCredentialProvider", + name: "github", + vendor: "GithubOauth2", + providerConfig: { githubOauth2ProviderConfig: { nested: { apiKey: "sssh" } } }, + }, + /secret material.*apiKey/, + ], + [ + "a secret reference with unexpected fields", + { + authorizerType: "ApiKeyCredentialProvider", + name: "svc-key", + secretRef: { ...SECRET_REF, extra: "bad" }, + }, + /secretRef/, + ], + [ + "an invalid credential name", + { authorizerType: "ApiKeyCredentialProvider", name: "bad name!" }, + /alphanumeric/, + ], + ])("rejects %s", (_label, value, message) => { + const result = CredentialSchema.safeParse(value); + expect(result.success).toBe(false); + expect(JSON.stringify(result.error?.issues)).toMatch(message); + }); +}); diff --git a/src/projectSchemas/credential.ts b/src/projectSchemas/credential.ts index d4bce69a3..ba8ef771e 100644 --- a/src/projectSchemas/credential.ts +++ b/src/projectSchemas/credential.ts @@ -14,20 +14,88 @@ export const CredentialTypeSchema = z.enum([ "PaymentCredentialProvider", ]); export type CredentialType = z.infer; +/** A reference to a secret the customer already keeps in AWS Secrets Manager. */ +export const SecretReferenceSchema = z + .object({ + secretId: z.string().min(1), + jsonKey: z.string().min(1), + }) + .strict(); +export type SecretReference = z.infer; export const ApiKeyCredentialSchema = z.object({ authorizerType: z.literal("ApiKeyCredentialProvider"), name: CredentialNameSchema, + /** External Secrets Manager reference; when absent the key comes from .env.local. */ + secretRef: SecretReferenceSchema.optional(), }); export type ApiKeyCredential = z.infer; -export const OAuthCredentialSchema = z.object({ - authorizerType: z.literal("OAuthCredentialProvider"), - name: CredentialNameSchema, - discoveryUrl: z.string().url().optional(), - scopes: z.array(z.string()).optional(), - vendor: z.string().default("CustomOauth2"), - managed: z.boolean().optional(), - usage: z.enum(["inbound", "outbound"]).optional(), -}); +const CUSTOM_OAUTH_VENDOR = "CustomOauth2"; +// Secret values never belong in agentcore.json; they travel via .env.local or +// Secrets Manager references. These key names match the SDK's inline-secret fields. +const SECRET_MATERIAL_KEYS = new Set(["clientSecret", "apiKey"]); +function findSecretMaterialKey(value: unknown): string | undefined { + if (typeof value !== "object" || value === null) return undefined; + for (const [key, nested] of Object.entries(value)) { + if (SECRET_MATERIAL_KEYS.has(key)) return key; + const found = findSecretMaterialKey(nested); + if (found) return found; + } + return undefined; +} +export const OAuthCredentialSchema = z + .object({ + authorizerType: z.literal("OAuthCredentialProvider"), + name: CredentialNameSchema, + /** Credential provider vendor (free-form to track the service without CLI releases). */ + vendor: z.string().default(CUSTOM_OAUTH_VENDOR), + /** Guided custom OAuth fields. */ + clientId: z.string().optional(), + discoveryUrl: z.string().url().optional(), + scopes: z.array(z.string()).optional(), + /** Complete Oauth2ProviderConfigInput (secret-free) for vendored providers. */ + providerConfig: z.record(z.string(), z.unknown()).optional(), + /** External Secrets Manager reference; when absent the secret comes from .env.local. */ + clientSecretRef: SecretReferenceSchema.optional(), + /** Whether this credential was auto-created by the CLI (e.g. for CUSTOM_JWT inbound auth). */ + managed: z.boolean().optional(), + }) + .superRefine((credential, ctx) => { + const hasGuidedFields = + credential.clientId !== undefined || + credential.discoveryUrl !== undefined || + credential.scopes !== undefined; + if (credential.providerConfig !== undefined && hasGuidedFields) { + ctx.addIssue({ + code: "custom", + message: + "providerConfig and the guided fields (clientId, discoveryUrl, scopes) are mutually exclusive", + }); + return; + } + if (credential.providerConfig === undefined) { + if (credential.vendor !== CUSTOM_OAUTH_VENDOR) { + ctx.addIssue({ + code: "custom", + message: `vendor "${credential.vendor}" requires providerConfig; guided fields only support ${CUSTOM_OAUTH_VENDOR}`, + }); + } else if (credential.discoveryUrl === undefined) { + ctx.addIssue({ + code: "custom", + message: `guided ${CUSTOM_OAUTH_VENDOR} requires discoveryUrl`, + }); + } + return; + } + const secretKey = findSecretMaterialKey(credential.providerConfig); + if (secretKey) { + ctx.addIssue({ + code: "custom", + message: + `providerConfig must not contain secret material (found "${secretKey}"). ` + + "Provide secrets via --client-secret, a secret reference, or .env.local.", + }); + } + }); export type OAuthCredential = z.infer; export const PaymentCredentialSchema = z.object({ authorizerType: z.literal("PaymentCredentialProvider"), From 10d9499391dc4281aea01dda6dd2cefc815af673 Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Wed, 19 Aug 2026 16:23:07 -0400 Subject: [PATCH 2/9] feat(project): add 'project add credentials' api-key and oauth commands Credentials are the first spec-only resource in FsProjectManager.addResource: no scaffolding, the spec entry lands in agentcore.json and secret values or commented placeholders land in agentcore/.env.local (existing keys are never overwritten, and a missing .gitignore guard for .env.local is restored). Secret flags accept only stdin ('-') or file:// sources; inline values are rejected so keys stay out of shell history. External Secrets Manager references skip .env.local entirely and are recorded in the spec. --- src/core/project/envLocal.ts | 73 +++++ src/core/project/manager.tsx | 40 ++- src/handlers/project/add/credentials/index.ts | 229 ++++++++++++++ src/handlers/project/add/index.ts | 2 + src/handlers/project/project.test.ts | 283 +++++++++++++++++- src/handlers/project/types.ts | 14 + 6 files changed, 633 insertions(+), 8 deletions(-) create mode 100644 src/core/project/envLocal.ts create mode 100644 src/handlers/project/add/credentials/index.ts diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts new file mode 100644 index 000000000..4d8d2d8c0 --- /dev/null +++ b/src/core/project/envLocal.ts @@ -0,0 +1,73 @@ +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import type { EnvLocalEntry } from "../../handlers/project/types"; + +/** The project-relative path of the local secrets file (read by `agentcore dev`). */ +export const ENV_LOCAL_RELATIVE_PATH = join("agentcore", ".env.local"); + +const KEY_LINE = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=/; + +/** + * Appends entries to a .env-format file, creating it when missing. Keys that + * already exist are never overwritten so user-managed values survive re-runs. + */ +export async function upsertEnvLocalEntries( + envPath: string, + entries: EnvLocalEntry[], +): Promise<{ written: string[]; skipped: string[] }> { + const existing = await readOrEmpty(envPath); + const existingKeys = new Set( + existing + .split("\n") + .map((line) => KEY_LINE.exec(line)?.[1]) + .filter((key) => key !== undefined), + ); + + const written: string[] = []; + const skipped: string[] = []; + let content = existing; + for (const entry of entries) { + if (existingKeys.has(entry.key)) { + skipped.push(entry.key); + continue; + } + const separator = content === "" || content.endsWith("\n") ? "" : "\n"; + content += `${separator}# ${entry.comment}\n${entry.key}=${entry.value ?? ""}\n`; + written.push(entry.key); + } + + if (written.length > 0) await writeFile(envPath, content); + return { written, skipped }; +} + +/** + * Makes sure the project's .gitignore keeps .env.local out of version control. + * Returns true when the file was created or amended. + */ +export async function ensureGitignoreCoversEnvLocal(rootPath: string): Promise { + const gitignorePath = join(rootPath, ".gitignore"); + const existing = await readOrEmpty(gitignorePath); + // ponytail: literal-line match, not full gitignore pattern semantics; a + // matcher library is warranted only if projects grow exotic ignore rules. + const covered = existing + .split("\n") + .map((line) => line.trim()) + .some((line) => line === ".env.local" || line === ".env*.local"); + if (covered) return false; + + const separator = existing === "" || existing.endsWith("\n") ? "" : "\n"; + await writeFile( + gitignorePath, + `${existing}${separator}# Local secrets (added by agentcore; never commit)\n.env.local\n`, + ); + return true; +} + +async function readOrEmpty(path: string): Promise { + try { + return await readFile(path, "utf8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return ""; + throw error; + } +} diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 4180cb03c..96271a6ab 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -19,6 +19,11 @@ import { type ReadWriteJson, } from "../../io"; import { defaultSource, type AssetSource } from "./source"; +import { + ENV_LOCAL_RELATIVE_PATH, + ensureGitignoreCoversEnvLocal, + upsertEnvLocalEntries, +} from "./envLocal"; import { createHarnessTreeFromSpec, createProjectTreeFromTemplate, TEMPLATES } from "./templates"; import { ProjectSpecSchema } from "../../projectSchemas/project"; import { enclosingProjectRoot } from "./fsUtils"; @@ -139,7 +144,8 @@ export class FsProjectManager implements ProjectManager { `a ${resourceType} with name '${resourceConfig.name}' already exists`, ); - const newResources = [...existingResources]; + // Widened: arms push their own shapes; the whole-spec safeParse below validates. + const newResources: unknown[] = [...existingResources]; const scaffoldedPaths: string[] = []; switch (resourceType) { @@ -160,6 +166,11 @@ export class FsProjectManager implements ProjectManager { "runtime case not yet implemented in FsProjectManager.addResource", ); } + case "credential": { + // Spec-only resource: no scaffolding, secrets go to .env.local below. + newResources.push(input.resourceConfig); + break; + } // TODO: add limited special casing for runtime and default for other resources that proxy directly to spec changes. } @@ -174,13 +185,9 @@ export class FsProjectManager implements ProjectManager { }); // rollback scaffolding changes on failed config writes to prevent bad state. + let newProjectSpec: z.infer; try { - const newProjectSpec = await this.json.write(agentCoreSpecPath, newSpecParseResult.data); - - return { - ...project, - spec: newProjectSpec, - }; + newProjectSpec = await this.json.write(agentCoreSpecPath, newSpecParseResult.data); } catch (err) { this.logger.warn( `failed to update ${agentCoreSpecPath}; attempting best-effort cleanup of scaffolded files`, @@ -197,6 +204,23 @@ export class FsProjectManager implements ProjectManager { ); throw err; } + + if (input.resourceType === "credential" && input.envEntries && input.envEntries.length > 0) { + const envPath = join(project.rootPath, ENV_LOCAL_RELATIVE_PATH); + yield { message: `Updating secrets file at '${envPath}'` }; + const { skipped } = await upsertEnvLocalEntries(envPath, input.envEntries); + for (const key of skipped) { + yield { message: `'${key}' already exists in ${ENV_LOCAL_RELATIVE_PATH}; left unchanged` }; + } + if (await ensureGitignoreCoversEnvLocal(project.rootPath)) { + yield { message: `Added '.env.local' to .gitignore so secrets stay out of git` }; + } + } + + return { + ...project, + spec: newProjectSpec, + }; } private async scaffoldHarness( @@ -280,5 +304,7 @@ function toProjectSpecKey(resourceType: ProjectResource) { return "harnesses"; case "runtime": return "runtimes"; + case "credential": + return "credentials"; } } diff --git a/src/handlers/project/add/credentials/index.ts b/src/handlers/project/add/credentials/index.ts new file mode 100644 index 000000000..0585d4cc1 --- /dev/null +++ b/src/handlers/project/add/credentials/index.ts @@ -0,0 +1,229 @@ +import z from "zod"; +import type { CredentialProviderVendorType } from "@aws-sdk/client-bedrock-agentcore-control"; +import { createHandler, flag, ProjectKey, Router, type Context } from "../../../../router"; +import { InputValidationError } from "../../../../errors"; +import { SourceResolver, type AppIO } from "../../../../io"; +import { parseSecretReference } from "../../../identity/parser"; +import { + parseProviderConfigFlags, + validateProviderConfigMode, +} from "../../../identity/oauth2-credential-provider/config"; +import type { AddProjectResourceConfig } from "../types"; +import type { AddResourceInput, EnvLocalEntry } from "../../types"; + +export function createAddCredentialsHandler(config: AddProjectResourceConfig): Router { + const credentials = new Router( + "credentials", + "add AgentCore Identity credential providers to the current project", + ); + credentials.handler(createAddApiKeyCredentialHandler(config)); + credentials.handler(createAddOauthCredentialHandler(config)); + return credentials; +} + +/** Derives the .env.local variable name a credential's secret is stored under. */ +function credentialEnvVarName(credentialName: string, suffix = ""): string { + return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; +} + +/** + * Resolves a secret flag while refusing inline values: an inline secret leaks + * into shell history and process listings, so only stdin and files are allowed. + * A single trailing newline is stripped (echo and editors add one). + */ +async function resolveSecretFlag( + resolver: SourceResolver, + name: string, + source: string | undefined, +): Promise { + if (source !== undefined && source !== "-" && !source.startsWith("file://")) { + throw new InputValidationError( + `--${name} must come from stdin ('-') or a file ('file://'); ` + + "inline secret values are not accepted", + ); + } + const value = await resolver.resolveText(name, source); + if (value === undefined) return undefined; + const normalized = value.replace(/\r?\n$/, ""); + if (normalized.includes("\n")) { + throw new InputValidationError(`--${name} must be a single-line value`); + } + return normalized; +} + +const createAddApiKeyCredentialHandler = (config: AddProjectResourceConfig) => + createHandler({ + name: "api-key", + description: "add an API key credential provider to the current project", + flags: [ + flag("name", "the name of the credential provider", z.string().optional()), + flag( + "api-key", + "the API key (file://path or - for stdin; inline values are rejected)", + z.string().optional(), + { sensitive: true }, + ), + flag( + "api-key-secret-reference", + 'external secret reference JSON: {"secretId":"","jsonKey":""}', + z.string().optional(), + ), + ], + handle: async (ctx, flags) => { + if (!flags.name) + throw new InputValidationError("required option '--name ' not specified"); + + const secretRef = flags["api-key-secret-reference"] + ? parseSecretReference("api-key-secret-reference", flags["api-key-secret-reference"]) + : undefined; + if (secretRef && flags["api-key"] !== undefined) { + throw new InputValidationError( + "--api-key and --api-key-secret-reference are mutually exclusive", + ); + } + + const resolver = new SourceResolver({ stdin: config.io.stdin }); + const apiKey = await resolveSecretFlag(resolver, "api-key", flags["api-key"]); + + const envEntries: EnvLocalEntry[] = secretRef + ? [] + : [ + { + key: credentialEnvVarName(flags.name), + value: apiKey, + comment: `API key for credential provider '${flags.name}' (set before deploy)`, + }, + ]; + + await addCredentialToProject(ctx, config, { + resourceConfig: { authorizerType: "ApiKeyCredentialProvider", name: flags.name, secretRef }, + envEntries, + }); + }, + }); + +const createAddOauthCredentialHandler = (config: AddProjectResourceConfig) => + createHandler({ + name: "oauth", + description: "add an OAuth2 credential provider to the current project", + flags: [ + flag("name", "the name of the credential provider", z.string().optional()), + flag( + "vendor", + "the OAuth2 vendor (e.g. GithubOauth2); custom providers use the guided flags instead", + z.string().default("CustomOauth2"), + ), + flag("client-id", "OAuth2 client ID (guided custom OAuth2)", z.string().optional()), + flag("discovery-url", "OAuth2 discovery URL (guided custom OAuth2)", z.string().optional()), + flag( + "scopes", + "OAuth2 scopes the provider grants (guided custom OAuth2)", + z.array(z.string()).optional(), + ), + flag( + "provider-configuration", + "complete secret-free Oauth2ProviderConfigInput JSON (required for vendored providers)", + z.string().optional(), + ), + flag( + "client-secret", + "the client secret (file://path or - for stdin; inline values are rejected)", + z.string().optional(), + { sensitive: true }, + ), + flag( + "client-secret-reference", + 'external secret reference JSON: {"secretId":"","jsonKey":""}', + z.string().optional(), + ), + ], + handle: async (ctx, flags) => { + if (!flags.name) + throw new InputValidationError("required option '--name ' not specified"); + + const secretRef = flags["client-secret-reference"] + ? parseSecretReference("client-secret-reference", flags["client-secret-reference"]) + : undefined; + if (secretRef && flags["client-secret"] !== undefined) { + throw new InputValidationError( + "--client-secret and --client-secret-reference are mutually exclusive", + ); + } + + const mode = parseProviderConfigFlags({ + clientId: flags["client-id"], + discoveryUrl: flags["discovery-url"], + providerConfiguration: flags["provider-configuration"], + }); + validateProviderConfigMode(mode, flags.vendor as CredentialProviderVendorType); + if (mode.kind === "complete" && flags.scopes !== undefined) { + throw new InputValidationError( + "--provider-configuration and --scopes are mutually exclusive", + ); + } + + const resolver = new SourceResolver({ stdin: config.io.stdin }); + const clientSecret = await resolveSecretFlag( + resolver, + "client-secret", + flags["client-secret"], + ); + + const resourceConfig = + mode.kind === "complete" + ? { + authorizerType: "OAuthCredentialProvider" as const, + name: flags.name, + vendor: flags.vendor, + providerConfig: mode.config, + clientSecretRef: secretRef, + } + : { + authorizerType: "OAuthCredentialProvider" as const, + name: flags.name, + vendor: flags.vendor, + clientId: flags["client-id"], + discoveryUrl: flags["discovery-url"], + scopes: flags.scopes, + clientSecretRef: secretRef, + }; + + const envEntries: EnvLocalEntry[] = secretRef + ? [] + : [ + { + key: credentialEnvVarName(flags.name, "_CLIENT_SECRET"), + value: clientSecret, + comment: `OAuth client secret for credential provider '${flags.name}' (set before deploy)`, + }, + ]; + + await addCredentialToProject(ctx, config, { resourceConfig, envEntries }); + }, + }); + +/** Runs the shared add flow: spec update, env entries, progress, and fill-before-deploy notice. */ +async function addCredentialToProject( + ctx: Context, + config: AddProjectResourceConfig, + input: Omit, "resourceType">, +): Promise { + const project = ctx.require(ProjectKey); + for await (const event of config.projectManager.addResource(project, { + resourceType: "credential", + ...input, + })) { + config.io.stderr.write(`${event.message}\n`); + } + + config.io.stderr.write(`added credential '${input.resourceConfig.name}' to '${project.name}'\n`); + notifyPlaceholders(config.io, input.envEntries ?? []); +} + +/** Tells the user which .env.local keys still need a value before deploy. */ +function notifyPlaceholders(io: AppIO, envEntries: EnvLocalEntry[]): void { + const placeholders = envEntries.filter((entry) => entry.value === undefined); + for (const entry of placeholders) { + io.stderr.write(`Set ${entry.key} in agentcore/.env.local before you deploy.\n`); + } +} diff --git a/src/handlers/project/add/index.ts b/src/handlers/project/add/index.ts index 8545ccba3..ec7383075 100644 --- a/src/handlers/project/add/index.ts +++ b/src/handlers/project/add/index.ts @@ -1,5 +1,6 @@ import { withProject } from "../../../middleware/"; import { Router } from "../../../router"; +import { createAddCredentialsHandler } from "./credentials"; import { createAddHarnessHandler } from "./harness"; import type { AddProjectResourceConfig } from "./types"; @@ -7,5 +8,6 @@ export function createAddProjectResourceHandler(config: AddProjectResourceConfig const projectAdd = new Router("add", "add project resources"); projectAdd.use(withProject({ projectManager: config.projectManager, cwd: process.cwd() })); projectAdd.handler(createAddHarnessHandler(config)); + projectAdd.handler(createAddCredentialsHandler(config)); return projectAdd; } diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index 8a15404c3..d80654824 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -1,5 +1,6 @@ import { afterEach, test, expect, describe } from "bun:test"; import { existsSync } from "node:fs"; +import { PassThrough } from "node:stream"; import { mkdir, mkdtemp, rm } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; @@ -13,8 +14,10 @@ import { import { DeserializationError, InputValidationError } from "../../errors"; import { FsReadWriteJson, type ReadWriteJson } from "../../io"; -async function run(args: string[], opts?: { core?: TestCoreClient }) { +async function run(args: string[], opts?: { core?: TestCoreClient; stdin?: string }) { const io = testIO(); + // testIO's stdin is a PassThrough; pre-filling and ending it simulates piped input. + if (opts?.stdin !== undefined) (io.io.stdin as unknown as PassThrough).end(opts.stdin); const core = opts?.core ?? new TestCoreClient(); const root = createRootHandler(core, { io: io.io, @@ -689,6 +692,284 @@ describe("project add harness", () => { }); }); +describe("project add credentials", () => { + test("api-key with a file:// secret records the spec entry and stores the key in .env.local", async () => { + const projectRoot = await inProject(); + const keyPath = join(projectRoot, "key.txt"); + await Bun.write(keyPath, "sk-123"); + + await run([ + "add", + "credentials", + "api-key", + "--name", + "svc-key", + "--api-key", + `file://${keyPath}`, + ]); + + const agentcoreJson = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect(agentcoreJson.credentials).toEqual([ + { authorizerType: "ApiKeyCredentialProvider", name: "svc-key" }, + ]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123"); + }); + + test("api-key without a secret writes a commented placeholder and tells the user to fill it", async () => { + const projectRoot = await inProject(); + const { io } = await run(["add", "credentials", "api-key", "--name", "svc-key"]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("# API key for credential provider 'svc-key' (set before deploy)"); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=\n"); + expect(io.stderr()).toContain( + "Set AGENTCORE_CREDENTIAL_SVC_KEY in agentcore/.env.local before you deploy", + ); + }); + + test("api-key with an external secret reference records it in the spec and skips .env.local", async () => { + const projectRoot = await inProject(); + const secretRef = { + secretId: "arn:aws:secretsmanager:us-west-2:123456789012:secret:s", + jsonKey: "apiKey", + }; + + await run([ + "add", + "credentials", + "api-key", + "--name", + "svc-key", + "--api-key-secret-reference", + JSON.stringify(secretRef), + ]); + + const agentcoreJson = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect(agentcoreJson.credentials).toEqual([ + { authorizerType: "ApiKeyCredentialProvider", name: "svc-key", secretRef }, + ]); + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).not.toContain("AGENTCORE_CREDENTIAL_SVC_KEY"); + }); + + const discoveryUrl = "https://idp.example.com/.well-known/openid-configuration"; + + test("oauth custom with guided flags and a stdin secret records the spec entry and the secret", async () => { + const projectRoot = await inProject(); + + await run( + [ + "add", + "credentials", + "oauth", + "--name", + "idp", + "--discovery-url", + discoveryUrl, + "--client-id", + "client-1", + "--scopes", + "openid", + "email", + "--client-secret", + "-", + ], + { stdin: "sssh" }, + ); + + const agentcoreJson = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect(agentcoreJson.credentials).toEqual([ + { + authorizerType: "OAuthCredentialProvider", + name: "idp", + vendor: "CustomOauth2", + clientId: "client-1", + discoveryUrl, + scopes: ["openid", "email"], + }, + ]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_IDP_CLIENT_SECRET=sssh"); + }); + + test("oauth vendored with --provider-configuration records the config and a secret placeholder", async () => { + const projectRoot = await inProject(); + + const { io } = await run([ + "add", + "credentials", + "oauth", + "--name", + "github", + "--vendor", + "GithubOauth2", + "--provider-configuration", + '{"githubOauth2ProviderConfig":{"clientId":"client-1"}}', + ]); + + const agentcoreJson = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect(agentcoreJson.credentials).toEqual([ + { + authorizerType: "OAuthCredentialProvider", + name: "github", + vendor: "GithubOauth2", + providerConfig: { githubOauth2ProviderConfig: { clientId: "client-1" } }, + }, + ]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_GITHUB_CLIENT_SECRET=\n"); + expect(io.stderr()).toContain( + "Set AGENTCORE_CREDENTIAL_GITHUB_CLIENT_SECRET in agentcore/.env.local before you deploy", + ); + }); + + test("preserves existing .env.local content and never overwrites an existing key", async () => { + const projectRoot = await inProject(); + const envPath = join(projectRoot, "agentcore", ".env.local"); + const original = await Bun.file(envPath).text(); + await Bun.write(envPath, `${original}AGENTCORE_CREDENTIAL_SVC_KEY=user-managed\n`); + + const { io } = await run(["add", "credentials", "api-key", "--name", "svc-key"]); + + const env = await Bun.file(envPath).text(); + expect(env).toStartWith(original); + expect(env.match(/AGENTCORE_CREDENTIAL_SVC_KEY=/g)).toHaveLength(1); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=user-managed"); + expect(io.stderr()).toContain("already exists"); + }); + + test("restores the .gitignore .env.local entry when the project lost it", async () => { + const projectRoot = await inProject(); + await Bun.write(join(projectRoot, ".gitignore"), "node_modules/\n"); + + const { io } = await run(["add", "credentials", "api-key", "--name", "svc-key"]); + + const gitignore = await Bun.file(join(projectRoot, ".gitignore")).text(); + expect(gitignore).toStartWith("node_modules/\n"); + expect(gitignore).toContain("\n.env.local\n"); + expect(io.stderr()).toContain(".gitignore"); + }); + + test("strips a single trailing newline from a file secret", async () => { + const projectRoot = await inProject(); + const keyPath = join(projectRoot, "key.txt"); + await Bun.write(keyPath, "sk-123\n"); + + await run([ + "add", + "credentials", + "api-key", + "--name", + "svc-key", + "--api-key", + `file://${keyPath}`, + ]); + + const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123\n"); + expect(env).not.toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123\n\n"); + }); + + test("rejects a duplicate credential name across credential types", async () => { + await inProject(); + await run(["add", "credentials", "api-key", "--name", "x"]); + await expect( + run(["add", "credentials", "oauth", "--name", "x", "--discovery-url", discoveryUrl]), + ).rejects.toThrow(/already exists/); + }); + + test.each<[string, string[], RegExp]>([ + [ + "api-key: an inline secret value", + ["api-key", "--name", "x", "--api-key", "sk-inline"], + /file:\/\//, + ], + ["api-key: a multi-line secret", ["api-key", "--name", "x", "--api-key", "-"], /single-line/], + [ + "oauth: an inline secret value", + ["oauth", "--name", "x", "--discovery-url", discoveryUrl, "--client-secret", "sssh"], + /file:\/\//, + ], + [ + "api-key: a secret combined with a secret reference", + [ + "api-key", + "--name", + "x", + "--api-key", + "-", + "--api-key-secret-reference", + '{"secretId":"arn:aws:secretsmanager:us-west-2:123:secret:s","jsonKey":"apiKey"}', + ], + /mutually exclusive/, + ], + [ + "oauth: a secret combined with a secret reference", + [ + "oauth", + "--name", + "x", + "--discovery-url", + discoveryUrl, + "--client-secret", + "-", + "--client-secret-reference", + '{"secretId":"arn:aws:secretsmanager:us-west-2:123:secret:s","jsonKey":"clientSecret"}', + ], + /mutually exclusive/, + ], + ["api-key: a missing --name", ["api-key"], /--name/], + ["oauth: a missing --name", ["oauth"], /--name/], + [ + "oauth: a vendored provider without --provider-configuration", + ["oauth", "--name", "x", "--vendor", "GithubOauth2"], + /--provider-configuration/, + ], + [ + "oauth: a guided custom provider without --discovery-url", + ["oauth", "--name", "x", "--client-id", "c"], + /--discovery-url/, + ], + [ + "oauth: --provider-configuration combined with --scopes", + [ + "oauth", + "--name", + "x", + "--vendor", + "GithubOauth2", + "--provider-configuration", + '{"githubOauth2ProviderConfig":{"clientId":"c"}}', + "--scopes", + "repo", + ], + /mutually exclusive/, + ], + [ + "oauth: secret material inside --provider-configuration", + [ + "oauth", + "--name", + "x", + "--vendor", + "GithubOauth2", + "--provider-configuration", + '{"githubOauth2ProviderConfig":{"clientId":"c","clientSecret":"sssh"}}', + ], + /secret material/, + ], + ])("rejects %s", async (_label, args, message) => { + await inProject(); + await expect(run(["add", "credentials", ...args], { stdin: "line1\nline2" })).rejects.toThrow( + message, + ); + }); +}); + describe("project build", () => { async function inBuildableProject(): Promise { const projectRoot = await inProject("MyAgent"); diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index c187abb96..26f0df3bc 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -1,4 +1,5 @@ import { HarnessSpecSchema } from "../../projectSchemas/harness"; +import type { CredentialSchema } from "../../projectSchemas/credential"; import type { ProjectSpecSchema } from "../../projectSchemas/project"; import type z from "zod"; import type { ProjectRuntimeSchema } from "../../projectSchemas/runtime"; @@ -40,6 +41,14 @@ export type Project = { spec: z.infer; }; +/** A line to add to agentcore/.env.local. Secret values travel here, never in the spec. */ +export type EnvLocalEntry = { + key: string; + /** An omitted value writes an empty placeholder the user fills before deploy. */ + value?: string; + comment: string; +}; + /** Discriminated union input for {@link ProjectManager.addResource}. */ export type AddResourceInput = | { @@ -49,6 +58,11 @@ export type AddResourceInput = | { resourceType: "runtime"; resourceConfig: z.input; + } + | { + resourceType: "credential"; + resourceConfig: z.input; + envEntries?: EnvLocalEntry[]; }; export type ProjectResource = AddResourceInput["resourceType"]; From 31decfec8d970cef6d48a39e12d5117b76397e92 Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Wed, 19 Aug 2026 16:28:01 -0400 Subject: [PATCH 3/9] test(project): fold trailing-newline coverage into the api-key happy path The standalone newline test duplicated the file-secret pipe end to end; the combined test feeds an echo-style newline-terminated file and asserts the stored value, covering both behaviors in one test. --- src/handlers/project/project.test.ts | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index d80654824..f026ef104 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -693,10 +693,11 @@ describe("project add harness", () => { }); describe("project add credentials", () => { - test("api-key with a file:// secret records the spec entry and stores the key in .env.local", async () => { + test("api-key with a file:// secret records the spec entry and stores the trailing-newline-stripped key in .env.local", async () => { const projectRoot = await inProject(); const keyPath = join(projectRoot, "key.txt"); - await Bun.write(keyPath, "sk-123"); + // The trailing newline mirrors `echo` and editor output; it must not reach the value. + await Bun.write(keyPath, "sk-123\n"); await run([ "add", @@ -714,7 +715,8 @@ describe("project add credentials", () => { ]); const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); - expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123"); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123\n"); + expect(env).not.toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123\n\n"); }); test("api-key without a secret writes a commented placeholder and tells the user to fill it", async () => { @@ -854,26 +856,6 @@ describe("project add credentials", () => { expect(io.stderr()).toContain(".gitignore"); }); - test("strips a single trailing newline from a file secret", async () => { - const projectRoot = await inProject(); - const keyPath = join(projectRoot, "key.txt"); - await Bun.write(keyPath, "sk-123\n"); - - await run([ - "add", - "credentials", - "api-key", - "--name", - "svc-key", - "--api-key", - `file://${keyPath}`, - ]); - - const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); - expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123\n"); - expect(env).not.toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123\n\n"); - }); - test("rejects a duplicate credential name across credential types", async () => { await inProject(); await run(["add", "credentials", "api-key", "--name", "x"]); From a82b5cff766e4e4c69229dfc97b5e23923c12f89 Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Wed, 19 Aug 2026 16:29:54 -0400 Subject: [PATCH 4/9] refactor(project): apply ponytail review to credentials add Inline the single-caller placeholder notice, extract the duplicated secret-reference exclusivity parse shared by both leaves, and simplify the envEntries guard. --- src/core/project/manager.tsx | 2 +- src/handlers/project/add/credentials/index.ts | 54 ++++++++++--------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 96271a6ab..98b5dc724 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -205,7 +205,7 @@ export class FsProjectManager implements ProjectManager { throw err; } - if (input.resourceType === "credential" && input.envEntries && input.envEntries.length > 0) { + if (input.resourceType === "credential" && input.envEntries?.length) { const envPath = join(project.rootPath, ENV_LOCAL_RELATIVE_PATH); yield { message: `Updating secrets file at '${envPath}'` }; const { skipped } = await upsertEnvLocalEntries(envPath, input.envEntries); diff --git a/src/handlers/project/add/credentials/index.ts b/src/handlers/project/add/credentials/index.ts index 0585d4cc1..93f301af2 100644 --- a/src/handlers/project/add/credentials/index.ts +++ b/src/handlers/project/add/credentials/index.ts @@ -2,7 +2,7 @@ import z from "zod"; import type { CredentialProviderVendorType } from "@aws-sdk/client-bedrock-agentcore-control"; import { createHandler, flag, ProjectKey, Router, type Context } from "../../../../router"; import { InputValidationError } from "../../../../errors"; -import { SourceResolver, type AppIO } from "../../../../io"; +import { SourceResolver } from "../../../../io"; import { parseSecretReference } from "../../../identity/parser"; import { parseProviderConfigFlags, @@ -26,6 +26,20 @@ function credentialEnvVarName(credentialName: string, suffix = ""): string { return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; } +/** Parses a secret-reference flag, rejecting a directly supplied secret alongside it. */ +function parseExclusiveSecretRef( + refFlag: string, + refValue: string | undefined, + secretFlag: string, + secretValue: string | undefined, +) { + if (!refValue) return undefined; + if (secretValue !== undefined) { + throw new InputValidationError(`--${secretFlag} and --${refFlag} are mutually exclusive`); + } + return parseSecretReference(refFlag, refValue); +} + /** * Resolves a secret flag while refusing inline values: an inline secret leaks * into shell history and process listings, so only stdin and files are allowed. @@ -73,14 +87,12 @@ const createAddApiKeyCredentialHandler = (config: AddProjectResourceConfig) => if (!flags.name) throw new InputValidationError("required option '--name ' not specified"); - const secretRef = flags["api-key-secret-reference"] - ? parseSecretReference("api-key-secret-reference", flags["api-key-secret-reference"]) - : undefined; - if (secretRef && flags["api-key"] !== undefined) { - throw new InputValidationError( - "--api-key and --api-key-secret-reference are mutually exclusive", - ); - } + const secretRef = parseExclusiveSecretRef( + "api-key-secret-reference", + flags["api-key-secret-reference"], + "api-key", + flags["api-key"], + ); const resolver = new SourceResolver({ stdin: config.io.stdin }); const apiKey = await resolveSecretFlag(resolver, "api-key", flags["api-key"]); @@ -141,14 +153,12 @@ const createAddOauthCredentialHandler = (config: AddProjectResourceConfig) => if (!flags.name) throw new InputValidationError("required option '--name ' not specified"); - const secretRef = flags["client-secret-reference"] - ? parseSecretReference("client-secret-reference", flags["client-secret-reference"]) - : undefined; - if (secretRef && flags["client-secret"] !== undefined) { - throw new InputValidationError( - "--client-secret and --client-secret-reference are mutually exclusive", - ); - } + const secretRef = parseExclusiveSecretRef( + "client-secret-reference", + flags["client-secret-reference"], + "client-secret", + flags["client-secret"], + ); const mode = parseProviderConfigFlags({ clientId: flags["client-id"], @@ -217,13 +227,7 @@ async function addCredentialToProject( } config.io.stderr.write(`added credential '${input.resourceConfig.name}' to '${project.name}'\n`); - notifyPlaceholders(config.io, input.envEntries ?? []); -} - -/** Tells the user which .env.local keys still need a value before deploy. */ -function notifyPlaceholders(io: AppIO, envEntries: EnvLocalEntry[]): void { - const placeholders = envEntries.filter((entry) => entry.value === undefined); - for (const entry of placeholders) { - io.stderr.write(`Set ${entry.key} in agentcore/.env.local before you deploy.\n`); + for (const entry of (input.envEntries ?? []).filter((e) => e.value === undefined)) { + config.io.stderr.write(`Set ${entry.key} in agentcore/.env.local before you deploy.\n`); } } From 1cd5d1401cc8e2030fdb2a72e746aff9ab4179f4 Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Wed, 19 Aug 2026 16:36:57 -0400 Subject: [PATCH 5/9] fix(project): require 3-character credential names to match ConfigIO E2E verification of 'project build' against the published @aws/agentcore-cdk found a contract drift: its ConfigIO requires credential names of 3-255 characters while the CLI allowed 1-128, so a 2-character name passed 'add' and then broke synth. Tighten the CLI to the intersection (3-128, service character set) until the L3 schema aligns with the service's 1-128 rule. --- src/handlers/project/project.test.ts | 4 ++-- src/projectSchemas/credential.test.ts | 5 +++++ src/projectSchemas/credential.ts | 5 ++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index f026ef104..751ba8bc7 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -858,9 +858,9 @@ describe("project add credentials", () => { test("rejects a duplicate credential name across credential types", async () => { await inProject(); - await run(["add", "credentials", "api-key", "--name", "x"]); + await run(["add", "credentials", "api-key", "--name", "dup"]); await expect( - run(["add", "credentials", "oauth", "--name", "x", "--discovery-url", discoveryUrl]), + run(["add", "credentials", "oauth", "--name", "dup", "--discovery-url", discoveryUrl]), ).rejects.toThrow(/already exists/); }); diff --git a/src/projectSchemas/credential.test.ts b/src/projectSchemas/credential.test.ts index 0de8b4f37..c176ecb4e 100644 --- a/src/projectSchemas/credential.test.ts +++ b/src/projectSchemas/credential.test.ts @@ -124,6 +124,11 @@ describe("credential schema", () => { { authorizerType: "ApiKeyCredentialProvider", name: "bad name!" }, /alphanumeric/, ], + [ + "a credential name shorter than 3 characters", + { authorizerType: "ApiKeyCredentialProvider", name: "ab" }, + /3 characters/, + ], ])("rejects %s", (_label, value, message) => { const result = CredentialSchema.safeParse(value); expect(result.success).toBe(false); diff --git a/src/projectSchemas/credential.ts b/src/projectSchemas/credential.ts index ba8ef771e..a5d260071 100644 --- a/src/projectSchemas/credential.ts +++ b/src/projectSchemas/credential.ts @@ -1,8 +1,11 @@ import z from "zod"; import { PaymentProviderSchema } from "./payment"; +// Min 3 keeps names inside what @aws/agentcore-cdk's ConfigIO accepts (3-255) +// so `add` never produces a spec that `build` rejects; max 128 and the character +// set are the Identity service's own limits. export const CredentialNameSchema = z .string() - .min(1, "Credential name is required") + .min(3, "Credential name must be at least 3 characters") .max(128, "Credential name must be 128 characters or less") .regex( /^[a-zA-Z0-9\-_]+$/, From 19fd64d147dacefbfe48a5af240d572595a23c5e Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Wed, 19 Aug 2026 22:00:58 -0400 Subject: [PATCH 6/9] refactor(project): route envLocal IO through the established io seams envLocal read/write now uses readTextFile and atomicWrite from src/io instead of raw node:fs calls, matching how the rest of core does file IO and making the secrets-file write atomic. Adds coverage for the create-when-missing .env.local branch, which the scaffolded template previously masked. Also drops a review-marker comment. --- src/core/project/envLocal.ts | 12 ++++++------ src/handlers/project/project.test.ts | 11 +++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts index 4d8d2d8c0..4678dbcd3 100644 --- a/src/core/project/envLocal.ts +++ b/src/core/project/envLocal.ts @@ -1,5 +1,5 @@ -import { readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; +import { atomicWrite, readTextFile } from "../../io"; import type { EnvLocalEntry } from "../../handlers/project/types"; /** The project-relative path of the local secrets file (read by `agentcore dev`). */ @@ -36,7 +36,7 @@ export async function upsertEnvLocalEntries( written.push(entry.key); } - if (written.length > 0) await writeFile(envPath, content); + if (written.length > 0) await atomicWrite(envPath, content); return { written, skipped }; } @@ -47,8 +47,8 @@ export async function upsertEnvLocalEntries( export async function ensureGitignoreCoversEnvLocal(rootPath: string): Promise { const gitignorePath = join(rootPath, ".gitignore"); const existing = await readOrEmpty(gitignorePath); - // ponytail: literal-line match, not full gitignore pattern semantics; a - // matcher library is warranted only if projects grow exotic ignore rules. + // Literal-line match, not full gitignore pattern semantics; a matcher + // library is warranted only if projects grow exotic ignore rules. const covered = existing .split("\n") .map((line) => line.trim()) @@ -56,7 +56,7 @@ export async function ensureGitignoreCoversEnvLocal(rootPath: string): Promise { try { - return await readFile(path, "utf8"); + return await readTextFile(path); } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") return ""; throw error; diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index 751ba8bc7..278008725 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -844,6 +844,17 @@ describe("project add credentials", () => { expect(io.stderr()).toContain("already exists"); }); + test("creates .env.local when the project lacks one", async () => { + const projectRoot = await inProject(); + const envPath = join(projectRoot, "agentcore", ".env.local"); + await rm(envPath); + + await run(["add", "credentials", "api-key", "--name", "svc-key"]); + + const env = await Bun.file(envPath).text(); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=\n"); + }); + test("restores the .gitignore .env.local entry when the project lost it", async () => { const projectRoot = await inProject(); await Bun.write(join(projectRoot, ".gitignore"), "node_modules/\n"); From 9f6190ffc1cd7a374164c630d0eed3bdfe8d644e Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Fri, 21 Aug 2026 11:29:09 -0400 Subject: [PATCH 7/9] Address review: EnvLocalFile class with rollback, 1:1 handler files, drop redundant gitignore write, standardize testIO stdin --- src/core/project/envLocal.test.ts | 53 +++++ src/core/project/envLocal.ts | 105 ++++---- src/core/project/manager.tsx | 47 ++-- .../project/add/credentials/api-key/index.ts | 61 +++++ src/handlers/project/add/credentials/index.ts | 225 +----------------- .../project/add/credentials/oauth/index.ts | 115 +++++++++ .../project/add/credentials/shared.ts | 70 ++++++ src/handlers/project/project.test.ts | 17 +- src/projectSchemas/credential.ts | 7 +- src/testing/testIO.tsx | 9 +- 10 files changed, 395 insertions(+), 314 deletions(-) create mode 100644 src/core/project/envLocal.test.ts create mode 100644 src/handlers/project/add/credentials/api-key/index.ts create mode 100644 src/handlers/project/add/credentials/oauth/index.ts create mode 100644 src/handlers/project/add/credentials/shared.ts diff --git a/src/core/project/envLocal.test.ts b/src/core/project/envLocal.test.ts new file mode 100644 index 000000000..c619a4eb0 --- /dev/null +++ b/src/core/project/envLocal.test.ts @@ -0,0 +1,53 @@ +import { afterEach, expect, test } from "bun:test"; +import { existsSync } from "node:fs"; +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { tmpdir } from "node:os"; +import { EnvLocalFile } from "./envLocal"; + +const roots: string[] = []; +afterEach(async () => { + await Promise.all(roots.splice(0).map((r) => rm(r, { recursive: true, force: true }))); +}); + +async function tempRoot(): Promise { + const root = await mkdtemp(join(tmpdir(), "envlocal-")); + roots.push(root); + // Real projects always have the agentcore/ dir; the class does not create it. + await mkdir(dirname(new EnvLocalFile(root).path), { recursive: true }); + return root; +} + +const ENTRY = { key: "SECRET", value: "v", comment: "c" }; + +test("rollback deletes the file it created", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await file.upsert([ENTRY]); + expect(existsSync(file.path)).toBe(true); + + await file.rollback(); + expect(existsSync(file.path)).toBe(false); +}); + +test("rollback restores the prior content of an existing file", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await Bun.write(file.path, "EXISTING=1\n"); + + await file.upsert([ENTRY]); + expect(await Bun.file(file.path).text()).toContain("SECRET=v"); + + await file.rollback(); + expect(await Bun.file(file.path).text()).toBe("EXISTING=1\n"); +}); + +test("rollback is a no-op when upsert wrote nothing", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await Bun.write(file.path, "SECRET=kept\n"); + + await file.upsert([ENTRY]); // key already present, so nothing is written + await file.rollback(); + expect(await Bun.file(file.path).text()).toBe("SECRET=kept\n"); +}); diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts index 4678dbcd3..57427736b 100644 --- a/src/core/project/envLocal.ts +++ b/src/core/project/envLocal.ts @@ -1,3 +1,4 @@ +import { rm } from "node:fs/promises"; import { join } from "node:path"; import { atomicWrite, readTextFile } from "../../io"; import type { EnvLocalEntry } from "../../handlers/project/types"; @@ -8,66 +9,72 @@ export const ENV_LOCAL_RELATIVE_PATH = join("agentcore", ".env.local"); const KEY_LINE = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=/; /** - * Appends entries to a .env-format file, creating it when missing. Keys that - * already exist are never overwritten so user-managed values survive re-runs. + * The project's `.env.local` secrets file, edited transactionally. `upsert` + * appends entries (never overwriting an existing key) and snapshots the prior + * state so `rollback` can undo the write if a later step in the same operation + * fails. Mirrors the class shape of {@link SourceResolver} so callers hold one + * object and reverse its effect, rather than tracking loose paths. */ -export async function upsertEnvLocalEntries( - envPath: string, - entries: EnvLocalEntry[], -): Promise<{ written: string[]; skipped: string[] }> { - const existing = await readOrEmpty(envPath); - const existingKeys = new Set( - existing - .split("\n") - .map((line) => KEY_LINE.exec(line)?.[1]) - .filter((key) => key !== undefined), - ); +export class EnvLocalFile { + // undefined: upsert has not written; null: file did not exist before the + // write; string: the file's content before the write. + private snapshot?: string | null; - const written: string[] = []; - const skipped: string[] = []; - let content = existing; - for (const entry of entries) { - if (existingKeys.has(entry.key)) { - skipped.push(entry.key); - continue; - } - const separator = content === "" || content.endsWith("\n") ? "" : "\n"; - content += `${separator}# ${entry.comment}\n${entry.key}=${entry.value ?? ""}\n`; - written.push(entry.key); + constructor(private readonly rootPath: string) {} + + /** The absolute path of the secrets file. */ + get path(): string { + return join(this.rootPath, ENV_LOCAL_RELATIVE_PATH); } - if (written.length > 0) await atomicWrite(envPath, content); - return { written, skipped }; -} + /** + * Appends entries, creating the file when missing. Keys that already exist + * are left unchanged so user-managed values survive re-runs. Returns the keys + * written and those skipped. + */ + async upsert(entries: EnvLocalEntry[]): Promise<{ written: string[]; skipped: string[] }> { + const existing = await readOrNull(this.path); + const existingKeys = new Set( + (existing ?? "") + .split("\n") + .map((line) => KEY_LINE.exec(line)?.[1]) + .filter((key) => key !== undefined), + ); -/** - * Makes sure the project's .gitignore keeps .env.local out of version control. - * Returns true when the file was created or amended. - */ -export async function ensureGitignoreCoversEnvLocal(rootPath: string): Promise { - const gitignorePath = join(rootPath, ".gitignore"); - const existing = await readOrEmpty(gitignorePath); - // Literal-line match, not full gitignore pattern semantics; a matcher - // library is warranted only if projects grow exotic ignore rules. - const covered = existing - .split("\n") - .map((line) => line.trim()) - .some((line) => line === ".env.local" || line === ".env*.local"); - if (covered) return false; + const written: string[] = []; + const skipped: string[] = []; + let content = existing ?? ""; + for (const entry of entries) { + if (existingKeys.has(entry.key)) { + skipped.push(entry.key); + continue; + } + const separator = content === "" || content.endsWith("\n") ? "" : "\n"; + content += `${separator}# ${entry.comment}\n${entry.key}=${entry.value ?? ""}\n`; + written.push(entry.key); + } - const separator = existing === "" || existing.endsWith("\n") ? "" : "\n"; - await atomicWrite( - gitignorePath, - `${existing}${separator}# Local secrets (added by agentcore; never commit)\n.env.local\n`, - ); - return true; + if (written.length > 0) { + this.snapshot = existing; + await atomicWrite(this.path, content); + } + return { written, skipped }; + } + + /** Restores the file to its pre-`upsert` state; a no-op when `upsert` wrote nothing. */ + async rollback(): Promise { + if (this.snapshot === undefined) return; + if (this.snapshot === null) await rm(this.path, { force: true }); + else await atomicWrite(this.path, this.snapshot); + } } -async function readOrEmpty(path: string): Promise { +/** Reads a file, returning null when it does not exist so callers can tell empty from absent. */ +async function readOrNull(path: string): Promise { try { return await readTextFile(path); } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return ""; + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; throw error; } } diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 98b5dc724..bb684754a 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -19,11 +19,7 @@ import { type ReadWriteJson, } from "../../io"; import { defaultSource, type AssetSource } from "./source"; -import { - ENV_LOCAL_RELATIVE_PATH, - ensureGitignoreCoversEnvLocal, - upsertEnvLocalEntries, -} from "./envLocal"; +import { ENV_LOCAL_RELATIVE_PATH, EnvLocalFile } from "./envLocal"; import { createHarnessTreeFromSpec, createProjectTreeFromTemplate, TEMPLATES } from "./templates"; import { ProjectSpecSchema } from "../../projectSchemas/project"; import { enclosingProjectRoot } from "./fsUtils"; @@ -147,6 +143,8 @@ export class FsProjectManager implements ProjectManager { // Widened: arms push their own shapes; the whole-spec safeParse below validates. const newResources: unknown[] = [...existingResources]; const scaffoldedPaths: string[] = []; + // Non-file work that a failed spec write must also reverse. + let envFile: EnvLocalFile | undefined; switch (resourceType) { case "harness": { @@ -167,8 +165,19 @@ export class FsProjectManager implements ProjectManager { ); } case "credential": { - // Spec-only resource: no scaffolding, secrets go to .env.local below. + // No file scaffolding; the secret placeholder is staged into .env.local + // and reversed with the spec write if that commit fails. newResources.push(input.resourceConfig); + if (input.envEntries?.length) { + envFile = new EnvLocalFile(project.rootPath); + yield { message: `Updating secrets file at '${envFile.path}'` }; + const { skipped } = await envFile.upsert(input.envEntries); + for (const key of skipped) { + yield { + message: `'${key}' already exists in ${ENV_LOCAL_RELATIVE_PATH}; left unchanged`, + }; + } + } break; } // TODO: add limited special casing for runtime and default for other resources that proxy directly to spec changes. @@ -190,10 +199,10 @@ export class FsProjectManager implements ProjectManager { newProjectSpec = await this.json.write(agentCoreSpecPath, newSpecParseResult.data); } catch (err) { this.logger.warn( - `failed to update ${agentCoreSpecPath}; attempting best-effort cleanup of scaffolded files`, + `failed to update ${agentCoreSpecPath}; attempting best-effort cleanup of staged changes`, ); - await Promise.all( - scaffoldedPaths.map((p) => + await Promise.all([ + ...scaffoldedPaths.map((p) => rm(p, { recursive: true, force: true }).catch((e) => { const error = AgentCoreCLIError.fromError(e); this.logger @@ -201,22 +210,16 @@ export class FsProjectManager implements ProjectManager { .warn(`failed to clean up ${p}`); }), ), - ); + envFile?.rollback().catch((e) => { + const error = AgentCoreCLIError.fromError(e); + this.logger + .child({ errorName: error.name, errorMessage: error.message }) + .warn(`failed to roll back ${ENV_LOCAL_RELATIVE_PATH}`); + }), + ]); throw err; } - if (input.resourceType === "credential" && input.envEntries?.length) { - const envPath = join(project.rootPath, ENV_LOCAL_RELATIVE_PATH); - yield { message: `Updating secrets file at '${envPath}'` }; - const { skipped } = await upsertEnvLocalEntries(envPath, input.envEntries); - for (const key of skipped) { - yield { message: `'${key}' already exists in ${ENV_LOCAL_RELATIVE_PATH}; left unchanged` }; - } - if (await ensureGitignoreCoversEnvLocal(project.rootPath)) { - yield { message: `Added '.env.local' to .gitignore so secrets stay out of git` }; - } - } - return { ...project, spec: newProjectSpec, diff --git a/src/handlers/project/add/credentials/api-key/index.ts b/src/handlers/project/add/credentials/api-key/index.ts new file mode 100644 index 000000000..1674c8401 --- /dev/null +++ b/src/handlers/project/add/credentials/api-key/index.ts @@ -0,0 +1,61 @@ +import z from "zod"; +import { createHandler, flag } from "../../../../../router"; +import { InputValidationError } from "../../../../../errors"; +import { SourceResolver } from "../../../../../io"; +import type { AddProjectResourceConfig } from "../../types"; +import type { EnvLocalEntry } from "../../../types"; +import { + addCredentialToProject, + credentialEnvVarName, + parseExclusiveSecretRef, + resolveSecretFlag, +} from "../shared"; + +export const createAddApiKeyCredentialHandler = (config: AddProjectResourceConfig) => + createHandler({ + name: "api-key", + description: "add an API key credential provider to the current project", + flags: [ + flag("name", "the name of the credential provider", z.string().optional()), + flag( + "api-key", + "the API key (file://path or - for stdin; inline values are rejected)", + z.string().optional(), + { sensitive: true }, + ), + flag( + "api-key-secret-reference", + 'external secret reference JSON: {"secretId":"","jsonKey":""}', + z.string().optional(), + ), + ], + handle: async (ctx, flags) => { + if (!flags.name) + throw new InputValidationError("required option '--name ' not specified"); + + const secretRef = parseExclusiveSecretRef( + "api-key-secret-reference", + flags["api-key-secret-reference"], + "api-key", + flags["api-key"], + ); + + const resolver = new SourceResolver({ stdin: config.io.stdin }); + const apiKey = await resolveSecretFlag(resolver, "api-key", flags["api-key"]); + + const envEntries: EnvLocalEntry[] = secretRef + ? [] + : [ + { + key: credentialEnvVarName(flags.name), + value: apiKey, + comment: `API key for credential provider '${flags.name}' (set before deploy)`, + }, + ]; + + await addCredentialToProject(ctx, config, { + resourceConfig: { authorizerType: "ApiKeyCredentialProvider", name: flags.name, secretRef }, + envEntries, + }); + }, + }); diff --git a/src/handlers/project/add/credentials/index.ts b/src/handlers/project/add/credentials/index.ts index 93f301af2..179ee66c3 100644 --- a/src/handlers/project/add/credentials/index.ts +++ b/src/handlers/project/add/credentials/index.ts @@ -1,15 +1,7 @@ -import z from "zod"; -import type { CredentialProviderVendorType } from "@aws-sdk/client-bedrock-agentcore-control"; -import { createHandler, flag, ProjectKey, Router, type Context } from "../../../../router"; -import { InputValidationError } from "../../../../errors"; -import { SourceResolver } from "../../../../io"; -import { parseSecretReference } from "../../../identity/parser"; -import { - parseProviderConfigFlags, - validateProviderConfigMode, -} from "../../../identity/oauth2-credential-provider/config"; +import { Router } from "../../../../router"; import type { AddProjectResourceConfig } from "../types"; -import type { AddResourceInput, EnvLocalEntry } from "../../types"; +import { createAddApiKeyCredentialHandler } from "./api-key"; +import { createAddOauthCredentialHandler } from "./oauth"; export function createAddCredentialsHandler(config: AddProjectResourceConfig): Router { const credentials = new Router( @@ -20,214 +12,3 @@ export function createAddCredentialsHandler(config: AddProjectResourceConfig): R credentials.handler(createAddOauthCredentialHandler(config)); return credentials; } - -/** Derives the .env.local variable name a credential's secret is stored under. */ -function credentialEnvVarName(credentialName: string, suffix = ""): string { - return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; -} - -/** Parses a secret-reference flag, rejecting a directly supplied secret alongside it. */ -function parseExclusiveSecretRef( - refFlag: string, - refValue: string | undefined, - secretFlag: string, - secretValue: string | undefined, -) { - if (!refValue) return undefined; - if (secretValue !== undefined) { - throw new InputValidationError(`--${secretFlag} and --${refFlag} are mutually exclusive`); - } - return parseSecretReference(refFlag, refValue); -} - -/** - * Resolves a secret flag while refusing inline values: an inline secret leaks - * into shell history and process listings, so only stdin and files are allowed. - * A single trailing newline is stripped (echo and editors add one). - */ -async function resolveSecretFlag( - resolver: SourceResolver, - name: string, - source: string | undefined, -): Promise { - if (source !== undefined && source !== "-" && !source.startsWith("file://")) { - throw new InputValidationError( - `--${name} must come from stdin ('-') or a file ('file://'); ` + - "inline secret values are not accepted", - ); - } - const value = await resolver.resolveText(name, source); - if (value === undefined) return undefined; - const normalized = value.replace(/\r?\n$/, ""); - if (normalized.includes("\n")) { - throw new InputValidationError(`--${name} must be a single-line value`); - } - return normalized; -} - -const createAddApiKeyCredentialHandler = (config: AddProjectResourceConfig) => - createHandler({ - name: "api-key", - description: "add an API key credential provider to the current project", - flags: [ - flag("name", "the name of the credential provider", z.string().optional()), - flag( - "api-key", - "the API key (file://path or - for stdin; inline values are rejected)", - z.string().optional(), - { sensitive: true }, - ), - flag( - "api-key-secret-reference", - 'external secret reference JSON: {"secretId":"","jsonKey":""}', - z.string().optional(), - ), - ], - handle: async (ctx, flags) => { - if (!flags.name) - throw new InputValidationError("required option '--name ' not specified"); - - const secretRef = parseExclusiveSecretRef( - "api-key-secret-reference", - flags["api-key-secret-reference"], - "api-key", - flags["api-key"], - ); - - const resolver = new SourceResolver({ stdin: config.io.stdin }); - const apiKey = await resolveSecretFlag(resolver, "api-key", flags["api-key"]); - - const envEntries: EnvLocalEntry[] = secretRef - ? [] - : [ - { - key: credentialEnvVarName(flags.name), - value: apiKey, - comment: `API key for credential provider '${flags.name}' (set before deploy)`, - }, - ]; - - await addCredentialToProject(ctx, config, { - resourceConfig: { authorizerType: "ApiKeyCredentialProvider", name: flags.name, secretRef }, - envEntries, - }); - }, - }); - -const createAddOauthCredentialHandler = (config: AddProjectResourceConfig) => - createHandler({ - name: "oauth", - description: "add an OAuth2 credential provider to the current project", - flags: [ - flag("name", "the name of the credential provider", z.string().optional()), - flag( - "vendor", - "the OAuth2 vendor (e.g. GithubOauth2); custom providers use the guided flags instead", - z.string().default("CustomOauth2"), - ), - flag("client-id", "OAuth2 client ID (guided custom OAuth2)", z.string().optional()), - flag("discovery-url", "OAuth2 discovery URL (guided custom OAuth2)", z.string().optional()), - flag( - "scopes", - "OAuth2 scopes the provider grants (guided custom OAuth2)", - z.array(z.string()).optional(), - ), - flag( - "provider-configuration", - "complete secret-free Oauth2ProviderConfigInput JSON (required for vendored providers)", - z.string().optional(), - ), - flag( - "client-secret", - "the client secret (file://path or - for stdin; inline values are rejected)", - z.string().optional(), - { sensitive: true }, - ), - flag( - "client-secret-reference", - 'external secret reference JSON: {"secretId":"","jsonKey":""}', - z.string().optional(), - ), - ], - handle: async (ctx, flags) => { - if (!flags.name) - throw new InputValidationError("required option '--name ' not specified"); - - const secretRef = parseExclusiveSecretRef( - "client-secret-reference", - flags["client-secret-reference"], - "client-secret", - flags["client-secret"], - ); - - const mode = parseProviderConfigFlags({ - clientId: flags["client-id"], - discoveryUrl: flags["discovery-url"], - providerConfiguration: flags["provider-configuration"], - }); - validateProviderConfigMode(mode, flags.vendor as CredentialProviderVendorType); - if (mode.kind === "complete" && flags.scopes !== undefined) { - throw new InputValidationError( - "--provider-configuration and --scopes are mutually exclusive", - ); - } - - const resolver = new SourceResolver({ stdin: config.io.stdin }); - const clientSecret = await resolveSecretFlag( - resolver, - "client-secret", - flags["client-secret"], - ); - - const resourceConfig = - mode.kind === "complete" - ? { - authorizerType: "OAuthCredentialProvider" as const, - name: flags.name, - vendor: flags.vendor, - providerConfig: mode.config, - clientSecretRef: secretRef, - } - : { - authorizerType: "OAuthCredentialProvider" as const, - name: flags.name, - vendor: flags.vendor, - clientId: flags["client-id"], - discoveryUrl: flags["discovery-url"], - scopes: flags.scopes, - clientSecretRef: secretRef, - }; - - const envEntries: EnvLocalEntry[] = secretRef - ? [] - : [ - { - key: credentialEnvVarName(flags.name, "_CLIENT_SECRET"), - value: clientSecret, - comment: `OAuth client secret for credential provider '${flags.name}' (set before deploy)`, - }, - ]; - - await addCredentialToProject(ctx, config, { resourceConfig, envEntries }); - }, - }); - -/** Runs the shared add flow: spec update, env entries, progress, and fill-before-deploy notice. */ -async function addCredentialToProject( - ctx: Context, - config: AddProjectResourceConfig, - input: Omit, "resourceType">, -): Promise { - const project = ctx.require(ProjectKey); - for await (const event of config.projectManager.addResource(project, { - resourceType: "credential", - ...input, - })) { - config.io.stderr.write(`${event.message}\n`); - } - - config.io.stderr.write(`added credential '${input.resourceConfig.name}' to '${project.name}'\n`); - for (const entry of (input.envEntries ?? []).filter((e) => e.value === undefined)) { - config.io.stderr.write(`Set ${entry.key} in agentcore/.env.local before you deploy.\n`); - } -} diff --git a/src/handlers/project/add/credentials/oauth/index.ts b/src/handlers/project/add/credentials/oauth/index.ts new file mode 100644 index 000000000..31fb4f2c0 --- /dev/null +++ b/src/handlers/project/add/credentials/oauth/index.ts @@ -0,0 +1,115 @@ +import z from "zod"; +import type { CredentialProviderVendorType } from "@aws-sdk/client-bedrock-agentcore-control"; +import { createHandler, flag } from "../../../../../router"; +import { InputValidationError } from "../../../../../errors"; +import { SourceResolver } from "../../../../../io"; +import { + parseProviderConfigFlags, + validateProviderConfigMode, +} from "../../../../identity/oauth2-credential-provider/config"; +import type { AddProjectResourceConfig } from "../../types"; +import type { EnvLocalEntry } from "../../../types"; +import { + addCredentialToProject, + credentialEnvVarName, + parseExclusiveSecretRef, + resolveSecretFlag, +} from "../shared"; + +export const createAddOauthCredentialHandler = (config: AddProjectResourceConfig) => + createHandler({ + name: "oauth", + description: "add an OAuth2 credential provider to the current project", + flags: [ + flag("name", "the name of the credential provider", z.string().optional()), + flag( + "vendor", + "the OAuth2 vendor (e.g. GithubOauth2); custom providers use the guided flags instead", + z.string().default("CustomOauth2"), + ), + flag("client-id", "OAuth2 client ID (guided custom OAuth2)", z.string().optional()), + flag("discovery-url", "OAuth2 discovery URL (guided custom OAuth2)", z.string().optional()), + flag( + "scopes", + "OAuth2 scopes the provider grants (guided custom OAuth2)", + z.array(z.string()).optional(), + ), + flag( + "provider-configuration", + "complete secret-free Oauth2ProviderConfigInput JSON (required for vendored providers)", + z.string().optional(), + ), + flag( + "client-secret", + "the client secret (file://path or - for stdin; inline values are rejected)", + z.string().optional(), + { sensitive: true }, + ), + flag( + "client-secret-reference", + 'external secret reference JSON: {"secretId":"","jsonKey":""}', + z.string().optional(), + ), + ], + handle: async (ctx, flags) => { + if (!flags.name) + throw new InputValidationError("required option '--name ' not specified"); + + const secretRef = parseExclusiveSecretRef( + "client-secret-reference", + flags["client-secret-reference"], + "client-secret", + flags["client-secret"], + ); + + const mode = parseProviderConfigFlags({ + clientId: flags["client-id"], + discoveryUrl: flags["discovery-url"], + providerConfiguration: flags["provider-configuration"], + }); + validateProviderConfigMode(mode, flags.vendor as CredentialProviderVendorType); + if (mode.kind === "complete" && flags.scopes !== undefined) { + throw new InputValidationError( + "--provider-configuration and --scopes are mutually exclusive", + ); + } + + const resolver = new SourceResolver({ stdin: config.io.stdin }); + const clientSecret = await resolveSecretFlag( + resolver, + "client-secret", + flags["client-secret"], + ); + + const resourceConfig = + mode.kind === "complete" + ? { + authorizerType: "OAuthCredentialProvider" as const, + name: flags.name, + vendor: flags.vendor, + providerConfig: mode.config, + clientSecretRef: secretRef, + } + : { + authorizerType: "OAuthCredentialProvider" as const, + name: flags.name, + vendor: flags.vendor, + clientId: flags["client-id"], + discoveryUrl: flags["discovery-url"], + scopes: flags.scopes, + clientSecretRef: secretRef, + }; + + const envEntries: EnvLocalEntry[] = secretRef + ? [] + : [ + { + key: credentialEnvVarName(flags.name, "_CLIENT_SECRET"), + value: clientSecret, + comment: `OAuth client secret for credential provider '${flags.name}' (set before deploy)`, + }, + ]; + + await addCredentialToProject(ctx, config, { resourceConfig, envEntries }); + }, + }); diff --git a/src/handlers/project/add/credentials/shared.ts b/src/handlers/project/add/credentials/shared.ts new file mode 100644 index 000000000..b744f8171 --- /dev/null +++ b/src/handlers/project/add/credentials/shared.ts @@ -0,0 +1,70 @@ +import { ProjectKey, type Context } from "../../../../router"; +import { InputValidationError } from "../../../../errors"; +import { SourceResolver } from "../../../../io"; +import { parseSecretReference } from "../../../identity/parser"; +import type { AddProjectResourceConfig } from "../types"; +import type { AddResourceInput } from "../../types"; + +/** Derives the .env.local variable name a credential's secret is stored under. */ +export function credentialEnvVarName(credentialName: string, suffix = ""): string { + return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`; +} + +/** Parses a secret-reference flag, rejecting a directly supplied secret alongside it. */ +export function parseExclusiveSecretRef( + refFlag: string, + refValue: string | undefined, + secretFlag: string, + secretValue: string | undefined, +) { + if (!refValue) return undefined; + if (secretValue !== undefined) { + throw new InputValidationError(`--${secretFlag} and --${refFlag} are mutually exclusive`); + } + return parseSecretReference(refFlag, refValue); +} + +/** + * Resolves a secret flag while refusing inline values: an inline secret leaks + * into shell history and process listings, so only stdin and files are allowed. + * A single trailing newline is stripped (echo and editors add one). + */ +export async function resolveSecretFlag( + resolver: SourceResolver, + name: string, + source: string | undefined, +): Promise { + if (source !== undefined && source !== "-" && !source.startsWith("file://")) { + throw new InputValidationError( + `--${name} must come from stdin ('-') or a file ('file://'); ` + + "inline secret values are not accepted", + ); + } + const value = await resolver.resolveText(name, source); + if (value === undefined) return undefined; + const normalized = value.replace(/\r?\n$/, ""); + if (normalized.includes("\n")) { + throw new InputValidationError(`--${name} must be a single-line value`); + } + return normalized; +} + +/** Runs the shared add flow: spec update, env entries, progress, and fill-before-deploy notice. */ +export async function addCredentialToProject( + ctx: Context, + config: AddProjectResourceConfig, + input: Omit, "resourceType">, +): Promise { + const project = ctx.require(ProjectKey); + for await (const event of config.projectManager.addResource(project, { + resourceType: "credential", + ...input, + })) { + config.io.stderr.write(`${event.message}\n`); + } + + config.io.stderr.write(`added credential '${input.resourceConfig.name}' to '${project.name}'\n`); + for (const entry of (input.envEntries ?? []).filter((e) => e.value === undefined)) { + config.io.stderr.write(`Set ${entry.key} in agentcore/.env.local before you deploy.\n`); + } +} diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index 278008725..c3b602840 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -1,6 +1,5 @@ import { afterEach, test, expect, describe } from "bun:test"; import { existsSync } from "node:fs"; -import { PassThrough } from "node:stream"; import { mkdir, mkdtemp, rm } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; @@ -15,9 +14,7 @@ import { DeserializationError, InputValidationError } from "../../errors"; import { FsReadWriteJson, type ReadWriteJson } from "../../io"; async function run(args: string[], opts?: { core?: TestCoreClient; stdin?: string }) { - const io = testIO(); - // testIO's stdin is a PassThrough; pre-filling and ending it simulates piped input. - if (opts?.stdin !== undefined) (io.io.stdin as unknown as PassThrough).end(opts.stdin); + const io = testIO({ stdin: opts?.stdin }); const core = opts?.core ?? new TestCoreClient(); const root = createRootHandler(core, { io: io.io, @@ -855,18 +852,6 @@ describe("project add credentials", () => { expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=\n"); }); - test("restores the .gitignore .env.local entry when the project lost it", async () => { - const projectRoot = await inProject(); - await Bun.write(join(projectRoot, ".gitignore"), "node_modules/\n"); - - const { io } = await run(["add", "credentials", "api-key", "--name", "svc-key"]); - - const gitignore = await Bun.file(join(projectRoot, ".gitignore")).text(); - expect(gitignore).toStartWith("node_modules/\n"); - expect(gitignore).toContain("\n.env.local\n"); - expect(io.stderr()).toContain(".gitignore"); - }); - test("rejects a duplicate credential name across credential types", async () => { await inProject(); await run(["add", "credentials", "api-key", "--name", "dup"]); diff --git a/src/projectSchemas/credential.ts b/src/projectSchemas/credential.ts index a5d260071..49db8a4d6 100644 --- a/src/projectSchemas/credential.ts +++ b/src/projectSchemas/credential.ts @@ -1,8 +1,9 @@ import z from "zod"; import { PaymentProviderSchema } from "./payment"; -// Min 3 keeps names inside what @aws/agentcore-cdk's ConfigIO accepts (3-255) -// so `add` never produces a spec that `build` rejects; max 128 and the character -// set are the Identity service's own limits. +// Max length and character set are the Identity service's own limits. The min +// is temporarily raised above 1 so `add` never produces a name that the pinned +// @aws/agentcore-cdk rejects at `build`; drop it back to 1 once a release that +// aligns the two ships. See credentials-add-followups. export const CredentialNameSchema = z .string() .min(3, "Credential name must be at least 3 characters") diff --git a/src/testing/testIO.tsx b/src/testing/testIO.tsx index 48b799fee..527c32a57 100644 --- a/src/testing/testIO.tsx +++ b/src/testing/testIO.tsx @@ -16,6 +16,9 @@ export interface TestIO { export interface TestIOOptions { isTTY?: boolean; + // stdin pre-fills the input stream with piped content, then ends it. Use this + // to test handlers that read a secret or prompt from stdin ('-'). + stdin?: string; } // collect wraps a PassThrough, accumulating everything written to it as a string. @@ -33,10 +36,12 @@ function collect(): { stream: NodeJS.WriteStream; read: () => string } { // testIO builds a fresh in-memory TestIO for a single test. stdin is an idle // PassThrough (no input) so screens that read input simply see nothing. -export function testIO({ isTTY = false }: TestIOOptions = {}): TestIO { +export function testIO({ isTTY = false, stdin: stdinContent }: TestIOOptions = {}): TestIO { const out = collect(); const err = collect(); - const stdin = new PassThrough() as unknown as NodeJS.ReadStream; + const stdinStream = new PassThrough(); + if (stdinContent !== undefined) stdinStream.end(stdinContent); + const stdin = stdinStream as unknown as NodeJS.ReadStream; for (const stream of [stdin, out.stream, err.stream]) { Object.defineProperty(stream, "isTTY", { configurable: true, value: isTTY }); From 0b39fb96aa3856238af2039deb0bdfb4855acdc0 Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Fri, 21 Aug 2026 15:01:24 -0400 Subject: [PATCH 8/9] fix(project): address review on credentials add - readTextFile: read via node:fs so the node-target bundle works (Bun.file is a Bun-runtime global, absent under node); options.signal passes through - .env.local values are single-quoted so node:util parseEnv round-trips them byte-for-byte; reject values containing a single quote with a clear message - move spec validation inside the rollback try so a rejected spec reverses the staged .env.local write instead of leaving it behind - reject two credential names that derive the same environment variable - rename EnvLocalFile.upsert to insertIfNew (never overwrites) --- src/core/project/envLocal.test.ts | 34 +++++++++++--- src/core/project/envLocal.ts | 45 +++++++++++++------ src/core/project/manager.tsx | 18 ++++---- .../project/add/credentials/shared.ts | 16 +++++++ src/handlers/project/project.test.ts | 14 ++++-- src/io/fileRead.ts | 7 ++- 6 files changed, 99 insertions(+), 35 deletions(-) diff --git a/src/core/project/envLocal.test.ts b/src/core/project/envLocal.test.ts index c619a4eb0..44834806d 100644 --- a/src/core/project/envLocal.test.ts +++ b/src/core/project/envLocal.test.ts @@ -3,6 +3,7 @@ import { existsSync } from "node:fs"; import { mkdir, mkdtemp, rm } from "node:fs/promises"; import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; +import { parseEnv } from "node:util"; import { EnvLocalFile } from "./envLocal"; const roots: string[] = []; @@ -23,7 +24,7 @@ const ENTRY = { key: "SECRET", value: "v", comment: "c" }; test("rollback deletes the file it created", async () => { const root = await tempRoot(); const file = new EnvLocalFile(root); - await file.upsert([ENTRY]); + await file.insertIfNew([ENTRY]); expect(existsSync(file.path)).toBe(true); await file.rollback(); @@ -35,19 +36,42 @@ test("rollback restores the prior content of an existing file", async () => { const file = new EnvLocalFile(root); await Bun.write(file.path, "EXISTING=1\n"); - await file.upsert([ENTRY]); - expect(await Bun.file(file.path).text()).toContain("SECRET=v"); + await file.insertIfNew([ENTRY]); + expect(await Bun.file(file.path).text()).toContain("SECRET='v'"); await file.rollback(); expect(await Bun.file(file.path).text()).toBe("EXISTING=1\n"); }); -test("rollback is a no-op when upsert wrote nothing", async () => { +test("rollback is a no-op when insertIfNew wrote nothing", async () => { const root = await tempRoot(); const file = new EnvLocalFile(root); await Bun.write(file.path, "SECRET=kept\n"); - await file.upsert([ENTRY]); // key already present, so nothing is written + await file.insertIfNew([ENTRY]); // key already present, so nothing is written await file.rollback(); expect(await Bun.file(file.path).text()).toBe("SECRET=kept\n"); }); + +test.each([ + ["left#right", "left#right"], + [" padded ", " padded "], + ['has"double', 'has"double'], + ["back\\slash", "back\\slash"], + ["dollar$sign", "dollar$sign"], +])("a value with %p round-trips through parseEnv", async (value, expected) => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await file.insertIfNew([{ key: "SECRET", value, comment: "c" }]); + + const parsed = parseEnv(await Bun.file(file.path).text()) as Record; + expect(parsed.SECRET).toBe(expected); +}); + +test("rejects a value that contains a single quote", async () => { + const root = await tempRoot(); + const file = new EnvLocalFile(root); + await expect(file.insertIfNew([{ key: "SECRET", value: "a'b", comment: "c" }])).rejects.toThrow( + /single quote/, + ); +}); diff --git a/src/core/project/envLocal.ts b/src/core/project/envLocal.ts index 57427736b..5774dde36 100644 --- a/src/core/project/envLocal.ts +++ b/src/core/project/envLocal.ts @@ -1,6 +1,7 @@ import { rm } from "node:fs/promises"; import { join } from "node:path"; import { atomicWrite, readTextFile } from "../../io"; +import { InputValidationError } from "../../errors"; import type { EnvLocalEntry } from "../../handlers/project/types"; /** The project-relative path of the local secrets file (read by `agentcore dev`). */ @@ -9,15 +10,15 @@ export const ENV_LOCAL_RELATIVE_PATH = join("agentcore", ".env.local"); const KEY_LINE = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=/; /** - * The project's `.env.local` secrets file, edited transactionally. `upsert` + * The project's `.env.local` secrets file, edited transactionally. `insertIfNew` * appends entries (never overwriting an existing key) and snapshots the prior * state so `rollback` can undo the write if a later step in the same operation * fails. Mirrors the class shape of {@link SourceResolver} so callers hold one * object and reverse its effect, rather than tracking loose paths. */ export class EnvLocalFile { - // undefined: upsert has not written; null: file did not exist before the - // write; string: the file's content before the write. + // undefined: no write yet; null: file did not exist before the write; + // string: the file's content before the write. private snapshot?: string | null; constructor(private readonly rootPath: string) {} @@ -32,8 +33,8 @@ export class EnvLocalFile { * are left unchanged so user-managed values survive re-runs. Returns the keys * written and those skipped. */ - async upsert(entries: EnvLocalEntry[]): Promise<{ written: string[]; skipped: string[] }> { - const existing = await readOrNull(this.path); + async insertIfNew(entries: EnvLocalEntry[]): Promise<{ written: string[]; skipped: string[] }> { + const existing = await this.readOrNull(); const existingKeys = new Set( (existing ?? "") .split("\n") @@ -50,7 +51,8 @@ export class EnvLocalFile { continue; } const separator = content === "" || content.endsWith("\n") ? "" : "\n"; - content += `${separator}# ${entry.comment}\n${entry.key}=${entry.value ?? ""}\n`; + // Each entry is two lines: # \n= + content += `${separator}# ${entry.comment}\n${entry.key}=${formatValue(entry.value)}\n`; written.push(entry.key); } @@ -61,20 +63,35 @@ export class EnvLocalFile { return { written, skipped }; } - /** Restores the file to its pre-`upsert` state; a no-op when `upsert` wrote nothing. */ + /** Restores the file to its pre-write state; a no-op when nothing was written. */ async rollback(): Promise { if (this.snapshot === undefined) return; if (this.snapshot === null) await rm(this.path, { force: true }); else await atomicWrite(this.path, this.snapshot); } + + private async readOrNull(): Promise { + try { + return await readTextFile(this.path); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + } + } } -/** Reads a file, returning null when it does not exist so callers can tell empty from absent. */ -async function readOrNull(path: string): Promise { - try { - return await readTextFile(path); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; - throw error; +/** + * Single-quotes a value so `node:util`'s `parseEnv` reads it back byte-for-byte. + * Single quotes are literal in that parser, so no character needs escaping, + * except a single quote itself, which the format cannot represent. + */ +function formatValue(value?: string): string { + if (!value) return ""; + if (value.includes("'")) { + throw new InputValidationError( + "a secret value that contains a single quote (') cannot be written to " + + ".env.local; supply it with a Secrets Manager reference instead", + ); } + return `'${value}'`; } diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index b324907df..5504a2e8c 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -182,7 +182,7 @@ export class FsProjectManager implements ProjectManager { if (input.envEntries?.length) { envFile = new EnvLocalFile(project.rootPath); yield { message: `Updating secrets file at '${envFile.path}'` }; - const { skipped } = await envFile.upsert(input.envEntries); + const { skipped } = await envFile.insertIfNew(input.envEntries); for (const key of skipped) { yield { message: `'${key}' already exists in ${ENV_LOCAL_RELATIVE_PATH}; left unchanged`, @@ -205,20 +205,20 @@ export class FsProjectManager implements ProjectManager { yield { message: `Updating project spec file at '${agentCoreSpecPath}'` }; const newSpec = { ...existingProjectSpec, [projectSpecKey]: newResources }; - const newSpecParseResult = ProjectSpecSchema.safeParse(newSpec); - if (!newSpecParseResult.success) - throw new InputValidationError(z.prettifyError(newSpecParseResult.error), { - cause: newSpecParseResult.error, - }); - - // rollback scaffolding changes on failed config writes to prevent bad state. + // Validate and write inside the same boundary so a rejected spec rolls back + // staged side effects (.env.local, scaffolded files) rather than leaving them. let newProjectSpec: z.infer; try { + const newSpecParseResult = ProjectSpecSchema.safeParse(newSpec); + if (!newSpecParseResult.success) + throw new InputValidationError(z.prettifyError(newSpecParseResult.error), { + cause: newSpecParseResult.error, + }); newProjectSpec = await this.json.write(agentCoreSpecPath, newSpecParseResult.data); } catch (err) { this.logger.warn( - `failed to update ${agentCoreSpecPath}; attempting best-effort cleanup of staged changes`, + `could not commit the spec update to ${agentCoreSpecPath}; attempting best-effort cleanup of staged changes`, ); await Promise.all([ ...scaffoldedPaths.map((p) => diff --git a/src/handlers/project/add/credentials/shared.ts b/src/handlers/project/add/credentials/shared.ts index b744f8171..869f535f7 100644 --- a/src/handlers/project/add/credentials/shared.ts +++ b/src/handlers/project/add/credentials/shared.ts @@ -56,6 +56,22 @@ export async function addCredentialToProject( input: Omit, "resourceType">, ): Promise { const project = ctx.require(ProjectKey); + + // Two names that differ only by '-' vs '_' derive the same environment + // variable, which would silently reuse one secret for both providers. + const newName = input.resourceConfig.name; + const clash = project.spec.credentials.find( + (existing) => + existing.name !== newName && + credentialEnvVarName(existing.name) === credentialEnvVarName(newName), + ); + if (clash) { + throw new InputValidationError( + `credential '${newName}' and '${clash.name}' derive the same environment variable name; ` + + "choose a name that differs by more than '-' and '_'", + ); + } + for await (const event of config.projectManager.addResource(project, { resourceType: "credential", ...input, diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index 941b6eab5..87e7ec616 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -324,8 +324,8 @@ describe("project add credentials", () => { ]); const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); - expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123\n"); - expect(env).not.toContain("AGENTCORE_CREDENTIAL_SVC_KEY=sk-123\n\n"); + expect(env).toContain("AGENTCORE_CREDENTIAL_SVC_KEY='sk-123'\n"); + expect(env).not.toContain("AGENTCORE_CREDENTIAL_SVC_KEY='sk-123'\n\n"); }); test("api-key without a secret writes a commented placeholder and tells the user to fill it", async () => { @@ -403,7 +403,7 @@ describe("project add credentials", () => { ]); const env = await Bun.file(join(projectRoot, "agentcore", ".env.local")).text(); - expect(env).toContain("AGENTCORE_CREDENTIAL_IDP_CLIENT_SECRET=sssh"); + expect(env).toContain("AGENTCORE_CREDENTIAL_IDP_CLIENT_SECRET='sssh'"); }); test("oauth vendored with --provider-configuration records the config and a secret placeholder", async () => { @@ -472,6 +472,14 @@ describe("project add credentials", () => { ).rejects.toThrow(/already exists/); }); + test("rejects two names that derive the same environment variable", async () => { + await inProject(); + await run(["add", "credentials", "api-key", "--name", "svc-key"]); + await expect(run(["add", "credentials", "api-key", "--name", "svc_key"])).rejects.toThrow( + /same environment variable/, + ); + }); + test.each<[string, string[], RegExp]>([ [ "api-key: an inline secret value", diff --git a/src/io/fileRead.ts b/src/io/fileRead.ts index 804532cea..8d4a14f4e 100644 --- a/src/io/fileRead.ts +++ b/src/io/fileRead.ts @@ -1,3 +1,5 @@ +import { readFile } from "node:fs/promises"; + export type ReadTextFileOptions = { signal?: AbortSignal; }; @@ -6,8 +8,5 @@ export async function readTextFile( path: string, options: ReadTextFileOptions = {}, ): Promise { - options.signal?.throwIfAborted(); - const text = await Bun.file(path).text(); - options.signal?.throwIfAborted(); - return text; + return readFile(path, { encoding: "utf8", signal: options.signal }); } From a37b46d55da47ab9edd8aec8a4a07ef3d30be901 Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Fri, 21 Aug 2026 16:13:09 -0400 Subject: [PATCH 9/9] refactor(io): move secret-flag resolution onto SourceResolver The stdin-or-file-only rule now has a second caller (runtime, PR 2035), so resolveSecretFlag moves off the credentials shared module and onto SourceResolver as resolveSecret. It throws SourceResolutionError (already a subtype of InputValidationError), which keeps the io package from depending on handler-level error types. Also bump the generated CDK app template's aws-cdk-lib to ~2.266.0 so it satisfies the @aws/agentcore-cdk peer floor once the credential constructs release. --- src/assets/cdk/package.json | 2 +- .../project/add/credentials/api-key/index.ts | 9 ++----- .../project/add/credentials/oauth/index.ts | 13 ++-------- .../project/add/credentials/shared.ts | 26 ------------------- src/io/source.ts | 22 ++++++++++++++++ 5 files changed, 27 insertions(+), 45 deletions(-) diff --git a/src/assets/cdk/package.json b/src/assets/cdk/package.json index 550a52797..0ac28f946 100644 --- a/src/assets/cdk/package.json +++ b/src/assets/cdk/package.json @@ -24,7 +24,7 @@ }, "dependencies": { "@aws/agentcore-cdk": "0.1.0-alpha.45", - "aws-cdk-lib": "~2.261.0", + "aws-cdk-lib": "~2.266.0", "constructs": "~10.7.0" } } diff --git a/src/handlers/project/add/credentials/api-key/index.ts b/src/handlers/project/add/credentials/api-key/index.ts index 1674c8401..a1f0ae0a6 100644 --- a/src/handlers/project/add/credentials/api-key/index.ts +++ b/src/handlers/project/add/credentials/api-key/index.ts @@ -4,12 +4,7 @@ import { InputValidationError } from "../../../../../errors"; import { SourceResolver } from "../../../../../io"; import type { AddProjectResourceConfig } from "../../types"; import type { EnvLocalEntry } from "../../../types"; -import { - addCredentialToProject, - credentialEnvVarName, - parseExclusiveSecretRef, - resolveSecretFlag, -} from "../shared"; +import { addCredentialToProject, credentialEnvVarName, parseExclusiveSecretRef } from "../shared"; export const createAddApiKeyCredentialHandler = (config: AddProjectResourceConfig) => createHandler({ @@ -41,7 +36,7 @@ export const createAddApiKeyCredentialHandler = (config: AddProjectResourceConfi ); const resolver = new SourceResolver({ stdin: config.io.stdin }); - const apiKey = await resolveSecretFlag(resolver, "api-key", flags["api-key"]); + const apiKey = await resolver.resolveSecret("api-key", flags["api-key"]); const envEntries: EnvLocalEntry[] = secretRef ? [] diff --git a/src/handlers/project/add/credentials/oauth/index.ts b/src/handlers/project/add/credentials/oauth/index.ts index 31fb4f2c0..96c71fe58 100644 --- a/src/handlers/project/add/credentials/oauth/index.ts +++ b/src/handlers/project/add/credentials/oauth/index.ts @@ -9,12 +9,7 @@ import { } from "../../../../identity/oauth2-credential-provider/config"; import type { AddProjectResourceConfig } from "../../types"; import type { EnvLocalEntry } from "../../../types"; -import { - addCredentialToProject, - credentialEnvVarName, - parseExclusiveSecretRef, - resolveSecretFlag, -} from "../shared"; +import { addCredentialToProject, credentialEnvVarName, parseExclusiveSecretRef } from "../shared"; export const createAddOauthCredentialHandler = (config: AddProjectResourceConfig) => createHandler({ @@ -75,11 +70,7 @@ export const createAddOauthCredentialHandler = (config: AddProjectResourceConfig } const resolver = new SourceResolver({ stdin: config.io.stdin }); - const clientSecret = await resolveSecretFlag( - resolver, - "client-secret", - flags["client-secret"], - ); + const clientSecret = await resolver.resolveSecret("client-secret", flags["client-secret"]); const resourceConfig = mode.kind === "complete" diff --git a/src/handlers/project/add/credentials/shared.ts b/src/handlers/project/add/credentials/shared.ts index 869f535f7..a9a1445c9 100644 --- a/src/handlers/project/add/credentials/shared.ts +++ b/src/handlers/project/add/credentials/shared.ts @@ -1,6 +1,5 @@ import { ProjectKey, type Context } from "../../../../router"; import { InputValidationError } from "../../../../errors"; -import { SourceResolver } from "../../../../io"; import { parseSecretReference } from "../../../identity/parser"; import type { AddProjectResourceConfig } from "../types"; import type { AddResourceInput } from "../../types"; @@ -24,31 +23,6 @@ export function parseExclusiveSecretRef( return parseSecretReference(refFlag, refValue); } -/** - * Resolves a secret flag while refusing inline values: an inline secret leaks - * into shell history and process listings, so only stdin and files are allowed. - * A single trailing newline is stripped (echo and editors add one). - */ -export async function resolveSecretFlag( - resolver: SourceResolver, - name: string, - source: string | undefined, -): Promise { - if (source !== undefined && source !== "-" && !source.startsWith("file://")) { - throw new InputValidationError( - `--${name} must come from stdin ('-') or a file ('file://'); ` + - "inline secret values are not accepted", - ); - } - const value = await resolver.resolveText(name, source); - if (value === undefined) return undefined; - const normalized = value.replace(/\r?\n$/, ""); - if (normalized.includes("\n")) { - throw new InputValidationError(`--${name} must be a single-line value`); - } - return normalized; -} - /** Runs the shared add flow: spec update, env entries, progress, and fill-before-deploy notice. */ export async function addCredentialToProject( ctx: Context, diff --git a/src/io/source.ts b/src/io/source.ts index 60ad1e952..69bae8401 100644 --- a/src/io/source.ts +++ b/src/io/source.ts @@ -45,6 +45,28 @@ export class SourceResolver { } } + /** + * Resolves a secret-bearing flag while refusing inline values: an inline + * secret leaks into shell history and process listings, so only stdin and + * files are allowed. A single trailing newline is stripped (echo and editors + * add one) and an embedded newline is rejected. + */ + async resolveSecret(name: string, source: string | undefined): Promise { + if (source !== undefined && source !== STDIN && !source.startsWith(FILE_PREFIX)) { + throw new SourceResolutionError( + `--${name} must come from stdin ('-') or a file ('file://'); ` + + "inline secret values are not accepted", + ); + } + const value = await this.resolveText(name, source); + if (value === undefined) return undefined; + const normalized = value.replace(/\r?\n$/, ""); + if (normalized.includes("\n")) { + throw new SourceResolutionError(`--${name} must be a single-line value`); + } + return normalized; + } + private async readStdin(name: string): Promise { if (this.stdinClaimedBy !== undefined) { throw new SourceResolutionError(