Make the crate do what its documentation already claimed
Review from ai-app's session, acted on. **A test that asserted nothing.** `a_missing_tunnel_explains_itself` returned early whenever `wg0` was up -- which it is on this VM and on the host, so it passed everywhere and never once checked the message. The same shape of test that let the file-permissions bug survive three codebases. `netif` now splits the lookup from the decision: `wg_address_among` and `addresses_among` take the interfaces, so the failure is reachable by handing in an empty list rather than by hoping the machine has no tunnel. Four tests where there were two, including that an IPv6-only `wg0` is not an answer. **Two modules the docs promised and the crate did not have.** `certs`, which is the piece where being written twice is worst -- a trust anchor built two ways can be built differently two ways, and the difference reaches a phone as an opaque handshake failure. And `format`, the RON house rules, which were byte-identical in both projects and so the clearest thing in the evidence table. `product` names the certificate and is the whole of what is per-project; the test decodes the DER and looks for it there rather than trusting what was passed in. **The README title still said wg-server-app**, three commits after everything else was renamed. Two judgement calls promoted from silent to written down, both of which would otherwise be inherited rather than chosen: `WG_INTERFACE` is a constant because *these* projects have one tunnel, which is the first thing a third user should expect to change; and `local_addresses` puts the emulator's host alias in every certificate, a SAN for an address the machine does not own. And a review heuristic the day kept proving: when you find a rule stated, grep for its siblings. Three bugs today were the correct rule already written down and applied to one member of a set.
This commit is contained in:
1 parent
592114bfc9
commit
841a3a8372
7 files changed
+1055
-40
No files matched your search
@@ -1,4 +1,4 @@
|
||||
# wg-server-app
|
||||
# wg-app-link
|
||||
|
||||
The private link between a phone and a machine you run, extracted from the
|
||||
two projects that had each written it.
|
||||
@@ -94,21 +94,21 @@ time the sharing paid rather than merely being argued for.
|
||||
|
||||
## What is here
|
||||
|
||||
`server/` — the `wg-app-link` crate. Three modules, each extracted only after
|
||||
`server/` — the `wg-app-link` crate. Five modules, each extracted only after
|
||||
diffing the two copies and finding nothing but a name between them.
|
||||
|
||||
- **`netif`** — `wg_address()`, which fails closed when the tunnel is down, and `local_addresses()` for the certificate's SANs. The product name is a parameter so the failure reads as advice rather than as a library complaining.
|
||||
- **`netif`** — `wg_address()`, which fails closed when the tunnel is down, and `local_addresses()` for the certificate's SANs. The product name is a parameter so the failure reads as advice rather than as a library complaining. Both split the *lookup* from the *decision*, so the failure path can be tested on a machine that has a tunnel — every machine this runs on does.
|
||||
- **`enroll`** — token generation, hex-SHA-256 storage, constant-time comparison, the `<scheme>://enroll?…` URI, and the terminal QR. The URI scheme is the parameter, because it is what routes a scan back to the right app.
|
||||
- **`certs`** — the CA generated once and never replaced, the leaf reissued every start. `product` names the organisation and common name and is the whole of what is per-project. Shared because being written twice is worst here: a trust anchor built two ways can be built differently two ways, and the difference surfaces as an opaque handshake failure on a phone.
|
||||
- **`format`** — the two RON house rules. This was byte-identical in both projects, which made it the clearest thing in the evidence table and the easiest deletion.
|
||||
- **`private`** — owner-only files and directories, taken from ai-app's version, with an `append_file` alongside `create_file` because a transcript must never be truncated by being opened.
|
||||
|
||||
## What should follow, and what should not
|
||||
|
||||
**Should follow, in this order.** Each is already near-identical:
|
||||
|
||||
1. `certs` — CA generated once and never replaced, leaf reissued every start. One `product: &str`.
|
||||
2. The RON `format` house rules — byte-identical today, so this is pure deletion.
|
||||
3. The Kotlin `EnrollmentScanActivity`, `PinnedCert`, and the enrollment/Keystore half of `ServerConfig`, as an Android library module. This is where the sharing pays most, because it is where the two copies have drifted furthest apart in *both* directions.
|
||||
4. Atomic owner-only config save. Both do temp-file-then-rename with the mode set before the rename; only the schema differs.
|
||||
1. The Kotlin `EnrollmentScanActivity`, `PinnedCert`, and the enrollment/Keystore half of `ServerConfig`, as an Android library module. This is where the sharing pays most, because it is where the two copies have drifted furthest apart in *both* directions — and `applyPinnedTls` is security-critical and identical, which is the same argument `certs` won on.
|
||||
2. Atomic owner-only config save. Both do temp-file-then-rename with the mode set before the rename; only the schema differs.
|
||||
|
||||
**Should not.** Naming these is the point of the exercise:
|
||||
|
||||
@@ -117,6 +117,37 @@ diffing the two copies and finding nothing but a name between them.
|
||||
- **Config schemas.** Shared house rules, separate contents.
|
||||
- **Anything above the link.** Projects, builds, sessions, providers. If a third project would not want it, it does not belong here.
|
||||
|
||||
## Two decisions worth stating, rather than inheriting
|
||||
|
||||
**`WG_INTERFACE` is a constant, not a parameter.** For these two projects a
|
||||
second answer would mean two ideas of what "the tunnel" is, and there is
|
||||
one tunnel. That reasoning is about *these* projects rather than about the
|
||||
world, so it is the first thing a third user should expect to change.
|
||||
|
||||
**`local_addresses` puts `10.0.2.2` in every certificate.** It is the alias
|
||||
an Android emulator reaches its host by, and not an address the machine
|
||||
owns — so this is a SAN for something that is not this host, present so a
|
||||
debug build can reach a server running beside it. Inherited from ai-app
|
||||
and correct there; written down here because promoting it makes it every
|
||||
future project's default rather than one project's choice.
|
||||
|
||||
## A review heuristic this repo keeps proving
|
||||
|
||||
**When you find a rule stated somewhere, grep for its siblings.** Three
|
||||
separate bugs today were the correct rule already written down, applied to
|
||||
one member of a set and not the others:
|
||||
|
||||
- `create_dir` set its mode a second time and explained why; `create_file`
|
||||
and `append_file`, three functions below, did not.
|
||||
- ai-app explained one ssh failure and not its sibling.
|
||||
- dev-updater's acceptance gate compared components by *declaration*, and
|
||||
`BuildState::matches` — which meant the same thing — compared them by
|
||||
full equality, so writing a measured field replaced a running build's
|
||||
state.
|
||||
|
||||
None was found by a test. All three were found by a second reader looking
|
||||
at code as a set, which is the thing this crate is actually buying.
|
||||
|
||||
## The name
|
||||
|
||||
`wg-app-link`: the link between an app and the machine it talks to, over
|
||||
|
||||
Reference in new issue
Block a user