Skip to content

[code-review] SaveAPIKey silently discards SetEndpointAPIKey error — key may not persist #117

Description

@topcheer

File and Lines

desktop/wailskit/config.go:313

Problem Description

SaveAPIKey discards the error returned by SetEndpointAPIKey:

func SaveAPIKey(vendor, endpoint, apiKey string) error {
    // ...
    cfg.SetEndpointAPIKey(vendor, endpoint, apiKey, vendorScoped)  // error discarded!
    return cfg.Save()
}

SetEndpointAPIKey returns errors in multiple conditions (confirmed in internal/config/config_keys.go):

  • c == nil"config is nil"
  • Vendor not in c.Vendors"vendor %q is not configured"
  • Endpoint not found (non-vendorScoped) → "endpoint %q is not configured for vendor %q"

When SetEndpointAPIKey fails, the config is unchanged. cfg.Save() then succeeds (writes unchanged config), and SaveAPIKey returns nil — reporting success to the frontend even though the key was never persisted.

Trigger Scenario

  1. Vendor "zai" has multiple endpoints (so vendorScoped = false)
  2. Frontend sends an endpoint name that doesn't match config (e.g., renamed/deleted endpoint, or case mismatch)
  3. SetEndpointAPIKey("zai", "wrong-name", key, false) returns error
  4. Error is discarded by SaveAPIKey
  5. cfg.Save() succeeds (writes unchanged config)
  6. Frontend shows "API key saved" — but key was never persisted
  7. All subsequent API calls fail with auth errors

Expected vs Actual Behavior

  • Expected: Error from SetEndpointAPIKey is propagated to caller
  • Actual: Error is silently discarded, success reported

Fix Suggestion

if err := cfg.SetEndpointAPIKey(vendor, endpoint, apiKey, vendorScoped); err != nil {
    return err
}
return cfg.Save()

Severity

Medium — Silent failure on credential management path. User believes key was saved when it wasn't.

Verification

Independently verified by subagent sa-174:

  • Confirmed SetEndpointAPIKey returns errors in config_keys.go lines 16-22, 33-34, 66-68
  • Confirmed SaveAPIKey has no fallback path — line 313 is the only SetEndpointAPIKey call
  • Confirmed desktop frontend SettingsPage.tsx sets apiKeySet(true) after SaveAPIKey returns nil

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions