guru-connect/reports/review-2026-06-05/gemini-server.md

GuruConnect server review — gemini — 2026-06-05

I have completed an independent code review of the GuruConnect server based on the provided Rust source files. Below is the prioritized list of findings, categorized by severity, followed by an overall assessment and the most critical recommendation.

Priority Findings List

SEVERITY	FILE:LINE	FINDING	CONCRETE FIX
CRITICAL	downloads.rs:142	Insecure Default API Key: The permanent agent download endpoint uses a hardcoded default "managed-agent" API key if none is provided. This is a weak, well-known credential that grants any agent immediate persistent access if enabled on the server.	Remove the default fallback. Require a valid, high-entropy API key or a session-scoped enrollment key for all permanent agent downloads.
CRITICAL	main.rs:163	Sensitive Credentials in Local File: On first run, the server writes the initial admin password to .admin-credentials. While it attempts to set 0o600 permissions, this file remains a high-value target for local privilege escalation or accidental exposure in backups/logs.	Provide the initial admin password via a one-time environment variable or secure secret manager. If a file must be used, ensure it is deleted automatically after the first successful login.
HIGH	main.rs:430+	Missing Rate Limiting on Admin CRUD: Sensitive admin endpoints (/api/users, /api/releases, etc.) lack per-IP rate limiting. A compromised low-privilege account or a brute-force attack on the admin plane could cause DoS or data corruption.	Apply the RateLimiter middleware to all /api/users, /api/releases, and /api/sites management routes.
HIGH	removal.rs:104	TOCTOU in Machine Removal: The legacy hard-delete path for machines performs a get_machine_by_agent_id lookup, then an optional uninstall command, then a DELETE. A machine could reconnect or be modified between the lookup and the delete, leading to inconsistent state or missed audits.	Wrap the removal logic in a database transaction and use SELECT ... FOR UPDATE or conditional DELETE statements (e.g., WHERE agent_id = $1 AND updated_at = $2) to ensure atomicity.
HIGH	relay.rs:664	Unbounded Input Queue in Viewer: The viewer's input channel has a capacity of 64 messages. While try_send is used (dropping excess), a large number of viewers could still pressure the agent's inbound bandwidth if not throttled correctly at the broadcast level.	Implement a global per-session input rate limit (not just per-viewer) to protect the agent from aggregate input flooding by multiple technicians.
MEDIUM	auth.rs:320	Weak Password Policy: The server only enforces a minimum length of 8 characters for new passwords. It does not check for complexity, common patterns, or use a "pwned password" list.	Integrate a password strength library (e.g., zxcvbn) and reject common passwords. Increase the minimum length to 12+ characters.
MEDIUM	jwt.rs:207	Lack of Token Refresh Mechanism: The server relies on 24-hour JWTs with no explicit refresh token flow. This encourages long-lived sessions and increases the impact of a stolen token.	Implement short-lived access tokens (e.g., 15 mins) and long-lived refresh tokens stored securely (e.g., HttpOnly cookies).
MEDIUM	downloads.rs:188	Path Sanitization Limit: sanitize_filename truncates at 32 characters but doesn't check for reserved Windows filenames (e.g., CON, PRN, NUL).	Use a more robust sanitization library or explicitly reject reserved Windows filenames to prevent issues on the client-side download.
LOW	rate_limit.rs:94	In-Memory Rate Limit Persistence: Rate limit state is purely in-memory. A server restart clears all lockouts/counters, allowing an attacker to resume brute-forcing immediately.	(Optional) Periodically persist the RateLimitState to the database or use a fast-access cache like Redis for distributed/persistent rate limiting.
LOW	events.rs:252	Audit Log Growth: The connect_session_events table grows indefinitely with every connection and admin action. There is no automated pruning or archival logic visible.	Implement a background task to prune or archive audit events older than a configured retention period (e.g., 90 days).

---

Overall Assessment

The GuruConnect server demonstrates a strong focus on security fundamentals, particularly in its use of Argon2id for password/enrollment hashing, SHA-256 for per-agent keys, and the implementation of a session-scoped viewer token model that successfully closes common relay-hijacking vulnerabilities. The architecture is modular and tenancy-ready, and the "fail-closed" approach to authentication (especially in the agent_ws_handler) is commendable.

However, the presence of an insecure default API key in the download path and the reliance on local files for sensitive bootstrap credentials represent significant "low-hanging fruit" for attackers. The concurrency controls in the removal paths also show potential for TOCTOU races that could complicate auditing and state management in high-load environments.

Single Most Important Thing to Fix

Remove the default "managed-agent" API key fallback in downloads.rs:142. This is a "backdoor" by default; any permanent agent download without an explicit key results in a credential that an attacker can predict and use to attempt unauthorized machine registration if the server's AGENT_API_KEY environment variable happens to match this weak default. All credentials should be high-entropy and unique.