From 004aa05085df87170eaa2791e3d8a80fa413f62a Mon Sep 17 00:00:00 2001 From: Turtle Date: Tue, 13 Oct 2026 15:37:16 +0800 Subject: [PATCH] fix(credentials-local): keep foreign big integers exact and cover reload warnings Re-serializing a document with a foreign integer beyond 2^53 no longer rounds it; the reload warning is tested; the GITLAB_TOKEN test no longer depends on the host environment; README and JSDoc describe the tolerated top-level keys. --- .../credentials-local/README.i18n.yaml | 8 +++--- .../credentials/credentials-local/README.md | 4 +-- .../credentials-local/README.zh.md | 4 +-- .../credentials-local/src/index.ts | 14 ++++++---- .../credentials-local/tests/local.spec.ts | 15 ++++++---- .../credentials-local/tests/watcher.spec.ts | 28 +++++++++++++++++++ 6 files changed, 53 insertions(+), 20 deletions(-) diff --git a/packages/credentials/credentials-local/README.i18n.yaml b/packages/credentials/credentials-local/README.i18n.yaml index bdc2048840..b4401964e7 100644 --- a/packages/credentials/credentials-local/README.i18n.yaml +++ b/packages/credentials/credentials-local/README.i18n.yaml @@ -30,8 +30,8 @@ en: a55fac148b62e007 zh: fdb3d19ccae676ee /deepseek-ai-dsh-credentials-local/use-this-package/the-credential-file: - en: cbd156a76595ee1f - zh: 171d3c8116699eea + en: b18c236af3c175ed + zh: 516c6bc7acfcf7e6 /deepseek-ai-dsh-credentials-local/use-this-package/who-can-read-the-file: en: 76bcb1dab6a112e2 zh: 42669953eef0fc53 @@ -42,8 +42,8 @@ en: 538553a9a48e2f93 zh: d981c42878e80762 /deepseek-ai-dsh-credentials-local/understand-the-implementation/design-philosophy: - en: 2f76cb96086897ce - zh: 6b75376681446446 + en: eaa1671bd6877d79 + zh: 50c56ac9293809f1 /deepseek-ai-dsh-credentials-local/understand-the-implementation/source-map: en: a83715807279a902 zh: fef1884e2a16bd46 diff --git a/packages/credentials/credentials-local/README.md b/packages/credentials/credentials-local/README.md index c7fb908b85..c3d15999be 100644 --- a/packages/credentials/credentials-local/README.md +++ b/packages/credentials/credentials-local/README.md @@ -106,7 +106,7 @@ records: kind: api-key # neither: the owner confirmed the ambient credential chain ``` -You can edit the file directly — the store reloads it automatically and picks up the change, including a key or record you delete. `refs` holds key values by environment-variable name; `records` holds per-plugin credentials by `/`, each tagged `api-key` or `grant`, whose grant payload the store keeps verbatim because only its owner can interpret it. Comments and the formatting of untouched entries survive product writes; a comment directly above an entry is that entry's note and is removed with it. An unknown top-level key, such as a token another tool wrote, is kept on disk and ignored with a startup warning, because only `refs` and `records` entries resolve. Inside the document anything else is refused loudly rather than silently ignored: a non-mapping root, a key that is not addressable in its space, a wrong-typed or empty value, an unknown record tag or field, duplicate keys, and malformed YAML all fail at startup, and on a live reload the last good content keeps serving with a warning. +You can edit the file directly — the store reloads it automatically and picks up the change, including a key or record you delete. `refs` holds key values by environment-variable name; `records` holds per-plugin credentials by `/`, each tagged `api-key` or `grant`, whose grant payload the store keeps verbatim because only its owner can interpret it. Comments and the formatting of untouched entries survive product writes; a comment directly above an entry is that entry's note and is removed with it. An unknown top-level key, such as a token another tool wrote, is kept on disk and ignored with a warning at startup and on each reload that changes the file, because only `refs` and `records` entries resolve. Inside the document anything else is refused loudly rather than silently ignored: a non-mapping root, a key that is not addressable in its space, a wrong-typed or empty value, an unknown record tag or field, duplicate keys, and malformed YAML all fail at startup, and on a live reload the last good content keeps serving with a warning. A key's value can be any text, multi-line values included — no quoting tricks needed. An empty value means "no key", which is why an empty string in the file is rejected: removing a key deletes it, it does not blank it. A `grant` payload must survive a JSON round trip, enforced on the way in and on the way out, so the store refuses a value it could not read back exactly as written. If the file on disk no longer parses, saving fails instead of overwriting content the product could not read. @@ -134,7 +134,7 @@ This section explains the design decisions behind the provider and points at the ### Design philosophy - **One honest precedence.** The managed store wins for every provider; launch environment, project `.env`, and harness-home `.env` are deterministic fallbacks in that order. -- **The document holds only credentials.** A versioned document with `refs` and `records` sections rather than a dotenv file: a store the harness owns and never materializes into the environment cannot also serve as the user's environment layer, which would shadow non-secret entries behind its precedence. +- **The document holds only credentials.** A foreign top-level key is tolerated but resolves nothing. A versioned document with `refs` and `records` sections rather than a dotenv file: a store the harness owns and never materializes into the environment cannot also serve as the user's environment layer, which would shadow non-secret entries behind its precedence. - **Writes patch, reloads replace.** Line edits preserve comments and untouched entries under the cross-process writer lock; reloads swap the parsed snapshot wholesale so a deleted entry never lingers in memory. - **Fail loud where trust is at stake.** Boot and reload reject a document that is unreadable, invalid, or readable beyond its owner; a live reload that fails keeps the last good snapshot and warns rather than taking the process down. diff --git a/packages/credentials/credentials-local/README.zh.md b/packages/credentials/credentials-local/README.zh.md index 63876ac3de..5249827cde 100644 --- a/packages/credentials/credentials-local/README.zh.md +++ b/packages/credentials/credentials-local/README.zh.md @@ -106,7 +106,7 @@ records: kind: api-key # neither: the owner confirmed the ambient credential chain ``` -你可以直接编辑该文件——存储会自动重载并接收变更，包括你删除的密钥或记录。`refs` 按环境变量名存放密钥值；`records` 按 `/` 存放各插件的凭据，每条都带 `api-key` 或 `grant` 标签，其中 grant 的 payload 由存储逐字保留，因为只有它的拥有者能解释。产品写入时会保留注释与未触及条目的排版；直接位于某条目上方的注释属于该条目的注解，会随它一起删除。未知的顶层键（例如其他工具写入的令牌）会保留在磁盘上并在启动时告警后忽略，因为只有 `refs` 与 `records` 中的条目会被解析。文档内其他内容都会被明确拒绝，而不是被静默忽略：非 mapping 的根、在其键空间内不可寻址的键、类型错误或空的值、未知的记录标签或字段、重复键以及格式错误的 YAML 都会在启动时失败；运行期热重载时则保留最后可用内容并告警。 +你可以直接编辑该文件——存储会自动重载并接收变更，包括你删除的密钥或记录。`refs` 按环境变量名存放密钥值；`records` 按 `/` 存放各插件的凭据，每条都带 `api-key` 或 `grant` 标签，其中 grant 的 payload 由存储逐字保留，因为只有它的拥有者能解释。产品写入时会保留注释与未触及条目的排版；直接位于某条目上方的注释属于该条目的注解，会随它一起删除。未知的顶层键（例如其他工具写入的令牌）会保留在磁盘上并被忽略，启动时以及每次改变文件内容的重载时都会告警，因为只有 `refs` 与 `records` 中的条目会被解析。文档内其他内容都会被明确拒绝，而不是被静默忽略：非 mapping 的根、在其键空间内不可寻址的键、类型错误或空的值、未知的记录标签或字段、重复键以及格式错误的 YAML 都会在启动时失败；运行期热重载时则保留最后可用内容并告警。 密钥的值可以是任意文本，包括多行值——不需要任何引号技巧。空值等于「没有密钥」，这正是文件中的空字符串被拒绝的原因：移除密钥是删除它，而不是把它置空。`grant` 的 payload 必须经受 JSON 往返，进出两个方向都会强制这一点，因此存储会拒绝无法逐字读回的值。如果磁盘上的文件已无法解析，保存会失败，而不是覆盖产品读不懂的内容。 @@ -134,7 +134,7 @@ records: ### 设计理念 - **一套明确的优先级。** 受管存储对每个提供方都胜出；启动环境、项目 `.env` 与 harness home 的 `.env` 依次是确定的后备来源。 -- **文档只存放凭据。** 它是带 `refs` 与 `records` 分节的版本化文档，而不是 dotenv 文件：一个 harness 拥有、且绝不物化进环境的存储，不能同时充当用户的环境层，否则会以自己的优先级遮蔽非机密条目。 +- **文档只存放凭据。** 外来的顶层键会被容忍，但不解析出任何凭据。它是带 `refs` 与 `records` 分节的版本化文档，而不是 dotenv 文件：一个 harness 拥有、且绝不物化进环境的存储，不能同时充当用户的环境层，否则会以自己的优先级遮蔽非机密条目。 - **写入打补丁，重载整体替换。** 行编辑在跨进程写锁下保留注释与未触及条目；重载整体交换解析后的快照，已删除条目绝不在内存滞留。 - **信任攸关处明确报错。** 启动与重载都会拒绝不可读、无效或可被属主之外读取的文档；运行期重载失败时会保留最后可用快照并告警，而不是拖垮进程。 diff --git a/packages/credentials/credentials-local/src/index.ts b/packages/credentials/credentials-local/src/index.ts index 9989d75009..48ae487c6d 100644 --- a/packages/credentials/credentials-local/src/index.ts +++ b/packages/credentials/credentials-local/src/index.ts @@ -25,8 +25,9 @@ * edits hot-publish through the seam, and each reload replaces the snapshot * wholesale so a deleted entry never lingers in memory. * - * The document holds nothing but credentials, which is why it is a strict - * `CredentialRef`-to-string mapping rather than a dotenv file: a store the + * The document holds credentials, which is why `refs` is a strict + * `CredentialRef`-to-string mapping rather than a dotenv file; a foreign + * top-level key another tool wrote is kept but resolves nothing: a store the * Harness owns and never materializes into the environment cannot also serve * as the user's environment layer; a store that doubled as the environment * layer would shadow non-secret entries behind its precedence, making them @@ -165,7 +166,7 @@ function describeYamlError(error: YAMLError): string { /** The document layout this build reads and writes. */ export const DOCUMENT_VERSION = 1 -/** One parsed credentials document: the two key spaces it stores, keyed as written. */ +/** One parsed credentials document: the two key spaces it stores, keyed as written, plus the top-level keys it does not read. */ export interface CredentialsDocument { /** Reference entries, keyed by {@link CredentialRef}. */ refs: Map @@ -186,7 +187,7 @@ export interface CredentialsDocument { * and needs no version. * @param text - the document's text. * @param filename - absolute path, quoted in errors. - * @returns the parsed references and records. + * @returns the parsed references, records, and ignored top-level keys. */ export function parseCredentialsDocument(text: string, filename: string): CredentialsDocument { // `prettyErrors` is on only for `linePos`; `error.message` is never used, @@ -426,8 +427,9 @@ function assertJsonValue(where: string, value: unknown, seen: Set): void */ function mutableDocument(text: string | undefined): Document { // `text` only ever caches content that parsed successfully, so this re-parse - // for the mutable comment-preserving tree cannot fail. - const document = text === undefined ? new Document({}) : parseDocument(text) + // for the mutable comment-preserving tree cannot fail. `intAsBigInt` keeps an + // integer beyond 2^53 in a foreign top-level entry exact when re-serialized. + const document = text === undefined ? new Document({}) : parseDocument(text, { intAsBigInt: true }) // Stamped on every edit so a document this provider creates is readable by // the same parser that admitted the one it edits; an existing stamp is // rewritten to the identical value. diff --git a/packages/credentials/credentials-local/tests/local.spec.ts b/packages/credentials/credentials-local/tests/local.spec.ts index 7c87325e63..d1436e0731 100644 --- a/packages/credentials/credentials-local/tests/local.spec.ts +++ b/packages/credentials/credentials-local/tests/local.spec.ts @@ -237,9 +237,9 @@ describe('layer ladder', () => { }) describe('document validation', () => { - // Every rejection below is a boot failure rather than a skipped entry: this - // document holds nothing but credentials, so an ignored key would read as - // "the secret I stored has no effect". + // Every rejection below is a boot failure rather than a skipped entry: inside + // `refs` and `records`, an ignored entry would read as "the secret I stored + // has no effect". Only a foreign top-level key is tolerated. it.each([ ['a non-mapping root', 'just a string\n', /must be a mapping/], ['a sequence root', '- DSH_CRED_TEST\n', /must be a mapping/], @@ -317,7 +317,9 @@ describe('document validation', () => { it('boots past an unknown top-level key, warns by name, and keeps it through a write', async () => { const dir = await tempDir() const path = join(dir, '.credentials.yaml') - await writeCredentials(path, 'version: 1\nGITLAB_TOKEN: glpat-secret\nrefs:\n DSH_CRED_TEST: sk-file\n') + vi.stubEnv('GITLAB_TOKEN', '') + await writeCredentials(path, 'version: 1\nGITLAB_TOKEN: glpat-secret\nACCOUNT_ID: 9007199254740993\n' + + 'refs:\n DSH_CRED_TEST: sk-file\n') const ctx = new Context() const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {}) const fiber = ctx.plugin(LocalCredentialProvider, { path, watch: false }) @@ -327,11 +329,12 @@ describe('document validation', () => { await fiber expect(await ctx.credentials.resolve(KEY)).toEqual({ value: 'sk-file', source: 'file' }) expect(await ctx.credentials.resolve(credentialRef('GITLAB_TOKEN'))).toBeUndefined() - expect(warn).toHaveBeenCalledWith(expect.stringContaining('ignoring unknown top-level'), '"GITLAB_TOKEN"', path) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('ignoring unknown top-level'), '"GITLAB_TOKEN", "ACCOUNT_ID"', path) expect(JSON.stringify(warn.mock.calls)).not.toContain('glpat-secret') await ctx.credentials.set(OTHER, 'sk-other') expect(await readFile(path, 'utf8')).toBe( - 'version: 1\nGITLAB_TOKEN: glpat-secret\nrefs:\n DSH_CRED_TEST: sk-file\n DSH_CRED_OTHER: sk-other\n') + 'version: 1\nGITLAB_TOKEN: glpat-secret\nACCOUNT_ID: 9007199254740993\n' + + 'refs:\n DSH_CRED_TEST: sk-file\n DSH_CRED_OTHER: sk-other\n') }) it('reads an empty document as an empty store', async () => { diff --git a/packages/credentials/credentials-local/tests/watcher.spec.ts b/packages/credentials/credentials-local/tests/watcher.spec.ts index 8b98fc4df1..273debf7bd 100644 --- a/packages/credentials/credentials-local/tests/watcher.spec.ts +++ b/packages/credentials/credentials-local/tests/watcher.spec.ts @@ -237,6 +237,34 @@ describe('watcher pipeline', () => { expect(seen).toEqual([KEY]) }) + it('warns about an unknown top-level key a live reload brings in, once per change', async () => { + const dir = await tempDir() + const path = join(dir, '.credentials.yaml') + await writeCredentials(path, 'version: 1\nrefs:\n DSH_CRED_PIPE: a\n') + const ctx = new Context() + const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {}) + const fiber = ctx.plugin(LocalCredentialProvider, { path, debounceMs: 5 }) + cleanups.push(async () => { + await fiber.dispose() + }) + await fiber + expect(warn).not.toHaveBeenCalled() + + await writeCredentials(path, 'version: 1\nFOREIGN: x\nrefs:\n DSH_CRED_PIPE: b\n') + const [instance] = await fakeInstances() + instance!.watcher.emit('all', 'change', path) + await vi.waitFor(async () => { + expect(await ctx.credentials.resolve(KEY)).toEqual({ value: 'b', source: 'file' }) + }) + expect(warn).toHaveBeenCalledTimes(1) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('ignoring unknown top-level'), '"FOREIGN"', path) + + // An event that finds the same text publishes and reports nothing again. + instance!.watcher.emit('all', 'change', path) + await new Promise(resolve => setTimeout(resolve, 50)) + expect(warn).toHaveBeenCalledTimes(1) + }) + it('treats an event for a still-absent file as a no-op', async () => { const dir = await tempDir() const path = join(dir, '.credentials.yaml')