Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

refactor(config): deduplicate exec helpers between secret-store, writer and config-resolver

Open
  • Is something wrong?
#739 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
20/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
typescript
Domain
cli

Research direction

Start by diffing src/config/secret-store.ts, src/config/writer.ts and packages/config-resolver/src/index.ts to confirm the duplicated helpers (shellEscape, psEncodedCommand, PRINTABLE_ASCII_RE, execOpts, the _testSetExecSync/_testSetPlatform seams). The issue says not to start until #664 and the icacls-to-execFile change for writer.ts have landed, so re-check those first. Done means no helper body is defined in more than one file, existing tests pass with only import paths changed, and the lazy load in loader.ts is preserved.

Written by the indexing model from the issue text.

Description

Summary

Process-exec helpers are copy-pasted across three files. Diffed and confirmed byte-identical:

  • shellEscape and psEncodedCommand: src/config/secret-store.ts:88-95 and packages/config-resolver/src/index.ts:139-146.
  • PRINTABLE_ASCII_RE: secret-store.ts:57 and config-resolver/src/index.ts:184.
  • execOpts: config-resolver/src/index.ts:135-137 and src/config/writer.ts:59-61 are identical. secret-store.ts:77-86 is the same with an optional input.
  • _execSync / _platform plus the _testSetExecSync / _testSetPlatform seams: secret-store.ts:125-148, config-resolver/src/index.ts:132-133,341-351, writer.ts:42-57.

secret-store.ts also defines spawnOpts (L110-119). Its body is identical to execOpts (L77-86); only the return type differs. The security -w … doc comment at L101-106 describes the spawnSync(SETSID_PERL, …) call in MacKeychainStore.put (~L184-195), but it is attached to the SETSID_PERL constant.

Sequencing (do NOT start yet)

  1. After #664's fix. This comment proposes moving secret-store.ts to execFile/spawn argv. That deletes every POSIX shellEscape use in that file (L214, 239, 252, 283, 294) and probably execOpts/spawnOpts as they are today.
  2. After the icacls → execFile PR for writer.ts. writer.ts only uses _execSync for icacls (L238). Once that change lands, its execOpts copy and _testSetExecSync seam either go away or change shape.

What remains after both

These are estimates and depend on how #664 lands:

  • psEncodedCommand (4 lines) is still in both secret-store.ts (Windows Credential Manager: L313, 330, 342) and config-resolver (L297).
  • PRINTABLE_ASCII_RE (1 line) is in both.
  • shellEscape stays only in config-resolver (L215, 238, 264) unless the resolver also moves to argv. It would then no longer be duplicated.
  • The _testSetExecSync / _testSetPlatform seams (~10 lines) remain in each file that still shells out.

That leaves roughly 15-25 duplicated lines in total. This is low priority. Once the prerequisites land, re-run the diff first. If only psEncodedCommand and the regex are left, it may be cheaper to leave them and close this issue.

Proposal

  • Export psEncodedCommand (and PRINTABLE_ASCII_RE, plus the exec options builder if one survives) from @elastic/config-resolver, which is already a workspace dependency, and import them in secret-store.ts. Don't add a src/lib/proc.ts: the package can't import from src/, so the package has to be the source.
  • Keep it a cheap import. loader.ts:31,381 lazy-loads @elastic/config-resolver to keep the help path fast. secret-store.ts is only reached from config commands and cloud/credentials.ts, so a static import there should be fine. Check this against ARCHITECTURE.md before merging.
  • Collapse execOpts/spawnOpts into one builder if both still exist.
  • Move the security -w comment onto the spawnSync call it describes.

Acceptance criteria

  • No helper function body is defined in more than one of src/config/secret-store.ts, src/config/writer.ts, packages/config-resolver/src/index.ts.
  • At most one exec options builder in secret-store.ts.
  • The security -w comment sits next to the spawnSync call in MacKeychainStore.put.
  • No behavior change. Existing secret-store, writer and config-resolver tests pass unmodified, apart from import paths.
  • No regression in help-path startup time (the lazy-load in loader.ts is preserved).

Related

#664, plus the pending icacls → execFile PR for src/config/writer.ts.

authored by Pi Coding Agent (Claude Opus 5.5)

Dominant language
TypeScript
Stars
47
Forks
27
Avg merge
2d 6h
Merged PRs (30d)
52

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from elastic/cli

All issues in elastic/cli

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.