Harden auth + finance redaction (sh-security-review confirmed mediums)

Two confirmed medium findings from the agentic security review:

- Fail-open SH_MCP_ENV: config defaulted to 'local' when the var was unset,
  so a deploy that forgot SH_MCP_ENV=aws would silently run LocalAuthProvider
  and accept static dev bearer tokens (dev-finance-admin -> finance:admin).
  Now fail-closed: SH_MCP_ENV must be explicitly 'local' or 'aws' or the
  server refuses to start. Plus an independent guard in LocalAuthProvider
  that refuses to construct in an AWS runtime (AWS_LAMBDA_FUNCTION_NAME /
  AWS_EXECUTION_ENV present), regardless of the env flag.

- Finance egress redaction gap: redactDeep only wholesale-masked a sensitive
  key when its value was a scalar; an object/array under a sensitive key was
  recursed into, letting a bare nested value (e.g. {account:{number:...}})
  escape the keyword-gated pattern matcher. Now the entire subtree under a
  sensitive key is masked. No current finance tool emitted such shapes (all
  flat strings), so this closes a latent hole in the universal safety net.

+4 tests (subtree redaction, AWS-runtime guard). 411 pass; coverage gate green.

Review also produced lows (memo free-text digits, unsalted argsHash,
unauth /openapi.json by-design, session-cap no-reset by-design) tracked
separately; 0 confirmed critical/high — review verdict PASS.

packages/shared/src/dispatch.test.ts

@@ -276,4 +276,18 @@ describe('redactDeep', () => {
     expect(redactDeep(null)).toBe(null);
     expect(redactDeep(true)).toBe(true);
   });
+
+  it('masks the ENTIRE subtree when a sensitive key holds an object or array (no nested escape)', () => {
+    // A bare value nested under a sensitive key has no keyword context, so it
+    // would slip past the pattern matcher if redactDeep recursed. The whole
+    // subtree must be masked instead.
+    const out = redactDeep({
+      account: { number: '021000021', branch: 'main' },
+      cardNumber: ['4111111111111111', '5500005555555559'],
+    }) as Record<string, unknown>;
+    expect(out['account']).toBe('[REDACTED]');
+    expect(out['cardNumber']).toBe('[REDACTED]');
+    expect(JSON.stringify(out)).not.toContain('021000021');
+    expect(JSON.stringify(out)).not.toContain('4111111111111111');
+  });
 });

packages/shared/src/dispatch.ts

@@ -137,7 +137,13 @@ export function redactDeep(value: unknown): unknown {
   if (value !== null && typeof value === 'object') {
     const out: Record<string, unknown> = {};
     for (const [key, v] of Object.entries(value as Record<string, unknown>)) {
-      if (SENSITIVE_FIELD_RE.test(key) && v !== null && v !== undefined && typeof v !== 'object') {
+      if (SENSITIVE_FIELD_RE.test(key) && v !== null && v !== undefined) {
+        // Key looks sensitive → mask the ENTIRE value wholesale, whether it is a
+        // scalar, an array, or a nested object. Recursing into a non-scalar here
+        // would lose the keyword context redact() needs, letting a bare nested
+        // value (e.g. { account: { number: "021000021" } }) escape unmasked.
+        // Over-masking on the finance tier is the correct trade: a false negative
+        // is a PII leak (design.md §2.5).
         out[key] = '[REDACTED]';
       } else {
         out[key] = redactDeep(v);

packages/shared/src/local-auth.test.ts

@@ -17,6 +17,26 @@ function bearer(token: string) {
 }
 
 describe('LocalAuthProvider safety', () => {
+  it('refuses to construct inside an AWS runtime even when env=local', () => {
+    for (const v of ['AWS_LAMBDA_FUNCTION_NAME', 'AWS_EXECUTION_ENV']) {
+      const prev = process.env[v];
+      process.env[v] = 'sh-mcp-finance';
+      try {
+        expect(
+          () =>
+            new LocalAuthProvider({
+              audience: OPS_AUDIENCE,
+              principals: defaultLocalPrincipals(),
+              env: 'local',
+            }),
+        ).toThrow(/refuses to run inside an AWS/);
+      } finally {
+        if (prev === undefined) delete process.env[v];
+        else process.env[v] = prev;
+      }
+    }
+  });
+
   it('refuses to construct unless SH_MCP_ENV=local', () => {
     expect(
       () =>

packages/shared/src/local-auth.ts

@@ -97,6 +97,17 @@ export class LocalAuthProvider implements AuthProvider {
           `(got SH_MCP_ENV=${JSON.stringify(config.env)}). It must never run in production.`,
       );
     }
+    // Defense in depth, independent of the env flag: refuse to run in a real AWS
+    // runtime. The env check above can be defeated by a misconfiguration that
+    // resolves env to 'local' in a deployed context; this positive prod signal
+    // (set by Lambda / the AWS runtime) cannot. Static dev tokens must never
+    // authenticate anywhere AWS is executing this code.
+    if (process.env['AWS_LAMBDA_FUNCTION_NAME'] || process.env['AWS_EXECUTION_ENV']) {
+      throw new Error(
+        'LocalAuthProvider refuses to run inside an AWS Lambda/execution context ' +
+          '(AWS_LAMBDA_FUNCTION_NAME / AWS_EXECUTION_ENV present). Dev auth is local-only.',
+      );
+    }
     this.audience = config.audience;
     this.principals = config.principals;
   }

servers/sh-mcp-finance/src/config.ts

@@ -28,9 +28,15 @@ function readEnv(name: string): string | undefined {
 }
 
 export function loadFinanceConfig(): FinanceConfig {
-  const rawEnv = readEnv('SH_MCP_ENV') ?? 'local';
+  // Fail CLOSED: SH_MCP_ENV must be set explicitly. An unset value must NEVER
+  // silently select local mode (LocalAuthProvider + static dev bearer tokens) on
+  // the finance service. A misconfigured deploy refuses to start.
+  const rawEnv = readEnv('SH_MCP_ENV');
   if (rawEnv !== 'local' && rawEnv !== 'aws') {
-    throw new Error(`SH_MCP_ENV must be "local" or "aws" (got "${rawEnv}").`);
+    throw new Error(
+      `SH_MCP_ENV must be explicitly set to "local" or "aws" ` +
+        `(got ${rawEnv === undefined ? 'unset' : `"${rawEnv}"`}); refusing to start.`,
+    );
   }
   const env = rawEnv;
   const port = Number(readEnv('PORT') ?? '8082');

servers/sh-mcp-ops/src/config.ts

@@ -32,9 +32,16 @@ function readEnv(name: string): string | undefined {
 
 /** Parse and validate the process environment into an {@link OpsConfig}. */
 export function loadOpsConfig(): OpsConfig {
-  const rawEnv = readEnv('SH_MCP_ENV') ?? 'local';
+  // Fail CLOSED: SH_MCP_ENV must be set explicitly. An unset value must NEVER
+  // silently select local mode (which wires LocalAuthProvider + static dev
+  // bearer tokens). A misconfigured deploy should refuse to start, not run dev
+  // auth on a finance/ops service.
+  const rawEnv = readEnv('SH_MCP_ENV');
   if (rawEnv !== 'local' && rawEnv !== 'aws') {
-    throw new Error(`SH_MCP_ENV must be "local" or "aws" (got "${rawEnv}").`);
+    throw new Error(
+      `SH_MCP_ENV must be explicitly set to "local" or "aws" ` +
+        `(got ${rawEnv === undefined ? 'unset' : `"${rawEnv}"`}); refusing to start.`,
+    );
   }
   const env = rawEnv;
   const port = Number(readEnv('PORT') ?? '8081');