Skip to content

Lazy-validate AAI_AUTH_PORT to prevent import-time crashes#53

Merged
alexkroman merged 2 commits into
mainfrom
claude/relaxed-franklin-0z2wn7
Jun 10, 2026
Merged

Lazy-validate AAI_AUTH_PORT to prevent import-time crashes#53
alexkroman merged 2 commits into
mainfrom
claude/relaxed-franklin-0z2wn7

Conversation

@alexkroman

Copy link
Copy Markdown
Collaborator

Summary

Refactor the loopback port configuration to validate AAI_AUTH_PORT lazily at call time rather than at module import time. This prevents malformed environment variables from crashing every CLI command (even --help) with a raw ValueError, instead surfacing clean CLIError exceptions only on the login path.

Key Changes

  • aai_cli/auth/endpoints.py

    • Replace module-level LOOPBACK_PORT = int(os.environ.get(...)) with a loopback_port() function that validates on demand
    • Add _invalid_auth_port() helper to emit clean CLIError (exit code 2) with actionable suggestions
    • Validate port range (1–65535 inclusive); reject 0 and out-of-range values
    • Catch ValueError from non-integer input and re-raise as CLIError to avoid raw exceptions at import time
    • Update redirect_uri() to call loopback_port() instead of referencing the constant
  • aai_cli/auth/loopback.py

    • Call loopback_port() once at the start of capture_callback() and reuse the result to avoid redundant validation
  • tests/test_auth_endpoints.py

    • Add comprehensive validation tests:
      • test_loopback_port_rejects_non_integer() — typo'd port surfaces as clean CLIError
      • test_loopback_port_rejects_above_max() — reject 65536+
      • test_loopback_port_accepts_boundary_values() — accept 1 and 65535
      • test_loopback_port_rejects_zero() — reject OS-assigned port 0
    • Update test_env_override_changes_redirect_uri() to use monkeypatch.setenv() instead of direct attribute patching
  • tests/test_auth_loopback.py

    • Replace monkeypatch.setattr(endpoints, "LOOPBACK_PORT", ...) with monkeypatch.setenv("AAI_AUTH_PORT", ...) throughout
    • Update all endpoints.LOOPBACK_PORT references to call endpoints.loopback_port()
  • aai_cli/init/runner.py

    • Fix file descriptor leak in spawn(): close the log file handle immediately after Popen returns (the child process retains its own duplicate), preventing the fd from being held open for the process's entire lifetime
  • tests/test_init_runner.py

    • Update assertions to verify the file is closed after spawn() returns, confirming the leak is fixed

Implementation Details

The lazy validation approach ensures that:

  • Module import succeeds even with a malformed AAI_AUTH_PORT
  • Only commands that actually call loopback_port() (i.e., aai login) trigger validation
  • Invalid configuration surfaces as a user-friendly CLIError with exit code 2 and a suggestion, not a raw traceback
  • The module sits on the CLI's hot import path, so this prevents every command from crashing on startup

https://claude.ai/code/session_01AsrPL95Kr31qCfvfb1Lkaz

claude and others added 2 commits June 10, 2026 02:43
- auth/endpoints.py parsed AAI_AUTH_PORT with a module-level int() at import
  time. This module is on the CLI's import hot path, so a malformed value
  (e.g. AAI_AUTH_PORT=abc) raised a raw ValueError that crashed *every* aai
  command, even 'aai --help' — not just 'aai login'. Resolve the port lazily
  in a validated loopback_port() that raises a clean CLIError (with range
  check) only on the login path.

- init/runner.spawn() passed log_path.open("w") straight to Popen and never
  closed the parent's handle, leaking a file descriptor for the lifetime of
  the (long-lived) tunnel process. Close it after Popen returns; the child
  keeps its own dup.

Tests updated to drive the port via AAI_AUTH_PORT, with added boundary
coverage (1, 65535, 65536, 0, non-integer).
@alexkroman alexkroman merged commit 1673446 into main Jun 10, 2026
11 checks passed
@alexkroman alexkroman deleted the claude/relaxed-franklin-0z2wn7 branch June 10, 2026 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants