Skip to content

Three pre-existing defects adjacent to the proxy TLS fail-closed work (#320) #324

Description

@gnanirahulnutakki

Split out of PR #320 review. All three are pre-existing#320 either reflowed the line or did not touch it at all — so they were deliberately left out of that PR to keep it scoped to the fail-closed change. All three are real and worth fixing.

1. generate_self_signed_cert() derives the cert SAN from the bind host (python/vibap/tls.py)

The SAN is hardcoded to x509.IPAddress(ipaddress.IPv4Address(hostname)), so the certificate identity is taken from the bind address. Verified on this repo's pinned deps:

localhost    -> RAISES AddressValueError: Expected 4 octets in 'localhost'
0.0.0.0      -> IPv4Address OK (0.0.0.0)
127.0.0.1    -> IPv4Address OK (127.0.0.1)

Two distinct failures:

  • Binding to a DNS name (localhost) raises AddressValueError outright.
  • A wildcard bind (0.0.0.0) produces a cert with SAN 0.0.0.0, which no client will match.

Fix: split the bind address from the certificate identity, and emit x509.DNSName for names vs x509.IPAddress for IP literals. The auto-generation path in resolve_tls_paths() has the same issue.

Note this now interacts with #320: with TLS failing closed, a config that previously degraded to plain HTTP will now be a hard startup failure. That is the correct direction, but it makes this bug more visible.

2. _ensure_default_home_dir() runs when no path actually derives from DEFAULT_HOME (python/vibap/proxy.py)

if log_path is None or state_dir is None or receipts_log_path is None:
    _ensure_default_home_dir()

With explicit log_path and state_dir but an omitted receipts_log_path, the receipts path is derived from log_path via .with_name("receipts_log.jsonl") — it never touches DEFAULT_HOME. The receipts_log_path is None disjunct is therefore both redundant and harmful: it can fail startup on a read-only home even though every effective path was supplied explicitly.

receipts_log_path is None and log_path is None is already subsumed by log_path is None, so the condition should reduce to:

if log_path is None or state_dir is None:
    _ensure_default_home_dir()

Needs a regression test covering explicit log_path + state_dir + omitted receipts_log_path against a non-writable home.

3. checks[:5] magic slice skips two plugin hooks (python/vibap/cli.py)

if claude_binary and all(check["ok"] for check in checks[:5]):

The slice omits subagent_start and subagent_stop, so claude plugin validate can be reported as runnable while either hook is missing. Replace the magic slice with the full plugin-check set — ideally by keeping plugin checks in their own list before appending claude_binary and active_passport, so the boundary cannot silently drift again when a check is added.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions