From e6a22f3b9b49bbcfe38a1ba9950ffec952bf78a5 Mon Sep 17 00:00:00 2001 From: ridercoder4 Date: Sun, 18 Oct 2026 13:39:32 +0100 Subject: [PATCH] test(config,deploy): map the secret field-name and raw StrKey rejection boundaries SW-081 (#181): the config secret guard is one regex, /(?:secret|private[_-]?key|seed|mnemonic|password|passphrase|token)/i, and the existing suite covers fifteen names without ever varying casing or separator style. Two properties were unpinned: nothing asserted that casing is irrelevant, so dropping the /i flag would leave the suite green; and only private[_-]?key permits a separator inside a term, so every other name matches on its head word alone. Adds 41 recognised names across casing, separator style and scope prefixes, each exercised through validateStellarForgeConfig and through the real loadProjectConfig path, asserting the value is never reflected. Also pins the boundary the guard does NOT cover - pass_phrase, encryptionKey, signingKey and similar are refused as ordinary unsupported fields rather than as secret material. They are still refused before use, just via the generic path, so the asymmetry is asserted rather than left to be discovered. SW-086 (#186): validateIdentityAlias is the single gate for both deploy --source and config identity. The existing rejection test proves the rule fires with two cases but not where its edge is, and the edge is asymmetric - a canonical 56-char G/S StrKey is rejected, while a near-miss (wrong length, lowercased, muxed M, contract C, or a tail carrying 0/1/8/-/outside base32) is accepted as an ordinary alias and forwarded to the Stellar CLI as an identity name. No secret is exposed there since none of those values is a usable seed, but it is worth pinning from both sides so widening the regex has to be deliberate. Adds 5 canonical shapes, 14 alias-rule rejections, the exact 64/65-character boundary, non-reflection of every rejected value, and rejection before any subprocess is planned. The regex was left unchanged - narrowing it is a behaviour change beyond a test issue and could reject legitimate aliases. --- .../config-secret-field-name-variants.test.ts | 298 ++++++++++++++++++ tests/deploy-strkey-rejection-matrix.test.ts | 252 +++++++++++++++ 2 files changed, 550 insertions(+) create mode 100644 tests/config-secret-field-name-variants.test.ts create mode 100644 tests/deploy-strkey-rejection-matrix.test.ts diff --git a/tests/config-secret-field-name-variants.test.ts b/tests/config-secret-field-name-variants.test.ts new file mode 100644 index 0000000..10b2b3c --- /dev/null +++ b/tests/config-secret-field-name-variants.test.ts @@ -0,0 +1,298 @@ +/** + * Secret-looking configuration field-name rejection matrix (SW-081, #181). + * + * `configuration-secret-reflection.test.ts` already covers fifteen field names, + * but they cluster into a handful of shapes — `privateKey` / `private_key` / + * `private-key`, `secretKey` / `secret`, `token` / `apiToken` / `authToken` / + * `my_token` — and the matrix never varies casing or separator style across the + * whole set. + * + * That matters because the guard is a single regex (src/config/schema.ts): + * + * /(?:secret|private[_-]?key|seed|mnemonic|password|passphrase|token)/i + * + * Two properties are worth pinning explicitly, because changing either one is a + * silent regression: + * + * 1. **Casing is irrelevant.** The `/i` flag means `SECRET_KEY`, `SecretKey` and + * `sEcReTkEy` are all rejected. Nothing asserts this, so dropping the flag + * would be invisible to the suite. + * 2. **Separator style is irrelevant for most terms.** Only `private[_-]?key` + * permits a separator *inside* the term. Every other alternative is matched + * by its head word alone — `secretKey`, `seedPhrase` and `apiToken` match on + * `secret`, `seed` and `token`. The last group below documents where that + * leaves the boundary, which is the useful part of the map. + */ + +import { writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { validateStellarForgeConfig } from '../src/config/schema.js'; +import { + loadProjectConfig, + PROJECT_CONFIG_FILENAME, +} from '../src/config/project.js'; +import { ValidationCliError } from '../src/errors/index.js'; +import { withTempDirectory } from './helpers/index.js'; + +const SECRET_REJECTION = 'must not contain secret material'; +const UNSUPPORTED_FIELD = + 'stellarforge.config.json contains an unsupported field.'; + +/** + * Names the guard does recognise. Each differs from the existing suite only by + * casing, separator style, or a scope prefix. + */ +const RECOGNISED_SECRET_NAMES = [ + // Casing — the `/i` flag is what makes these equivalent to the plain forms. + 'SECRET', + 'Secret', + 'SECRET_KEY', + 'SecretKey', + 'sEcReTkEy', + 'PRIVATEKEY', + 'PrivateKey', + 'SEED', + 'SeedPhrase', + 'MNEMONIC', + 'Mnemonic', + 'PASSWORD', + 'PassPhrase', + 'Token', + 'TOKEN', + 'ACCESS_TOKEN', + 'RefreshToken', + + // Separator style across the head-word terms. `private[_-]?key` is the only + // alternative that accepts a separator inside the term. + 'secret_key', + 'secret-key', + 'secretkey', + 'api_token', + 'api-token', + 'refresh_token', + 'service_token', + 'auth_token', + 'seed_phrase', + 'seed-phrase', + 'seedphrase', + 'privatekey', + 'private_key', + 'private-key', + + // Scope prefixes a real config might drift toward. + 'stellarforge_secret', + 'stellarforge-token', + 'signingSecret', + 'signing_secret', + 'signingToken', + 'credentialSecret', + 'bearerToken', + 'accessSecret', + 'deploymentToken', + 'privateSecretKeyBase', +] as const; + +/** + * Secret-adjacent names the guard does *not* recognise. `passphrase` is the only + * multi-word alternative in the regex and it tolerates no separator, and there is + * no alternative matching a bare `key`. These are therefore rejected as ordinary + * unsupported fields — still refused before use, but with the generic message + * rather than the secret-specific one. + * + * Asserted deliberately: if the regex is ever widened to catch them, this test + * fails and the change has to be made on purpose. + */ +const SECRET_ADJACENT_UNRECOGNISED_NAMES = [ + 'pass_phrase', + 'pass-phrase', + 'encryptionKey', + 'encryption_key', + 'signingKey', + 'signing-key', + 'signing_key', + 'hdAccountIndex', +] as const; + +function captureValidationError(value: unknown): ValidationCliError { + try { + validateStellarForgeConfig(value); + } catch (error: unknown) { + expect(error).toBeInstanceOf(ValidationCliError); + return error as ValidationCliError; + } + + throw new Error('Expected configuration validation to fail.'); +} + +describe('secret field-name variants are rejected (#181)', () => { + it.each(RECOGNISED_SECRET_NAMES)( + 'rejects %j as secret material, not as an unknown field', + (field) => { + const error = captureValidationError({ + version: 1, + [field]: 'irrelevant-value', + }); + + expect(error.message).toContain(SECRET_REJECTION); + expect(error.message).not.toContain('unsupported field'); + }, + ); + + it.each(RECOGNISED_SECRET_NAMES)( + 'never reflects the value of %j', + (field) => { + const sentinel = 'SENTINEL9-do-not-log'; + const error = captureValidationError({ version: 1, [field]: sentinel }); + + expect(error.message).toContain(SECRET_REJECTION); + expect(error.message).not.toContain(sentinel); + // The field name is not secret material, and is not echoed either. + expect(error.message).not.toContain(field); + }, + ); + + it.each(RECOGNISED_SECRET_NAMES)( + 'rejects %j through the on-disk load path too', + (field) => + withTempDirectory((root: string) => { + const sentinel = 'SENTINEL9-do-not-log'; + writeFileSync( + join(root, PROJECT_CONFIG_FILENAME), + JSON.stringify({ version: 1, [field]: sentinel }), + 'utf8', + ); + + const error = captureLoadError(root); + + expect(error.message).toContain(SECRET_REJECTION); + expect(error.message).not.toContain(sentinel); + }), + ); + + it('rejects a secret-named field even when its value is empty', () => { + const error = captureValidationError({ version: 1, apiToken: '' }); + + expect(error.message).toContain(SECRET_REJECTION); + }); + + it('rejects a secret-named field before the version check runs', () => { + // Field-name rejection happens first, so a malformed version cannot mask a + // secret-bearing key. + const error = captureValidationError({ version: 99, api_token: 'x' }); + + expect(error.message).toContain(SECRET_REJECTION); + }); + + it('rejects a secret-named field that is the only field, with no version', () => { + const error = captureValidationError({ secretKey: 'x' }); + + expect(error.message).toContain(SECRET_REJECTION); + }); +}); + +describe('the guard boundary is explicit (#181)', () => { + it.each(SECRET_ADJACENT_UNRECOGNISED_NAMES)( + 'refuses %j as an unsupported field rather than as secret material', + (field) => { + const error = captureValidationError({ + version: 1, + [field]: 'irrelevant-value', + }); + + // Still refused before the config is usable — just via the generic path. + expect(error.message).toBe(UNSUPPORTED_FIELD); + }, + ); + + it.each(SECRET_ADJACENT_UNRECOGNISED_NAMES)( + 'never reflects the value of %j', + (field) => { + const sentinel = 'SENTINEL9-do-not-log'; + const error = captureValidationError({ version: 1, [field]: sentinel }); + + expect(error.message).toBe(UNSUPPORTED_FIELD); + expect(error.message).not.toContain(sentinel); + }, + ); + + it('refuses a secret hidden one level down as an unsupported field', () => { + // The guard walks top-level keys only, so a nested secret cannot slip through. + const error = captureValidationError({ + version: 1, + nested: { apiToken: 'SENTINEL9-do-not-log' }, + }); + + expect(error.message).toBe(UNSUPPORTED_FIELD); + expect(error.message).not.toContain('SENTINEL9-do-not-log'); + }); +}); + +describe('legitimate schema fields remain accepted (#181)', () => { + it('accepts the minimal config', () => { + expect(validateStellarForgeConfig({ version: 1 })).toEqual({ version: 1 }); + }); + + it.each(['testnet', 'futurenet', 'mainnet'] as const)( + 'accepts network %s', + (network) => { + expect(validateStellarForgeConfig({ version: 1, network })).toEqual({ + version: 1, + network, + }); + }, + ); + + it.each([ + 'deployer', + 'team-deployer', + 'team_deployer', + 'Alice', + 'a1', + ])('accepts the named identity alias %j', (identity) => { + expect(validateStellarForgeConfig({ version: 1, identity })).toEqual({ + version: 1, + identity, + }); + }); + + it('accepts a fully populated valid config', () => { + expect( + validateStellarForgeConfig({ + version: 1, + network: 'testnet', + identity: 'deployer', + }), + ).toEqual({ version: 1, network: 'testnet', identity: 'deployer' }); + }); + + it('loads a valid config from disk', () => + withTempDirectory((root: string) => { + writeFileSync( + join(root, PROJECT_CONFIG_FILENAME), + JSON.stringify({ + version: 1, + network: 'testnet', + identity: 'deployer', + }), + 'utf8', + ); + + expect(loadProjectConfig(root)).toEqual({ + version: 1, + network: 'testnet', + identity: 'deployer', + }); + })); +}); + +function captureLoadError(root: string): ValidationCliError { + try { + loadProjectConfig(root); + } catch (error: unknown) { + expect(error).toBeInstanceOf(ValidationCliError); + return error as ValidationCliError; + } + + throw new Error('Expected configuration loading to fail.'); +} diff --git a/tests/deploy-strkey-rejection-matrix.test.ts b/tests/deploy-strkey-rejection-matrix.test.ts new file mode 100644 index 0000000..3642882 --- /dev/null +++ b/tests/deploy-strkey-rejection-matrix.test.ts @@ -0,0 +1,252 @@ +/** + * Raw StrKey rejection matrix for deploy and config identity (SW-086, #186). + * + * `validateIdentityAlias` (src/config/schema.ts) is the single gate in front of + * both the deploy `--source` and the config `identity`. It rejects a value when + * either rule matches: + * + * STELLAR_ACCOUNT_OR_SECRET_STRKEY /^[GS][A-Z2-7]{55}$/ + * !SAFE_IDENTITY_ALIAS /^[A-Za-z][A-Za-z0-9_-]{0,63}$/ + * + * The first rule is what stops a raw account address or secret seed being passed + * through as an identity name. The existing `deploy-source-rejection.test.ts` + * covers it with two cases — `'S' + 'A'.repeat(55)` and `'G' + 'A'.repeat(55)` — + * which is enough to prove the rule fires but not enough to describe where its + * edge is. + * + * That edge is genuinely asymmetric, and this file pins it from both sides so a + * future change to either regex has to be deliberate: + * + * - a canonical 56-character `G…`/`S…` StrKey is rejected; + * - a *near-miss* — wrong length, lowercased, mixed-case, or carrying a + * character outside the base32 alphabet — is not a valid key, so it is + * accepted as an ordinary alias and forwarded to the Stellar CLI as one. + * + * No secret material is exposed by the second group: none of those values is a + * usable seed, so the CLI simply fails to resolve them as an identity. It is + * worth knowing about, though, and it is asserted explicitly below rather than + * left to be discovered. + */ + +import { writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { validateIdentityAlias } from '../src/config/schema.js'; +import { resolveTestnetDeploymentPlan } from '../src/deployment/project.js'; +import { ValidationCliError } from '../src/errors/index.js'; +import { withTempDirectory } from './helpers/index.js'; + +const IDENTITY_REJECTION = + 'Stellar identity must be a named Stellar CLI identity alias.'; + +const BASE32 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'; + +/** A well-formed StrKey body of `length` characters drawn from the base32 set. */ +function strkeyBody(length: number): string { + return Array.from( + { length }, + (_, index) => BASE32[index % BASE32.length]!, + ).join(''); +} + +const CANONICAL_ACCOUNT = `G${strkeyBody(55)}`; +const CANONICAL_SEED = `S${strkeyBody(55)}`; + +/** Canonical, uppercase, exactly 56 characters — the shapes that must be refused. */ +const CANONICAL_STRKEYS = [ + { name: 'G account address', value: CANONICAL_ACCOUNT }, + { name: 'S secret seed', value: CANONICAL_SEED }, + { name: 'G address over the full base32 alphabet', value: `G${BASE32.repeat(2).slice(0, 55)}` }, + { name: 'S seed over the full base32 alphabet', value: `S${BASE32.repeat(2).slice(0, 55)}` }, + { name: 'G address with digits 2-7', value: `G${'2'.repeat(55)}` }, +] as const; + +/** + * Rejected by the alias rule rather than the StrKey rule: these cannot be a key, + * and they cannot be an alias either. + */ +const NOT_AN_ALIAS = [ + { name: 'G-shaped value with a space', value: `G${strkeyBody(54)} ` }, + { name: 'G-shaped value with an equals sign', value: `G${strkeyBody(54)}=` }, + { name: 'G-shaped value with a tab', value: `G${strkeyBody(54)}\t` }, + { name: 'G-shaped value with a slash', value: `G${strkeyBody(54)}/x` }, + { name: '65 characters, one past the alias limit', value: `G${strkeyBody(64)}` }, + { name: 'leading digit', value: `1${strkeyBody(55)}` }, + { name: 'leading hyphen', value: `-${strkeyBody(55)}` }, + { name: 'leading dot', value: `.${strkeyBody(55)}` }, + { name: 'empty string', value: '' }, + { name: 'whitespace only', value: ' ' }, + { name: 'path traversal', value: '../identity' }, + { name: 'absolute path', value: '/etc/passwd' }, + { name: 'seed phrase', value: 'seed phrase words' }, + { name: 'shell fragment', value: 'name;rm -rf /' }, +] as const; + +/** + * StrKey-shaped values that are *not* valid keys and are therefore treated as + * plain aliases. Asserted explicitly so that widening the StrKey rule has to be a + * conscious decision rather than a side effect of editing the regex. + */ +const NEAR_MISS_STRKEYS = [ + { name: '55 characters, one short', value: `G${strkeyBody(54)}` }, + { name: '57 characters, one long', value: `G${strkeyBody(56)}` }, + { name: 'lower-cased G address', value: CANONICAL_ACCOUNT.toLowerCase() }, + { name: 'lower-cased S seed', value: CANONICAL_SEED.toLowerCase() }, + { name: 'mixed-case G address', value: `G${strkeyBody(54).toLowerCase()}` }, + { name: 'muxed M account', value: `M${strkeyBody(56)}` }, + { name: 'contract C address', value: `C${strkeyBody(56)}` }, + { name: 'G address with a 0, outside base32', value: `G${'0'.repeat(55)}` }, + { name: 'G address with a 1, outside base32', value: `G${'1'.repeat(55)}` }, + { name: 'G address with an 8, outside base32', value: `G${'8'.repeat(55)}` }, + { name: 'G address with hyphens', value: `G${'-'.repeat(55)}` }, + { name: 'S seed with underscores', value: `S${'_'.repeat(55)}` }, +] as const; + +/** Named aliases that must keep working — the regression the whole rule protects. */ +const SAFE_ALIASES = [ + 'deployer', + 'alice', + 'team-deployer', + 'team_deployer', + 'A1', +] as const; + +function captureIdentityError(value: string): ValidationCliError { + try { + validateIdentityAlias(value); + } catch (error: unknown) { + expect(error).toBeInstanceOf(ValidationCliError); + return error as ValidationCliError; + } + + throw new Error(`Expected ${JSON.stringify(value)} to be rejected.`); +} + +describe('canonical raw StrKeys are refused (#186)', () => { + it.each(CANONICAL_STRKEYS)('rejects a $name', ({ value }) => { + const error = captureIdentityError(value); + + expect(error.message).toContain(IDENTITY_REJECTION); + }); + + it.each(CANONICAL_STRKEYS)('never reflects a $name in the error', ({ value }) => { + const error = captureIdentityError(value); + + // The message is fixed text; nothing interpolates the rejected value. + expect(error.message).not.toContain(value); + expect(error.message).not.toContain(value.slice(1, 12)); + }); + + it('rejects a 56-character value on length alone, whatever the prefix', () => { + // Any G/S-prefixed 56-character uppercase base32 string hits the rule; the + // rule cannot distinguish a real key from a fabricated one, by design. + expect(() => validateIdentityAlias(`G${strkeyBody(55)}`)).toThrow( + IDENTITY_REJECTION, + ); + expect(() => validateIdentityAlias(`S${strkeyBody(55)}`)).toThrow( + IDENTITY_REJECTION, + ); + }); +}); + +describe('values that cannot be an alias are refused (#186)', () => { + it.each(NOT_AN_ALIAS)('rejects $name', ({ value }) => { + const error = captureIdentityError(value); + + expect(error.message).toContain(IDENTITY_REJECTION); + }); + + it.each(NOT_AN_ALIAS)('never reflects $name in the error', ({ value }) => { + const error = captureIdentityError(value); + + expect(error.message).not.toContain(value); + }); + + it('keeps the boundary at 64 characters', () => { + // 64 is the alias limit and is accepted; 65 is one past it and is not. + expect(validateIdentityAlias(`A${'a'.repeat(63)}`)).toBe(`A${'a'.repeat(63)}`); + expect(() => validateIdentityAlias(`A${'a'.repeat(64)}`)).toThrow( + IDENTITY_REJECTION, + ); + }); +}); + +describe('the StrKey rule boundary is explicit (#186)', () => { + it.each(NEAR_MISS_STRKEYS)('accepts a $name as a plain alias', ({ value }) => { + // None of these is a valid Stellar key, so nothing secret is exposed. They + // are forwarded as an identity name and the Stellar CLI fails to resolve + // them. Pinned so a widened rule fails loudly instead of by accident. + expect(validateIdentityAlias(value)).toBe(value); + }); +}); + +describe('named aliases keep working (#186)', () => { + it.each(SAFE_ALIASES)('accepts %j unchanged', (alias) => { + expect(validateIdentityAlias(alias)).toBe(alias); + }); +}); + +describe('deploy refuses a raw StrKey before spawning (#186)', () => { + it.each(CANONICAL_STRKEYS)( + 'rejects a $name before any subprocess is planned', + ({ value }) => + withTempDirectory((root: string) => { + // A valid workspace, so only the source can be the reason for failure. + writeFileSync(join(root, 'Cargo.toml'), '[workspace]\nmembers = []\n', 'utf8'); + + expect(() => + resolveTestnetDeploymentPlan({ + cwd: root, + network: 'testnet', + source: value, + }), + ).toThrow(IDENTITY_REJECTION); + }), + ); + + it.each(NOT_AN_ALIAS)( + 'rejects $name before any subprocess is planned', + ({ value }) => + withTempDirectory((root: string) => { + writeFileSync(join(root, 'Cargo.toml'), '[workspace]\nmembers = []\n', 'utf8'); + + expect(() => + resolveTestnetDeploymentPlan({ + cwd: root, + network: 'testnet', + source: value, + }), + ).toThrow(IDENTITY_REJECTION); + }), + ); + + it('plans a deployment for a named alias in a generated workspace', () => + withTempDirectory((root: string) => { + writeFileSync(join(root, 'Cargo.toml'), '[workspace]\nmembers = []\n', 'utf8'); + + const plan = resolveTestnetDeploymentPlan({ + cwd: root, + network: 'testnet', + source: 'deployer', + }); + + expect(plan).toHaveLength(1); + expect(plan[0]?.args).toContain('--source-account'); + expect(plan[0]?.args).toContain('deployer'); + // The alias is forwarded as its own argv entry, never joined into a string. + expect(Array.isArray(plan[0]?.args)).toBe(true); + })); + + it('does not check the source before the network target', () => + withTempDirectory((root: string) => { + // Ordering matters: a wrong network is reported on its own terms, so a user + // fixing one problem does not just walk into the next. + expect(() => + resolveTestnetDeploymentPlan({ + cwd: root, + network: 'mainnet', + source: CANONICAL_SEED, + }), + ).toThrow('supports only the explicit `--network testnet` target'); + })); +});