Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions npm/src/utils/provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,32 @@
}
}

/**
* Resolve base URL for a provider from environment variables.
* Mirrors the env-var resolution in ProbeAgent and FallbackManager so that
* lightweight LLM calls (dedup, etc.) route through the same API gateway.
* @param {string} providerName
* @returns {string|undefined}
*/
export function resolveBaseUrl(providerName) {
const llmBaseUrl = process.env.LLM_BASE_URL;

Check warning on line 95 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: quality

architecture Issue

The new resolveBaseUrl() function lacks dedicated unit tests. While it mirrors existing patterns in ProbeAgent and FallbackManager, the function has no test coverage for environment variable resolution, provider-specific URL fallback logic, or edge cases like undefined provider names.
Raw output
Add a dedicated test file (provider.test.js) or extend existing tests to verify: 1) Provider-specific URLs take precedence over LLM_BASE_URL, 2) LLM_BASE_URL fallback works for all providers, 3) Unknown providers return LLM_BASE_URL, 4) All environment variable combinations work correctly.

Check warning on line 95 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: quality

architecture Issue

No integration tests verify that dedup LLM calls actually route through the resolved base URLs. The fix addresses a critical production issue where dedup calls bypassed API gateways, but there's no test ensuring this behavior works end-to-end.
Raw output
Add integration test that mocks an API gateway scenario: set GOOGLE_API_URL to a custom endpoint, verify createLanguageModel produces a provider with that baseURL, and confirm dedup calls use the configured gateway URL rather than the default Google endpoint.

Check warning on line 95 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: quality

architecture Issue

The resolveBaseUrl() function duplicates environment variable resolution logic already present in ProbeAgent.initializeModel() and FallbackManager.buildFallbackProvidersFromEnv(). This creates triple maintenance burden for the same logic.
Raw output
Consider consolidating base URL resolution into a single shared utility or configuration object to avoid synchronization issues. Alternatively, add JSDoc comments explicitly referencing the other implementations to maintain awareness of the duplication.
switch (providerName) {

Check warning on line 96 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: architecture

documentation Issue

The ANTHROPIC_BASE_URL alias for ANTHROPIC_API_URL is implemented but not documented in any user-facing documentation. All documentation files only mention ANTHROPIC_API_URL, creating a potential source of confusion for users.
Raw output
Either document the ANTHROPIC_BASE_URL alias in the relevant documentation files or remove the alias if it's not officially supported. The current implementation includes it for consistency with existing code, but users have no way to discover this feature.
case 'anthropic':
return process.env.ANTHROPIC_API_URL || process.env.ANTHROPIC_BASE_URL || llmBaseUrl;
case 'openai':
return process.env.OPENAI_API_URL || llmBaseUrl;
case 'google':
return process.env.GOOGLE_API_URL || llmBaseUrl;
case 'bedrock':
return process.env.AWS_BEDROCK_BASE_URL || llmBaseUrl;
default:
return llmBaseUrl;
}
}

Check warning on line 109 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: architecture

architecture Issue

The resolveBaseUrl() function duplicates the exact same environment variable resolution pattern that already exists in two other files (ProbeAgent.js lines 1140-1145 and FallbackManager.js lines 393-398). This creates a third instance of the same logic, increasing maintenance burden and risk of inconsistencies.
Raw output
Consider consolidating the base URL resolution logic into a single shared utility function that all three locations can call. Alternatively, since provider.js is now the 'single source of truth' per its module documentation, consider refactoring ProbeAgent and FallbackManager to use this new resolveBaseUrl() function instead of maintaining their own copies.

Check warning on line 109 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: architecture

architecture Issue

The new resolveBaseUrl() function lacks dedicated test coverage. While similar logic is tested indirectly through fallbackManager.test.js (lines 386-394), that test only covers ANTHROPIC_API_URL and doesn't test the resolveBaseUrl() function directly, nor does it test other provider-specific URLs or the LLM_BASE_URL fallback behavior.
Raw output
Add a dedicated test file (provider.test.js) that tests resolveBaseUrl() with all providers, the precedence hierarchy (provider-specific > LLM_BASE_URL), and edge cases like undefined values. This would ensure the function works correctly and provide test coverage for a utility that's now critical for dedup functionality.

Check warning on line 109 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: security

security Issue

The resolveBaseUrl() function reads base URLs directly from environment variables without any validation or sanitization. This could enable Server-Side Request Forgery (SSRF) attacks if an attacker can manipulate environment variables (e.g., through .env file injection, process.env manipulation in vulnerable code, or compromised CI/CD pipelines). Malicious URLs like 'file:///etc/passwd', 'http://169.254.169.254/latest/meta-data/' (AWS IMDSv1), or internal network addresses could be used to exfiltrate data or access internal services.
Raw output
Add URL validation to ensure base URLs use allowed protocols (https://, http://) and optionally restrict to allowed domains or block private/internal network ranges. Consider implementing a whitelist of allowed base URL patterns or adding validation similar to the MCP client's URL constructor usage (see probe/npm/src/agent/mcp/client.js:96-97).

Check warning on line 109 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: security

security Issue

Base URLs from environment variables are used without sanitization, which could lead to information disclosure if malicious URLs contain credentials or sensitive data in query parameters that might be logged or exposed in error messages.
Raw output
Strip credentials and sensitive query parameters from URLs before use, or ensure that logging and error handling throughout the codebase sanitizes URLs to prevent credential leakage.
/**
* Create a language model instance from provider name + model name.
* Resolves API keys from environment automatically.
* Resolves API keys and base URLs from environment automatically.
* Returns null on failure (graceful degradation for optional features).
* @param {string} providerName - 'anthropic' | 'openai' | 'google' | 'bedrock'
* @param {string} modelName - Model identifier (e.g., 'gemini-2.0-flash')
Expand All @@ -95,10 +118,11 @@
export async function createLanguageModel(providerName, modelName) {
if (!providerName) return null;
const resolvedModel = modelName || DEFAULT_MODELS[providerName];
if (!resolvedModel) return null;

Check warning on line 121 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: quality

logic Issue

createLanguageModel() uses a broad catch-all try-catch that returns null on any error, silently hiding configuration issues. This makes debugging difficult when baseURL resolution fails due to invalid URLs or provider configuration.
Raw output
Consider logging errors in debug mode or differentiating between expected failures (missing API keys) and unexpected errors (invalid URLs, provider instantiation failures). At minimum, add console.error in debug mode to aid troubleshooting.
try {

Check warning on line 122 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: security

security Issue

The resolved baseURL is passed directly to createProviderInstance() without validation. While the underlying AI SDK providers may have their own validation, this creates a trust boundary issue. The codebase demonstrates security-conscious patterns elsewhere (e.g., path-validation.js with symlink attack prevention, bashPermissions.js for command validation), but URL validation is missing here.
Raw output
Consider adding a validateBaseUrl() function that checks URL format, protocol, and optionally domain allowlists. This would align with the codebase's defense-in-depth approach seen in path validation and DSL code validation.
const apiKey = resolveApiKey(providerName);
const provider = createProviderInstance({ provider: providerName, ...(apiKey ? { apiKey } : {}) });
const baseURL = resolveBaseUrl(providerName);

Check warning on line 124 in npm/src/utils/provider.js

View check run for this annotation

probelabs / Visor: architecture

architecture Issue

The createLanguageModel() function uses a try-catch block that returns null on any error, silently swallowing all exceptions. While this provides graceful degradation for optional features like dedup, it makes debugging difficult and could hide real configuration issues (e.g., invalid baseURL format, network connectivity problems during provider initialization).
Raw output
Consider adding debug logging when errors occur (similar to how checkDelegateDedup logs errors when debug mode is enabled). This would help users understand why dedup is failing without breaking the graceful degradation behavior. Alternatively, consider differentiating between different error types (configuration errors vs runtime errors).
const provider = createProviderInstance({ provider: providerName, ...(apiKey ? { apiKey } : {}), ...(baseURL ? { baseURL } : {}) });
return provider(resolvedModel);
} catch {
return null;
Expand Down
Loading