fix(admin): address follow-up Copilot findings on the prior fixes

- loadAdminProviders now validates against a stored-shape schema where
  secrets are plain strings, so a hand-edited ADMIN_PROVIDERS holding an
  {isSet} marker is dropped instead of later crashing maskSecret().
- loadSettings guards against array values (typeof [] === 'object'),
  which would otherwise overlay numeric keys onto process.env.
- Admin SecretInput uses the bare id so the shared component's
  <Label htmlFor> stays associated (only one ProviderDetail mounts).
- Add tests: marker-secret rejection, array-values guard, bedrock
  multi-secret round-trip.
This commit is contained in:
dayuan.jiang
2026-06-15 00:16:01 +09:00
parent ee4613c388
commit 2cc64bbe5f
5 changed files with 61 additions and 6 deletions

View File

@@ -7,6 +7,7 @@ import {
adminProvidersToConfig,
deriveEnvUpdates,
loadAdminProviders,
maskAdminProviders,
mergeSecrets,
type StoredAdminProvider,
validateAdminProviders,
@@ -341,6 +342,22 @@ describe("loadAdminProviders", () => {
expect(loadAdminProviders()).toHaveLength(1)
})
it("round-trips a bedrock provider with multiple string secrets", () => {
const bedrock = provider({
provider: "bedrock",
apiKey: undefined,
awsAccessKeyId: "AKIA123",
awsSecretAccessKey: "secret",
awsRegion: "us-west-2",
models: ["claude-x"],
})
saveSettings({ [ADMIN_PROVIDERS_KEY]: JSON.stringify([bedrock]) })
const loaded = loadAdminProviders()
expect(loaded).toHaveLength(1)
expect(loaded[0].awsAccessKeyId).toBe("AKIA123")
expect(loaded[0].awsSecretAccessKey).toBe("secret")
})
it("drops malformed entries and keeps valid ones", () => {
saveSettings({
[ADMIN_PROVIDERS_KEY]: JSON.stringify([
@@ -364,4 +381,18 @@ describe("loadAdminProviders", () => {
saveSettings({ [ADMIN_PROVIDERS_KEY]: "{ broken" })
expect(loadAdminProviders()).toEqual([])
})
it("drops entries whose secret is an {isSet} marker, not a string", () => {
// A hand-edited file could hold a transit-only marker object; if it
// slipped through, maskSecret() would throw on a non-string value.
saveSettings({
[ADMIN_PROVIDERS_KEY]: JSON.stringify([
{ ...provider(), apiKey: { isSet: true, hint: "…1234" } },
]),
})
const loaded = loadAdminProviders()
expect(loaded).toEqual([])
// Masking the loaded list must not throw
expect(() => maskAdminProviders(loaded)).not.toThrow()
})
})

View File

@@ -57,13 +57,22 @@ describe("loadSettings", () => {
expect(loadSettings()).toEqual({ GOOD: "ok" })
})
it("returns empty object when values is null or an array", () => {
it("returns empty object when values is null", () => {
fs.writeFileSync(
process.env.SETTINGS_FILE!,
JSON.stringify({ version: 1, values: null }),
)
expect(loadSettings()).toEqual({})
})
it("returns empty object when values is an array (no numeric keys)", () => {
fs.writeFileSync(
process.env.SETTINGS_FILE!,
JSON.stringify({ version: 1, values: ["a", "b"] }),
)
// Without the Array.isArray guard this would yield { "0": "a", ... }
expect(loadSettings()).toEqual({})
})
})
describe("applyToEnv / saveSettings", () => {