Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

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

Aperta
  • Qualcosa non va?
#739 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
20/100
Tipo di issue
Refactoring
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
typescript
Ambito
cli

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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)

Lingua principale
TypeScript
Stelle
49
Fork
27
Merge medio
1g 18h
PR unite (30g)
65

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di elastic/cli

Tutte le issue di elastic/cli

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.