?? falls through on both absent and null/undefined return values

The current expression:

provider.getConfiguredCredentialValue?.(cfg) ?? provider.getCredentialValue(search)

will fall through to getCredentialValue(search) not only when getConfiguredCredentialValue is absent (optional chaining returns undefined), but also when it is defined and explicitly returns null or undefined (e.g., "no credential at this config path"). This means the legacy cfg.tools.web.search path is always consulted as a fallback even for providers that have fully migrated to getConfiguredCredentialValue.

If the intent is backward-compat double-checking, that is fine and working as designed. However, if the intent is "use only the new path once the method is defined", the check should use a ternary instead:

const configuredCredential = provider.getConfiguredCredentialValue
  ? provider.getConfiguredCredentialValue(cfg)
  : provider.getCredentialValue(search);

A comment clarifying the intended dual-path behaviour would prevent future confusion regardless.

This is a comment left during a code review.
Path: src/security/audit-extra.sync.ts
Line: 343-344

Comment:
**`??` falls through on both absent and `null`/`undefined` return values**

The current expression:

```ts
provider.getConfiguredCredentialValue?.(cfg) ?? provider.getCredentialValue(search)
```

will fall through to `getCredentialValue(search)` not only when `getConfiguredCredentialValue` is absent (optional chaining returns `undefined`), but also when it is **defined** and explicitly returns `null` or `undefined` (e.g., "no credential at this config path"). This means the legacy `cfg.tools.web.search` path is always consulted as a fallback even for providers that have fully migrated to `getConfiguredCredentialValue`.

If the intent is backward-compat double-checking, that is fine and working as designed. However, if the intent is "use only the new path once the method is defined", the check should use a ternary instead:

```ts
const configuredCredential = provider.getConfiguredCredentialValue
  ? provider.getConfiguredCredentialValue(cfg)
  : provider.getCredentialValue(search);
```

A comment clarifying the intended dual-path behaviour would prevent future confusion regardless.

How can I resolve this? If you propose a fix, please make it concise.

Missing regression tests for Brave and Perplexity

The PR replaces the previously hardcoded checks for BRAVE_API_KEY and PERPLEXITY_API_KEY with the generic resolveBundledPluginWebSearchProviders mechanism, but no regression test cases cover these two providers in the new collectSmallModelRiskFindings suite.

If the Brave or Perplexity provider's envVars array is ever accidentally modified (or the provider's plugin ID changes), the regression would be silent — neither the old hardcoded path nor any new test would catch it. Consider adding equivalent it.each entries for BRAVE_API_KEY and PERPLEXITY_API_KEY to match the pattern used for the six new providers.

This is a comment left during a code review.
Path: src/security/audit-extra.sync.test.ts
Line: 59-107

Comment:
**Missing regression tests for Brave and Perplexity**

The PR replaces the previously hardcoded checks for `BRAVE_API_KEY` and `PERPLEXITY_API_KEY` with the generic `resolveBundledPluginWebSearchProviders` mechanism, but no regression test cases cover these two providers in the new `collectSmallModelRiskFindings` suite.

If the Brave or Perplexity provider's `envVars` array is ever accidentally modified (or the provider's plugin ID changes), the regression would be silent — neither the old hardcoded path nor any new test would catch it. Consider adding equivalent `it.each` entries for `BRAVE_API_KEY` and `PERPLEXITY_API_KEY` to match the pattern used for the six new providers.

How can I resolve this? If you propose a fix, please make it concise.

Validate credential shape before treating it as configured

hasConfiguredCredentialValue currently treats any non-null/undefined non-string as a valid credential. That makes values like false, 0, or unresolved secret-ref objects count as “configured,” so hasWebSearchKey can report web_search exposure even when no usable API key exists at runtime. This creates false critical models.small_params findings for malformed or unresolved credentials; the check should mirror runtime credential normalization (non-empty string or resolvable secret input) instead of accepting arbitrary objects/primitives.

Useful? React with 👍 / 👎.

Cache web-search key detection outside the model loop

hasWebSearchKey now calls hasBundledWebSearchCredential, which resolves bundled providers through plugin-resolution paths instead of a constant-time field/env check. collectSmallModelRiskFindings invokes this for every small model, so a config with multiple small-model entries repeatedly re-runs bundled provider resolution work (including manifest/runtime registry traversal), turning this audit path into avoidable synchronous overhead; computing this once per audit run (or memoizing in hasWebSearchKey) would keep the check cheap and prevent latency spikes in larger configs.

Useful? React with 👍 / 👎.