Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

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

Abierto
  • ¿Algo no está bien?
#739 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
20/100
Tipo de issue
Refactorización
Claridad
Bastante claro
Estado de actividad
Activo
Stack tecnológico
typescript
Área
cli

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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)

Lenguaje dominante
TypeScript
Estrellas
49
Forks
27
Merge medio
1 d 23 h
PR fusionados (30 d)
67

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de elastic/cli

Todos los issues de elastic/cli

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.