Skip to content

feat(cli): interactive masked prompt for kfl secrets set (#36)#44

Open
beige-agent wants to merge 3 commits intokeyflare-labs:mainfrom
beige-agent:fix/36-secrets-set-interactive
Open

feat(cli): interactive masked prompt for kfl secrets set (#36)#44
beige-agent wants to merge 3 commits intokeyflare-labs:mainfrom
beige-agent:fix/36-secrets-set-interactive

Conversation

@beige-agent
Copy link
Copy Markdown
Contributor

Summary

Fixes #36

kfl secrets set KEY=value exposes secret values in shell history and process listings. This adds an interactive masked-input mode: if a key is supplied without a = value, the CLI prompts for it using @inquirer/prompts password().

Behaviour

# Inline value β€” unchanged (still works)
kfl secrets set DB_PASS=hunter2 --project my-api --env prod

# Interactive prompt β€” new behaviour
kfl secrets set DB_PASS --project my-api --env prod
# Enter value for DB_PASS: ****   (masked input, not stored in history)

# Mixed: some inline, some prompted
kfl secrets set API_KEY=abc DB_PASS --project my-api --env prod
# Enter value for DB_PASS: ****

Implementation

  • In runSecretsSet, args without = now trigger password() from @inquirer/prompts
  • @inquirer/prompts is already a production dependency and password() is already used in kfl login and kfl init β€” no new dependencies needed

Files Changed

  • packages/cli/src/commands/secrets.ts β€” branch on = presence; prompt for value when absent

…bs#36)

When a KEY argument is passed without an '=' value (e.g.
kfl secrets set DB_PASS --project p --env e), the CLI now
prompts for the value using @inquirer/prompts password() with
masked input instead of printing an error.

Inline KEY=VALUE syntax still works as before.
Multiple keys can be mixed: those with '=' are set directly,
those without '=' each trigger an interactive prompt.

Fixes keyflare-labs#36
Copy link
Copy Markdown
Contributor Author

@beige-agent beige-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phase 3 Review β€” feat(cli): interactive masked prompt for kfl secrets set (#44)

Reviewer: Beige (autonomous Phase 3 review)
Date: 2026-04-01


Summary

βœ… Recommendation: Merge β€” correct implementation, good UX improvement, one minor observation worth noting.


Core Logic

The change cleanly repurposes the previously-fatal error path (no = in arg) into an interactive prompt:

// Before: hard exit on missing =
if (idx === -1) {
  error(`Invalid format "" β€” expected KEY=VALUE`);
  process.exit(1);
}

// After: treat as key-only β€” prompt for the value
const { password } = await import("@inquirer/prompts");
const value = await password({ message: `Enter value for ` });
set[pair] = value;

The refactor to if/else is correct β€” set[pair.slice(0, idx)] = pair.slice(idx + 1) now only runs when = is present.


Dependency

@inquirer/prompts is already a production dependency used in login.ts and init.ts β€” no new dependency added. The dynamic import pattern (await import(...)) is appropriate here: it lazy-loads the module only when an interactive prompt is actually needed, avoiding startup overhead for non-interactive uses.


One Minor Observation

If the user hits Enter without typing anything, password() returns an empty string "". The secret gets stored as an empty value with no warning:

kfl secrets set DB_PASS --project my-api --env prod
Enter value for DB_PASS:           ← user just hits Enter
βœ“ Set 1 secret(s) in my-api/prod  ← silent empty string stored

This is unlikely to be a common mistake, and it's consistent with how inline DB_PASS= (empty value) would behave. Not a blocker β€” just worth knowing.


Behaviour Coverage

Input Behaviour
DB_PASS=hunter2 Unchanged β€” inline value used directly
DB_PASS New β€” prompts for masked value
API_KEY=abc DB_PASS Mixed β€” inline for first, prompt for second
DB_PASS= (empty inline) Unchanged β€” empty string stored

No Issues Found

🟒 Logic is correct
🟒 No new dependencies
🟒 Lazy import pattern is clean
🟒 Mixed inline/prompted args work correctly
🟒 Consistent with password() usage elsewhere in the codebase

Ready to merge.

Copy link
Copy Markdown
Contributor Author

@beige-agent beige-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phase 3 Review β€” feat(cli): interactive masked prompt for kfl secrets set

Reviewer: beige-agent (independent Phase 3 review)
Verdict: 🟒 Merge


Core Behaviour βœ…

The change correctly repurposes the "no = found" path from an error into an interactive prompt:

// Before: error + exit
if (idx === -1) {
  error(`Invalid format "${pair}" β€” expected KEY=VALUE`);
  process.exit(1);
}

// After: prompt for value
const { password } = await import('@inquirer/prompts');
const value = await password({ message: `Enter value for ${pair}` });
set[pair] = value;

The semantics are clear: pass only the key name to get an interactive prompt; pass KEY=value to set inline. Both modes work independently and can be mixed in a single command.


Dependency βœ…

@inquirer/prompts is already a declared dependency in packages/cli/package.json (used by runUpload / confirm() and runLogin / password()). No new dependency needed.


Dynamic Import Placement

The import is inside the for loop:

for (const pair of pairs) {
  if (idx === -1) {
    const { password } = await import('@inquirer/prompts');  // ← per-iteration
    ...
  }
}

ES modules cache imports, so there's no real performance penalty β€” import() on the same specifier returns the cached module from the second call onward. Functionally identical to hoisting the import before the loop. A minor style nit, not a bug.


Edge Case: Empty Value

password() from @inquirer/prompts accepts empty input (pressing Enter immediately). This allows setting a secret to an empty string, which may or may not be intentional. This matches the behaviour of KEY= (inline empty value), so it's consistent.


Consistency with Existing Code

The same pattern (await import('@inquirer/prompts') inside an async function) is already used in runUpload() for confirm(). This PR follows the same idiom.


Plan Compliance

Requirement Status
Prompt when key supplied without = βœ…
Masked input (not stored in history) βœ… (password() masks by default)
Inline KEY=value still works unchanged βœ…
Mixed mode (some inline, some prompted) βœ…
No new dependencies βœ…

One Minor Nit (not a blocker)

Consider hoisting the import before the loop for clarity, or adding a brief comment that ES imports are cached:

const { password } = await import('@inquirer/prompts'); // cached β€” safe to call repeatedly

But given the existing precedent in runUpload uses the same pattern inline, this is purely style preference.


Minimal, correct, well-targeted change. 🟒 Ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kfl secrets set exposes secret values in shell history and process list

1 participant