Hacktoberfest 2026：维护者为十月标记出来的 issue，仍然开放、适合新手。 浏览 Hacktoberfest issue

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

已关闭
  • 哪里不对？
#739 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
3/5
预计耗时
1-2 天
新手友好度
20/100
Issue 类型
重构
描述清晰度
基本清楚
活跃度
活跃
技术栈
typescript
领域
cli

调研方向

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.

由索引模型根据 Issue 内容生成。

描述

Stale

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)

主要语言
TypeScript
星标
53
派生
28
平均合并
23 小时 41 分钟
30 天内合并 PR
44

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

elastic/cli 的其他 Issue

查看 elastic/cli 的全部 Issue

相似的 Issue

更多 TypeScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。