feat: standalone Node.js server + web admin UI for VM hosting

[charindithjaindu]

No description provided.

[codereviewbot-ai]

Potential runtime error: info.display_name is accessed without verifying that info is an object. If info is undefined or not an object, this will throw an exception and may disrupt the polling process for all accounts.

Recommended solution:
Add a type check to ensure info is an object before accessing display_name:

return (typeof info === 'object' && info && 'display_name' in info && info.display_name) || name.replace('models/', '').replace(/-/g, ' ');

[codereviewbot-ai]

Concurrency/Data Race Issue with In-Memory Session Store

The sessions Map is accessed and mutated by multiple functions (cleanupExpired, issueToken, revokeToken, isTokenValid) without any synchronization. In a multi-threaded or clustered Fastify deployment, this can lead to data races, inconsistent state, or memory leaks. For example, simultaneous calls to cleanupExpired and issueToken could interfere with each other.

Recommendation:

• For production, use a process-safe session store (e.g., Redis) or implement locking/mutex mechanisms if you must use in-memory storage.
• If this is strictly for single-process, standalone use, document this limitation clearly to avoid misuse.

[codereviewbot-ai]

Potential Performance Bottleneck in Token Cleanup

The cleanupExpired function iterates over all sessions and is called on every token issuance and validation. As the number of sessions grows, this could become a significant performance bottleneck, especially under high load.

Recommendation:

• Consider using a more efficient data structure for session expiry (e.g., a priority queue or a sorted map by expiry time), or schedule periodic cleanup rather than invoking it on every operation.

[codereviewbot-ai]

The call to fs.readFileSync(candidate, 'utf-8') is not wrapped in a try-catch block. If the file is deleted or becomes unreadable between the existence check and this read, the function will throw and potentially crash the process.

Recommendation: Wrap the file read in a try-catch block and handle errors gracefully, returning { path: candidate, loaded: 0 } or a similar safe fallback.

[codereviewbot-ai]

Security Issue: Weak Admin Password Allowed

The code only checks for the presence of AGM_ADMIN_PASSWORD, but does not enforce any minimum strength or length requirements. This could allow weak passwords, increasing the risk of unauthorized access.

Recommendation:
Add a check to enforce a minimum password length and consider validating password complexity. For example:

if (!adminPassword || adminPassword.length < 12) {
  throw new Error('AGM_ADMIN_PASSWORD must be at least 12 characters long.');
}

[codereviewbot-ai]

Type Validation Issue:
In readSafeStorageKey, after reading the file, there is no validation that encryptedKey is a Buffer. If the file is corrupted or written incorrectly, fs.readFile may return unexpected data, leading to runtime errors during decryption.

Recommendation:
Add a type check before decryption:

if (!Buffer.isBuffer(encryptedKey)) {
  logger.error('Security: safeStorage key file is not a buffer');
  return null;
}

This will prevent cryptographic operations on invalid data.

[codereviewbot-ai]

Potential Race Condition and Performance Issue in Polling

The useEffect hook sets up a polling interval (every 10 seconds) to call reloadAll, which triggers both reloadAccounts and reloadProxy. If either of these async functions takes longer than 10 seconds, multiple overlapping requests may occur, leading to race conditions, inconsistent UI state, or unnecessary backend load.

Recommended Solution:

• Use a mechanism to ensure only one polling request is active at a time, such as a flag or abort controller.
• Alternatively, consider using a debounced or sequential polling approach:

useEffect(() => {
  if (!authed) return;
  let active = true;
  const poll = async () => {
    await reloadAll();
    if (active) setTimeout(poll, 10000);
  };
  poll();
  return () => { active = false; };
}, [authed, reloadAll]);

This ensures that a new poll only starts after the previous one completes.

[codereviewbot-ai]

API Key Masking May Leak Key Length

When the API key is hidden, the UI displays a string of bullet characters ('•'.repeat(Math.min(apiKey.length, 32))). This reveals the length of the API key (up to 32), which could be sensitive information and may aid attackers in brute-forcing the key.

Recommended Solution:

• Always display a fixed number of masking characters, regardless of the actual key length:

: '•'.repeat(32)

This prevents leaking the key length.

[codereviewbot-ai]

The request function attempts to parse all response bodies as JSON without checking the Content-Type header. This can result in runtime errors if the response is not JSON (e.g., empty or plain text responses). To improve robustness, check if the response's Content-Type includes application/json before parsing, and handle parsing errors gracefully:

const contentType = res.headers.get('Content-Type') || '';
let body = null;
if (text && contentType.includes('application/json')) {
  try {
    body = JSON.parse(text);
  } catch (e) {
    throw new Error('Failed to parse JSON response');
  }
}

[codereviewbot-ai]

The error handling logic in the request function directly exposes backend error messages to the client by using body?.error as the thrown error message. This could leak sensitive information if the backend returns detailed error messages. Consider sanitizing or generalizing error messages before surfacing them to the user, for example:

const message = res.status >= 500 ? 'Server error' : (body?.error ?? `${res.status} ${res.statusText}`);
throw new Error(message);

[codereviewbot-ai]

Security Issue: Unrestricted CORS

The call to app.enableCors() on line 29 enables Cross-Origin Resource Sharing (CORS) for all origins by default. This can expose the server to cross-origin requests from any domain, which may not be secure in production environments.

Recommendation:
Specify allowed origins explicitly or make CORS configuration part of your ProxyConfig. For example:

app.enableCors({ origin: ['https://yourdomain.com'] });

Or derive the allowed origins from configuration to avoid exposing the server unnecessarily.

[Draculabo]

Hi, thank you for the PR.

Is this change strictly necessary? I try to follow the principle of "Occam's Razor"—avoiding unnecessary complexity unless it's essential. Approving this would increase the long-term maintenance burden of the project.

Could you please open an issue first to explain:

What exactly this PR does?

Why is it needed and what specific problem does it solve?

Is this a genuine pain point for users?

I’d like to understand the rationale before committing to the additional maintenance.