Review CLI configuration and secret handling

Review exercise · TypeScript

Advanced45 min

Review brief

A deployment CLI combines defaults, a project configuration file, environment variables, and command-line flags. It also caches effective nonsecret settings and logs a diagnostic summary to help troubleshoot CI runs.

Behavioral contract

Resolve ordinary settings with flags overriding environment variables, which override the project file, which overrides defaults.
The deployment token may come only from the `--token` flag or `DEPLOY_TOKEN`; a token in the project file is rejected.
Only `${CLI_REGION}` and `${CLI_PROFILE}` may be interpolated in file string values, and a referenced allowlisted variable must be present.
The local cache contains only endpoint, timeout, log level, region, and profile and is written with owner-only mode `0600`.
Diagnostic events may include setting source names but never secret values or a serialized resolved configuration.
Unresolved or disallowed interpolation fails before anything is cached or logged.

Review instructions

1Review the loader, dependency boundary, and tests for precedence and end-to-end secret flow.
2Prioritize no more than six findings and separate distinct exposure surfaces only when they need different remediation.

Learning objectives

• Trace values through layered configuration
• Keep secrets out of files, caches, and diagnostics
• Constrain environment interpolation explicitly

src/config-types.tsTypeScript

@@ -0,0 +1,25 @@

1+export interface CliConfig {

2+ endpoint: string;

3+ timeoutMilliseconds: number;

4+ logLevel: "debug" | "info" | "warn";

5+ region?: string;

6+ profile?: string;

7+ token?: string;

8+}

10+export type ConfigInput = Partial<CliConfig>;

11+

12+export interface ConfigDependencies {

13+ defaults: CliConfig;

14+ environment: Record<string, string | undefined>;

15+ readProjectConfig(): Promise<ConfigInput>;

16+ writeCache(path: string, contents: string, mode: number): Promise<void>;

17+ diagnostics: {

18+ info(message: string, fields: Record<string, unknown>): void;

19+ };

20+}

21+

22+export interface LoadConfigInput {

23+ flags: ConfigInput;

24+ dependencies: ConfigDependencies;

25+}

Reference solution

The findings below are ordered by priority.

Findings

1. Lower-priority file values override flags and environment variables

Location: src/load-config.ts:40-45

The spread order applies flags before environment and the project file last. A checked-in endpoint or timeout silently defeats an explicit CI flag, and a file token can replace DEPLOY_TOKEN.

Merge in increasing priority: defaults, validated file settings, validated environment settings, and flags last.

2. The project file is allowed to provide a deployment token

Location: src/config-types.ts:10 and src/load-config.ts:23-27,40-45

ConfigInput includes every CliConfig property, so project-file parsing accepts token and the generic merge uses it. This permits a long-lived credential in a project file even though only a flag or the dedicated environment variable is authorized.

Use distinct types and validation for file, environment, and flag inputs. Reject a file token before resolving any values.

3. The effective cache persists the secret with broad permissions

Location: src/load-config.ts:47-51

The complete resolved object, including token, is serialized to disk with mode 0644. Other local users can read the credential, and it remains after the process exits.

Construct an explicit nonsecret cache payload and write it with mode 0600; never serialize the token.

4. Diagnostics emit the resolved token

Location: src/load-config.ts:52-54

Passing the full configuration as a structured field sends the secret to whichever console, log collector, or CI artifact receives diagnostics. File permissions do not mitigate this separate exposure path.

Log only the names of selected sources or an explicitly redacted summary whose schema cannot contain secret values.

5. Interpolation can read any environment variable and hides missing values

Location: src/load-config.ts:3-19

The regular expression accepts arbitrary names, including unrelated credentials, and replaces an absent variable with an empty string. A malicious or mistaken project file can copy process secrets into ordinary settings, while typos silently produce malformed configuration.

Reject names outside CLI_REGION and CLI_PROFILE, and fail if an allowlisted reference is absent before caching or diagnostics.

6. Tests avoid precedence, interpolation, and every secret sink

Location: src/load-config.test.ts:5-30

The single test supplies only nonsecret file values and checks neither cached contents nor mode. It cannot detect reversed precedence, forbidden token sources, arbitrary interpolation, missing variables, diagnostic leakage, or invalid numeric environment input.

Add a source-precedence matrix and failure cases, then inspect exact cache and diagnostic arguments to prove that a recognizable token never leaves its authorized in-memory field.

Reasonable non-findings

Reading the project file asynchronously before merging is reasonable.
Caching effective nonsecret settings is explicitly supported and need not be removed.
A structured diagnostic event is preferable to string concatenation when its schema excludes secrets.