7.5 KiB
Bounded Logs and Reliable Settings Implementation Plan
Intent: Stop unbounded disk usage and make existing settings predictable without changing authentication or tunnel behavior.
Current Behavior: Both log writers append forever. Installer interpolation corrupts special characters and replaces local settings; unknown runtime flags can fall through to connection.
Expected Outcome: Each log has a bounded current file and one bounded previous file (5 MiB each by default); custom settings survive updates; settings round-trip literally; mistyped commands fail before connecting.
Target-Perspective Output: Isolated CLI/filesystem examples show capped current/archive sizes and recent records, valid JSON with quoted settings, preserved configuration after two renders, and a rejected unknown flag.
Truth Owner: CLI env for runtime settings and the existing TOML for SSO. CLI owns the log limit reported to Swift; each process owns its log writer.
Contract Boundary: Keep current status/events/auth modes, adding optional logging metadata. Installer preserves unowned env lines and existing TOML profile/rules while updating the selected mode and username.
Cutover: Replace append-only writers in place; retain existing log names and add only .1. Do not mutate the installed setup or real logs during development.
Displaced Path: Raw sed template interpolation and unbounded writes are removed, with no parallel logging backend or new dependency.
Value Density: Bounded storage, correct literal settings, and update preservation address observable defects; avoid a general settings framework and wholesale shell/Swift rewrite.
Acceptance Evidence: Before/after automated regression checks using temporary files/homes and fake clients; shell smoke and Swift release build. Include long-running writes, oversized existing files/records, log I/O failure, quoted settings, update preservation, invalid flags.
Evidence Lane: Automated local CLI/filesystem only. No GUI, live connection, installation, Keychain writes, or deletion of real logs.
Kill Criteria: No append-only production writer; no raw sed settings interpolation; no unknown-flag path into connect; tests exercise the changed behavior.
Architecture Slice: bin/vpn-lemanapro.sh -> raw client stream/log + status JSON -> VPNManager.swift -> bounded app log. install.sh -> env templates/TOML -> same CLI. Authentication patches, DNS cleanup, health/reconnect semantics and unrelated dirty work are excluded.
Plan Review Gate: Requires PRE review before execution.
Tasks and ownership
- Root: bounded shell writer in
bin/vpn-lemanapro.sh, Foundation-onlyBoundedLog.swiftused byVPNManager.swift, log-limit setting and optional JSON metadata. Test both writers with actual temporary files, including existing oversized logs, repeated rollover and write failures; preserve event parsing. Unknown runtime flags fail with status 2. Literal JSON encoding for BW item. Fix_prompt_readEOF propagation and explicitly stop every caller before subsequent persistence/credential actions; empty Enter keeps existing behavior. Test username, secret and save-choice EOF with fake input and a timeout. Do not change auth modes/sources, health or reconnect semantics. - Configuration agent (parallel, disjoint):
install.sh, env templates, new isolated configuration regression check. Precedence: flags > invocation env > saved env > defaults for supported installer settings; preserve arbitrary runtime settings and TOML custom profile/rules. Replace raw sed interpolation with stdlib literal escaping, reusing existing quoting conventions. Verify a second render preserves custom settings and special characters; installer environment mode takes effect. Do not edit runtime, Swift, README or smoke test. - Root integration: wire small regression checks into existing automated workflow as appropriate, document editable settings, precedence and rotation in README. Fix only evidence-backed issues in changed paths. Record remaining audit findings instead of opportunistic auth/refactoring work.
- Read-only reviewer: POST contract/correctness/maintainability review. Root resolves blockers and runs syntax checks, focused regressions,
tests/smoke.sh,swift build -c release --package-path app,git diff --check.
Risks and evidence
LEMANA_VPN_LOG_MAX_BYTES defaults to 5242880; accept decimal integers 1..999999999, reject empty explicit values, zero, negative and malformed values before side effects. Swift defaults to 5242880 before status or with an old CLI and uses the validated optional CLI value thereafter. Verify saved env -> status JSON -> Swift decoding/writer setting. PRE review findings above are incorporated; proceed to execution.
| Contract | Risk | Independent oracle |
|---|---|---|
| Log size and recent history | Disk growth, copying giant legacy file | Current and .1 each <= configured byte limit, newest record retained, only two files |
| Log failures | VPN output consumer exits | Fake output still produces expected JSON event with unwritable log destination |
| Settings round-trip/update | Corruption, command substitution, silent resets | Exact recovered values, sentinel command never executed, custom TOML values preserved |
| CLI validation | Typo initiates SSO | Exit 2 before client/status file creation |
| Compatibility | Existing dirty auth work regresses | Baseline vs final smoke results and Swift build, live acceptance explicitly unrun |
Deferred audit findings
- Arbitrary
OPENCONNECT_LITE_CONFIG_DIRand XDG runtime path need a separate end-to-end contract; do not mask this with status-only changes. - The pre-existing CLI
_persist_usernamewriter selects the first username-like TOML line; unlike the revised installer it is not scoped to[credentials]. Custom profiles containing another username need a follow-up shared configuration-write contract and regression before broadening editable SSO settings. - Broader extraction of embedded SSO patches and reconnect/state-machine changes require their own evidence and are outside this slice.
Execution evidence
- Baseline: shell syntax, existing smoke, release Swift build and diff whitespace checks passed before edits.
- Historical app log was measured read-only at 2,552,898,484 bytes. Repeated closed-terminal credential prompts dominated the sampled tail. Current baseline still reproduced a closed-input loop; the new regression times out on that baseline and passes with explicit EOF propagation.
python3 tests/configuration.py: 10 checks pass, covering literal round-trips, saved settings, precedence, legacy inputs, TOML variants and fresh help without Python. The first six regression cases all failed against the saved baseline installer.python3 tests/logging.py: passes actual CLI/Swift file writes, bounded legacy/repeated/oversized records, reduced limits, private permissions, closed username/password/TOTP/Bitwarden/consent input, preserved empty Enter, JSON output on log failure, settings decoding and rejected flags/limits.- Final shell syntax,
tests/smoke.sh(smoke ok),swift build -c release --package-path appandgit diff --check: PASS. - PRE/POST correctness and maintainability reviews completed; findings in the changed slice resolved. CLI ordinary-write microbenchmark improved from the first implementation's 4.800 s to 1.806 s per 300 records (old append-only baseline 0.831 s); the remaining overhead checks file size before writing.
- Real logs and installed files were not changed. No GUI, live VPN, installation or credential-store operation was performed. Live connection acceptance remains untested.