From fc3c1d51b1be9db6ba6ad286584fbc34bb305f38 Mon Sep 17 00:00:00 2001 From: Andrew Guschin Date: Fri, 30 Oct 2026 03:32:00 +0400 Subject: Fix protected activation ordering and diagnostic review findings --- docs/implementation-review.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 docs/implementation-review.md (limited to 'docs/implementation-review.md') diff --git a/docs/implementation-review.md b/docs/implementation-review.md new file mode 100644 index 0000000..0bf25c0 --- /dev/null +++ b/docs/implementation-review.md @@ -0,0 +1,33 @@ +# First-release implementation review + +The implementation was reviewed against `40cf3276cdc74bf6f1795990ed600891a637d1fe`. Independent standards and specification reviews inspected candidate `d608ffe`. Corrections followed in the same branch. Review findings do not establish that the release acceptance matrix has passed. + +## Standards + +The standards review found no hard repository-rule violations and three design observations: + +- Numeric TUI screen state allowed a subscription row to be clamped against the configuration list. Refresh now uses the current screen's row bounds. +- Entry validation used free-form strings. Saved entries now use a serialized status enum, and the storage boundary rejects contradictory validation/connection data. +- The controller created Xray probes directly while activation used the runtime boundary. Probe creation now goes through that boundary as well. + +Standards findings: three design observations, zero hard violations. The row-selection bug was the most consequential observation and is corrected. + +## Spec + +The specification review found five correctness issues: + +- Retry could bypass a failed activation preflight. Runtime retries now require successful compatibility checks, and each managed Xray service start repeats the version check. +- An unset selection could reuse a TUN left by the previous controller. Startup first blocks forwarding, and no-selection reconciliation stops the owned core and removes capture. Enabled selection changes block forwarding before committing new state. +- An untrusted local listener could impersonate the probe's SOCKS endpoint. The adapter now generates a private TLS identity for an authenticated HTTPS proxy. Diagnostic requests authenticate that identity before using the proxy, so a reused TCP port cannot authorize a different route. +- Manual diagnostic requests ignored provider cooldowns. Retry-After now prevents further diagnostic requests to that provider, including manual Ping and Speed. +- Competing HTTP and measurement timeouts could turn a completed duration-limited Speed observation into a generic error. Diagnostics now use one measurement deadline and report consumed bytes when the duration cap ends a nonempty measurement. + +Spec findings: five correctness issues, corrected. The stale-selection forwarding path was the most serious finding. VM CLI regressions cover failed preflight retries and startup with stale capture and unset selection. The probe TLS changes and duration/rate-limit behavior still require end-to-end validation with the required current Xray runtime. + +## Validation boundary + +After the user's VM-only instruction, all test execution used the supplied ALT VM. Compilation and formatting used the local checkout. The VM checkpoint precedes package/test preparation and is restored after validation. + +The current package available from that VM's refreshed Sisyphus repository is Xray 26.3.27. The implementation rejects activation with it. The tests therefore distinguish actual guard/controller/polkit/editor/recovery behavior from unverified native-TUN and diagnostic operation with the researched newer Xray contract. The product installer is not counted as a successful installation in this environment; system-bus tests stage their integration fixtures explicitly. + +The final available suite passed on 2026-10-30: 10 Rust CLI/controller acceptance tests and 21 VM workflow checks. The [captured output](validation/linux-vm.txt) records the tested executable digest and each result. Checks include TCP and UDP DNS destination policy, IPv4/IPv6 blocking, retained incoming connections, denial/allow authorization, ordinary-user TUI restoration, editor privacy, failed-preflight retries, stale capture with unset selection, malformed TOML recovery, missing durable state, and uninstall retention/purge. -- cgit v1.2.3