From 2cc010cf2be8e3132fc85650f005f81847ab9979 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Mon, 21 Sep 2026 11:39:12 +0000 Subject: [PATCH] =?UTF-8?q?tasks:=20decompose=20the=20vendor=20group=20?= =?UTF-8?q?=E2=80=94=20six=20dependency-ordered=20tasks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inline vendor (vendor.md, ADR-011 §Amendment) is the root of the crate's dependency graph; this decomposes it before any downstream group (params/wire/producer/consumer/doors). - vendor-crate-scaffolding: Cargo.toml (wasm-clean tokio subset, no alkcall yet), lib.rs, vendor/ skeleton, bundled MIT notice - vendor-wire-vocabulary: consts, codecs, errors, macros — with the extraction-boundary pins (error-variant pruning, pub command codecs, macro de-exporting) - vendor-server-typestate: Socks5ServerProtocol + the new_reply publication (carried change #2); BIND's driving is producer-group work, the vendor carries only the surface - vendor-client-session: Socks5Stream::use_stream path + the AsyncRead/AsyncWrite impls; Config pruned (connect_timeout cut); tests against a raw-byte mock server so it runs parallel with the typestate task - vendor-differential-tests: RFC edge-case suite over duplex streams, pinning upstream behavior per vendor.md's differential posture - review-vendor-extraction: the gate before downstream groups — file-by-file diff vs /workspace/fast-socks5 must contain only cut/carried-change/adjustment hunks Graph validated: all dependency ids resolve, no cycles; topological order scaffolding → wire → (typestate ∥ client) → tests → review. --- .../review-vendor-extraction.md | 85 ++++++++++ tasks/implementation/vendor-client-session.md | 123 ++++++++++++++ .../vendor-crate-scaffolding.md | 93 +++++++++++ .../vendor-differential-tests.md | 115 +++++++++++++ .../implementation/vendor-server-typestate.md | 155 ++++++++++++++++++ .../implementation/vendor-wire-vocabulary.md | 134 +++++++++++++++ 6 files changed, 705 insertions(+) create mode 100644 tasks/implementation/review-vendor-extraction.md create mode 100644 tasks/implementation/vendor-client-session.md create mode 100644 tasks/implementation/vendor-crate-scaffolding.md create mode 100644 tasks/implementation/vendor-differential-tests.md create mode 100644 tasks/implementation/vendor-server-typestate.md create mode 100644 tasks/implementation/vendor-wire-vocabulary.md diff --git a/tasks/implementation/review-vendor-extraction.md b/tasks/implementation/review-vendor-extraction.md new file mode 100644 index 0000000..8f0d60b --- /dev/null +++ b/tasks/implementation/review-vendor-extraction.md @@ -0,0 +1,85 @@ +--- +id: review-vendor-extraction +name: Review — the vendor extraction before dependent layers +status: pending +depends_on: [implementation/vendor-differential-tests] +scope: narrow +risk: low +impact: project +level: review +--- + +## Description + +Review checkpoint before any layer builds on the vendor. Everything +else in the crate (params, wire, producer, consumer, doors, backends) +consumes the vendored typestate/codecs, so extraction errors — a +behavior drift vs upstream, a missed cut, a wasm regression — would +propagate into every downstream task. This review is the gate between +the vendor group and the rest of the decomposition. + +Review against the architecture, in this order: + +1. **Extraction completeness vs the cut table** (vendor.md): every + "What is Extracted" row is present; every "What is NOT Extracted" + row is absent. Diff `src/vendor/` against + `/workspace/fast-socks5/src/` file-by-file — the diff must contain + exactly: the extraction cut (deletions per the cut table), the two + carried changes (`new_reply` publication; the BIND surface being + present-and-public), the extraction-time adjustments (anyhow/log + removal, error-variant pruning, macro de-exporting, the + no-unwrap translations), and the provenance/license notes. Any diff + hunk outside those categories is a finding. +2. **Wasm-clean invariant** (ADR-009 §Amendment, ADR-011 §Amendment): + `cargo check --target wasm32-unknown-unknown` and native + `--no-default-features`-equivalent cleanliness; no `net`-shaped + feature exists; `tokio` stays on the wasm-clean subset in + `[dependencies]`. +3. **Convention compliance** (AGENTS.md): no `unwrap`/`expect`/panics + outside tests; `thiserror` error surface; no comments in code + except doc comments and the pinned non-obvious-correctness notes; + naming; module structure (one module per file, `lib.rs` + re-exports). +4. **RFC fidelity spot-check**: run the differential test suite; for + two or three of the trickiest pinned expectations (e.g. the + reply-form byte counts, the method-negotiation preference order), + independently verify the pinned bytes against the reference + checkout and RFC 1928. +5. **License/provenance** (ADR-011): the vendor point (v1.0.0) noted + in-tree; the MIT notice present and referenced. +6. **Docs sync**: `vendor.md`'s extraction table and cut table match + what landed (if the implementer discovered a boundary correction — + e.g. a transitive dependency the cut table missed — the doc was + updated in the same change, not silently diverged). + +## Acceptance Criteria + +- [ ] The file-by-file diff vs upstream contains only + cut-table/carried-change/adjustment/provenance changes — no + unexplained divergence +- [ ] Both wasm checks pass (`cargo check` and + `cargo clippy --target wasm32-unknown-unknown -- -D warnings`) +- [ ] Full verification suite green: `cargo test`, + `cargo clippy --all-targets -- -D warnings`, `cargo fmt --check` +- [ ] No convention violations (unwrap/panics/error surface/naming) +- [ ] License + provenance notes verified present +- [ ] `vendor.md` matches the implemented tree (or the mismatch is + filed as a blocker before proceeding) +- [ ] Findings recorded; critical findings block the downstream task + groups until resolved + +## References + +- docs/architecture/vendor.md (the spec under review) +- docs/architecture/decisions/011-inline-vendor.md +- docs/architecture/decisions/009-feature-gates.md +- AGENTS.md (conventions 1–17; the verification commands) +- /workspace/fast-socks5 (the reference checkout for the diff) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/vendor-client-session.md b/tasks/implementation/vendor-client-session.md new file mode 100644 index 0000000..8401fc9 --- /dev/null +++ b/tasks/implementation/vendor-client-session.md @@ -0,0 +1,123 @@ +--- +id: implementation/vendor-client-session +name: Vendor — the client session (Socks5Stream::use_stream path) +status: pending +depends_on: [implementation/vendor-wire-vocabulary] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Extract the client session — the consumer half's stream-session base +(ADR-002; POC #1's fit) — into `src/vendor/client.rs`: the +`Socks5Stream::use_stream` path plus its `AsyncRead`/`AsyncWrite` +impls, generic over the backing stream. The consumer layer (a later +task group) opens a channels `BiStream` and feeds it here; the +first-party associate session in `consumer.rs` was informed by this +shape (vendor.md's extraction table). + +**What is extracted** (upstream `src/client.rs` → `src/vendor/client.rs`): + +1. `Socks5Stream` struct + the generic impl block, verbatim: + `use_stream`, `request`, `send_version_and_methods`, + `which_method_accepted`, `use_password_auth`, `request_header`, + `read_request_reply`, `get_socket`/`get_socket_ref`/`get_socket_mut`. +2. The `AsyncRead`/`AsyncWrite` impls for `Socks5Stream` (upstream + `client.rs:641–681`) verbatim — the consumer's session rides these + (a `Socks5Stream` is itself a stream, so `pump_bidi`-shaped + consumers and the front doors can pipe through it). +3. The client `Config` struct — **pruned**: upstream carries + `connect_timeout` (only consumed by the cut `connect_raw`) and + `skip_auth` (consumed by `use_stream`). Keep `skip_auth`; drop + `connect_timeout` and its setter (its only consumer is cut). If + dropping it leaves `Config` a single-field struct, keep it anyway + (fidelity: the field is load-bearing in `use_stream`'s handshake + branch) or inline the flag — implementer's choice, but the + `use_stream` behavior must stay byte-identical. + +**What is NOT extracted** (vendor.md's cut table row 2): + +- `Socks5Datagram` (struct + impl — the kernel `UdpSocket` associate + client; the crate's associate session is first-party in + `consumer.rs` per ADR-003's handle swap). +- The `Socks5Stream` convenience impl + (`connect`/`connect_with_password`/`connect_raw` — the + `tokio::net::TcpStream` dialers; the consumer's transport is a + channels `BiStream`, not a TCP dial). + +**Extraction-time adjustments** (mechanical): + +- Drop `anyhow::Context` — upstream wraps nearly every client error in + `.context(...)`. The error surface is `SocksError` (thiserror): + replace `.context("...")?` with plain `?` where the error variant + already carries the meaning, and with the typed variants + (`SocksError::Io`, `AuthMethodUnacceptable`, ...) where upstream + used `anyhow::Error::msg(...).into()` (upstream `client.rs:285` — + the missing-target-addr case) or `SocksError::Other(anyhow!(...))` + (upstream `client.rs:528` — but that site is inside the cut + `Socks5Datagram`, so it goes with the cut). No behavior change to + the wire: the same bytes are read/written; only the error typing + changes. +- Drop `log` calls (`debug!` in the extracted region). +- `use_password_auth`'s `unreachable!()` (upstream `client.rs:207` — + `methods.get(1)` being `Some(AuthenticationMethod::None)`): + convention 2 forbids panics in library code. Replace with a typed + error path (`SocksError::AuthenticationRejected(...)` or an + `AuthMethodUnacceptable`) — the state is reachable only by misuse + of the public API, but the faithful translation is an error return, + not a panic. +- `client_src`-style `"[::]:0".parse().unwrap()` sites are all inside + the cut `Socks5Datagram` region — verify none remain. + +## Acceptance Criteria + +- [ ] `src/vendor/client.rs` exists with the inventory above; + `cargo test` compiles +- [ ] `cargo check --target wasm32-unknown-unknown` clean +- [ ] No `Socks5Datagram`, no `Socks5Stream` convenience + impl (`connect`/`connect_with_password`/`connect_raw`) anywhere + under `src/vendor/` +- [ ] No `anyhow`, `log`, `tokio::net`, `UdpSocket` anywhere under + `src/vendor/` +- [ ] No `unwrap`/`expect`/`unreachable!`/panic paths in the extracted + client code (convention 2) +- [ ] `Socks5Stream::use_stream` over an in-memory duplex completes: + no-auth handshake, password handshake (both accept and reject + paths), CONNECT request + reply read (success and each mapped + failure code), domain-target and IP-target request encoding — + against a raw-byte mock server (pinned byte sequences written + directly, not the vendored server typestate — keeping this + task parallel with the typestate extraction and the client's + wire expectations uncorrelated from the server implementation; + tokio duplex streams, no sockets) +- [ ] `Socks5Stream` implements `AsyncRead` + `AsyncWrite` (the + impls extracted) and the impls are exercised by at least one + test (e.g. `tokio::io::copy` through the session) +- [ ] `cargo clippy --all-targets -- -D warnings` and + `cargo fmt --check` clean + +## References + +- docs/architecture/vendor.md (the extraction table row 5, the cut + table row 2) +- docs/architecture/decisions/002-protocol-base.md (the client side: + `Socks5Stream::use_stream`) +- docs/architecture/decisions/003-udp-handle-swap.md (why + `Socks5Datagram` is cut — the associate session is first-party) +- docs/research/poc-connect-wasm-findings.md §Wire/API notes + (`Socks5Stream::request` target construction; the + `AuthenticationRejected` unit-variant note) +- Upstream: `/workspace/fast-socks5/src/client.rs` +- POC #4 gate map: docs/research/poc-connect-wasm-findings.md + (client.rs row) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/vendor-crate-scaffolding.md b/tasks/implementation/vendor-crate-scaffolding.md new file mode 100644 index 0000000..de38df9 --- /dev/null +++ b/tasks/implementation/vendor-crate-scaffolding.md @@ -0,0 +1,93 @@ +--- +id: implementation/vendor-crate-scaffolding +name: Vendor — crate scaffolding (Cargo.toml, lib.rs, vendor module skeleton, license notice) +status: pending +depends_on: [] +scope: narrow +risk: low +impact: project +level: implementation +--- + +## Description + +Create the `alksocks` crate skeleton so the vendor extraction tasks have +their targets. This is the first implementation work in the repo — `src/` +does not exist yet. The crate is born wasm-clean: the default build is +protocol-only with zero kernel-net code (ADR-009 §Amendment, ADR-011 +§Amendment), so the wasm check must be green from the first commit. + +Deliverables: + +1. **`Cargo.toml`** mirroring the sibling crates' posture + (`/workspace/@alkdev/alktunnels`, `/workspace/@alkdev/alktty`): + - `name = "alksocks"`, `version = "0.1.0"`, `edition = "2021"`, + `rust-version = "1.88"`, `license = "MIT OR Apache-2.0"`, + description/keywords/categories/repository per the siblings. + - `[features]`: `default = []`, `local = []`, `composed = []` — + declared empty now (ADR-009 pins the two-feature layout; the + features are populated by their modules' later tasks; `composed` + will add the alktunnels dep, `local` will add `tokio/net` + + `socket2`). + - `[dependencies]`: `tokio = { version = "1", default-features = false, + features = ["rt", "sync", "io-util", "macros", "time"] }` (the + wasm-clean subset per AGENTS.md convention 4 — **never + `features = ["full"]`**), `thiserror = "2"`, `tracing = "0.1"`. + **No `alkcall` yet** — the vendor does not need it; the + producer/consumer tasks pin `alkcall = "0.8.0"` when they land. + - `[dev-dependencies]`: `tokio = { version = "1", features = ["full", + "test-util", "macros"] }` (tests are native-only; they are not + part of the wasm-clean surface). +2. **`src/lib.rs`**: `#![forbid(unsafe_code)]`, brief crate docs + (what the crate is, pointing at `docs/architecture/overview.md` — + doc comments are fine and expected), `pub mod vendor;`. +3. **`src/vendor/` skeleton** preserving upstream file names so per-file + diffs against the reference checkout stay mechanical (the + maintenance posture, vendor.md §Maintenance Posture): + - `src/vendor/mod.rs` — module doc carrying the provenance note: + extracted from fast-socks5 **v1.0.0** (MIT, third-party upstream), + per ADR-011; the extraction cut + carried changes are recorded in + `docs/architecture/vendor.md`; submodule declarations for `server`, + `client`, `util` (stub files, filled by the extraction tasks). + - `src/vendor/server.rs`, `src/vendor/client.rs`, + `src/vendor/util/mod.rs`, `src/vendor/util/stream.rs`, + `src/vendor/util/target_addr.rs` — empty stubs. +4. **Bundled license notice** at `src/vendor/LICENSE`: the upstream MIT + license text plus attribution (fast-socks5 v1.0.0, + github.com/dizda/fast-socks5), referenced from the vendor module doc + (ADR-011's license obligation — upstream has no per-file headers, so + the bundled notice is the obligation's carrier). + +## Acceptance Criteria + +- [ ] `cargo test` passes (empty crate compiles and runs) +- [ ] `cargo clippy --all-targets -- -D warnings` clean +- [ ] `cargo fmt --check` clean +- [ ] `cargo check --target wasm32-unknown-unknown` clean (the wasm gate + is green from birth — no `mio`/`socket2`/`tokio::net` in the graph) +- [ ] `cargo clippy --target wasm32-unknown-unknown -- -D warnings` clean +- [ ] `Cargo.toml` has no `alkcall`, no `anyhow`, no `log`, no + `async-trait`, no `tokio-stream`, no `socket2` in `[dependencies]` +- [ ] `src/vendor/LICENSE` exists with the MIT text + v1.0.0 attribution, + referenced from the vendor module doc +- [ ] `#![forbid(unsafe_code)]` present in `lib.rs` + +## References + +- docs/architecture/vendor.md (the vendor spec) +- docs/architecture/decisions/011-inline-vendor.md (§Amendment — the + extraction posture, license obligation, wasm gate) +- docs/architecture/decisions/009-feature-gates.md (the feature layout) +- docs/architecture/overview.md §Dependencies (the tokio subset) +- Sibling Cargo.toml posture: `/workspace/@alkdev/alktunnels/Cargo.toml`, + `/workspace/@alkdev/alktty/Cargo.toml` +- Upstream license: `/workspace/fast-socks5/Cargo.toml` (v1.0.0, MIT) + + `/workspace/fast-socks5/LICENSE` + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/vendor-differential-tests.md b/tasks/implementation/vendor-differential-tests.md new file mode 100644 index 0000000..acde409 --- /dev/null +++ b/tasks/implementation/vendor-differential-tests.md @@ -0,0 +1,115 @@ +--- +id: implementation/vendor-differential-tests +name: Vendor — the differential RFC edge-case test suite +status: pending +depends_on: [implementation/vendor-server-typestate, implementation/vendor-client-session] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Build the crate's RFC edge-case test suite for the extracted vendor — +the differential-testing posture vendor.md pins: "the crate's test +suite covers the edge cases the extracted code must keep passing; when +in doubt, diff behavior against the reference checkout." The per-task +unit tests (wire vocabulary, typestate, client session) cover the +happy paths; this task covers the RFC edge cases and the +extraction-fidelity checks, using `/workspace/fast-socks5` as the +behavioral reference (read-only — tests never depend on it at build +time; the reference is consulted while writing the tests, and the +assertions pin the expected bytes). + +Test infrastructure: tokio duplex streams (`tokio::io::duplex`) — +no sockets, no `local` feature, runs in the default wasm-clean crate's +test suite (tests are native-only via dev-deps; the crate itself stays +wasm-clean). A first-party test harness drives the vendored typestate +server against the vendored client session over duplex pairs. + +**Edge-case coverage** (each pinned with explicit expected bytes where +the wire form is the point): + +1. **Negotiation edge cases**: method-list parsing (empty method list, + zero methods offered → 0xff), method-order preference (server picks + its first match, not the client's first offer), version mismatch + (0x04 greeting → refusal), truncated greeting (EOF mid-handshake → + typed error, no hang), password-auth sub-protocol (empty username → + `EmptyUsername`, empty password → `EmptyPassword`, reject path → + 0x01/0xff reply then error). +2. **Command-read edge cases**: unknown command byte (0x04+ → + `UnknownCommand` → 0x07 reply), reserved-byte tolerance (RSV ≠ 0 — + upstream reads it without checking; pin the upstream behavior), + address-type edge cases (ATYP 0x02/0x05+ → `IncorrectAddressType` + → 0x08 reply), domain-length boundary (255-byte domain OK, + 256+ rejected at encode time by `to_be_bytes`), truncated address + fields (EOF mid-address → typed error). +3. **Reply-form edge cases** (the `new_reply` carried change's + regression guard): v4 reply = 10 bytes, v6 reply = 22 bytes, + VER=0x05/RSV=0x00 in every reply, error replies carry + `0.0.0.0:0`-equivalent (ATYP=1, zero addr+port), success replies + carry the caller's `sock_addr` verbatim. +4. **Client-session edge cases**: reply#1 with a non-zero reply code → + the mapped `ReplyError` (the `from_u8` table), domain-target + request encoding (ATYP=3, len-prefixed), the UDP-ASSOCIATE + zero-target fallback (upstream `request_header`'s + `target_addr: None` branch — pin the 10-byte zero form), + `skip_auth` behavior (no method negotiation bytes on the wire). +5. **UDP codec edge cases**: `parse_udp_request` with garbage RSV → + `GarbageInReserved`, FRAG ≠ 0 passthrough (the parser returns frag; + the *caller* refuses — pin the parser's contract), + `new_udp_header`/`parse_udp_request` round-trip for v4/v6/domain. +6. **`transfer`/`wait_on_tcp` semantics**: `transfer` pumps both + directions to completion over duplexes (including half-close + propagation); `wait_on_tcp` returns `EOF` on clean close, + `UnexpectedUdpControlGarbage` on a control byte. + +**Differential method**: for each behavior above, consult the +reference checkout's behavior (its own tests + code reading) and pin +the *same* expectation in-tree, noting the upstream source line in the +test's doc comment where the expectation is non-obvious. Where the +extraction deliberately diverges (the pruned error variants, the +published `new_reply`), the test pins the *in-tree* contract and the +divergence is already recorded in vendor.md — no new divergence may be +introduced by this task. + +## Acceptance Criteria + +- [ ] The six coverage groups above each have at least one test; + all pass in `cargo test` +- [ ] Tests use only duplex/in-memory streams — no `TcpListener`/ + `UdpSocket`/`tokio::net` in test code (the suite runs in the + default-feature build) +- [ ] Every non-obvious expectation carries a doc-comment note naming + the upstream behavior it pins (file + line in + `/workspace/fast-socks5`) +- [ ] `cargo test` (default features) green; + `cargo test --all-features` green (no feature-gated vendor code + exists, so both must pass identically) +- [ ] `cargo check --target wasm32-unknown-unknown` still clean + (tests are dev-only; the crate surface unchanged) +- [ ] `cargo clippy --all-targets -- -D warnings` and + `cargo fmt --check` clean + +## References + +- docs/architecture/vendor.md §Maintenance Posture (the + differential-testing contract) +- docs/architecture/decisions/011-inline-vendor.md (differential + testing guards the RFC edge cases) +- docs/architecture/decisions/002-protocol-base.md (faithful + reply-code mapping) +- Reference checkout: `/workspace/fast-socks5` (v1.0.0 — behavior + source for the pinned expectations; its `src/lib.rs` test module and + `src/server.rs` test module are the upstream coverage to mirror + where extractable) +- RFC 1928 (the wire form's authority) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/vendor-server-typestate.md b/tasks/implementation/vendor-server-typestate.md new file mode 100644 index 0000000..c60b5c5 --- /dev/null +++ b/tasks/implementation/vendor-server-typestate.md @@ -0,0 +1,155 @@ +--- +id: implementation/vendor-server-typestate +name: Vendor — the server typestate (Socks5ServerProtocol + carried changes) +status: pending +depends_on: [implementation/vendor-wire-vocabulary] +scope: narrow +risk: medium +impact: component +level: implementation +--- + +## Description + +Extract the server typestate — fast-socks5's explicit API, the +producer's conversation driver (ADR-002) — into +`src/vendor/server.rs`, and apply the two carried changes that touch +this file (BIND's reply surface + the `new_reply` publication). This +is the highest-fidelity-sensitive extraction: the typestate is the +RFC conversation's state machine, and the producer layer (a later +task group) drives it directly over a channels `BiStream`. + +**What is extracted** (upstream `src/server.rs` → `src/vendor/server.rs`): + +1. `SocksServerError` enum + `to_reply_error` verbatim (the + server-side wire error vocabulary; keep `UnexpectedUdpControlGarbage` + — the ASSOCIATE quiet-wait semantics consume it). +2. `ErrorContext` trait + its two impls (`err_when`) verbatim — + the typestate's internal error-context idiom, used pervasively in + the extracted region. +3. The typestate core, verbatim: + - `states` module (`Opened`/`Authenticated`/`CommandRead`), + `Socks5ServerProtocol` + `new`/`start`. + - `CheckResult` trait + impls. + - `accept_no_auth` / `accept_password_auth` / `finish_auth` / + `skip_auth_this_is_not_rfc_compliant`. + - `AuthMethodSuccessState`, `AuthMethod` traits. + - `NoAuthentication`/`NoAuthenticationImpl`, + `PasswordAuthentication`/`PasswordAuthenticationImpl` (+ the + `password_states` module, `PasswordAuthenticationStarted` alias). + - `auth_method_enums!` macro + `StandardAuthentication` (keep the + macro `#[macro_export]`-free per the same adjustment as + `read_exact!` — `pub(crate) use` scoping; upstream exports it at + the crate root). + - `negotiate_auth`, `read_command`, `reply_success`, `reply_error`. + - `transfer` + `wait_on_tcp` (the typestate's own two-pump seam — + vendor.md's extraction table; `transfer` is the interception-point + shape, `wait_on_tcp` is the ASSOCIATE control-stream reader). + - `try_notify!` macro (internal to the extracted region; same + scoping adjustment). +4. **Carried change #2 — `new_reply` publication** (vendor.md's + carried changes; POC #5 ledger ask #3): the reply encoder becomes + public as a `reply(code, addr)`-shaped constructor. Upstream: + `fn new_reply(error: &ReplyError, sock_addr: SocketAddr) -> Vec` + (private, `server.rs:1380`). In-tree: `pub fn new_reply(...)` + (or `pub fn reply(code: &ReplyError, addr: SocketAddr) -> Vec` + — either name; pin one and use it consistently) so the BIND + reply#2 writer and the consumer's reply readers don't re-encode + the 10/22-byte reply form. The vendored `reply_success`/`reply_error` + call it; nothing else changes. + +**What is NOT extracted** (the cut — POC #4's gate map + the recipe's +two additions; vendor.md's cut table row 1): + +- `run_tcp_proxy`, `run_udp_proxy`/`run_udp_proxy_custom`, + `udp_bind_random_port`, `handle_udp_*`/`transfer_udp` (all + kernel-net; the producer drives the typestate directly — POC #2's + shape — and the local backends are first-party tokio code, ADR-008). +- The legacy `Socks5Server`/`Incoming`/`Socks5Socket` (struct + impls, + including the `AsyncRead`/`AsyncWrite` impls and the `Unpin` impl — + they exist only for the legacy API). +- `DnsResolveHelper` trait + impl (net-gated upstream; the wrapper + resolves nothing — domain targets pass through unresolved, ADR-012). +- The legacy `Config` + `Authentication` trait + impls + (`SimpleUserPassword`/`DenyAuthentication`/`AcceptAuthentication`, + `authenticate_callback`) — the recipe's addition: ungated upstream + but consumed only by the cut legacy region. With it go the + `#[allow(deprecated)]` attributes on the cut impls. +- The net-only test module (`test_bind` — binds a real listener). + +**Extraction-time adjustments** (mechanical): + +- Drop `anyhow::Context` imports and `log` calls (`trace!`/`debug!`/ + `error!`/`warn!` in the extracted region — drop or re-point at + `tracing`; the typestate's traces are per-field debug logs, drop + them; keep behavior identical). +- `reply_error`'s `"0.0.0.0:0".parse().unwrap()` (upstream + `server.rs:945`): replace the `unwrap` with a `const` parsed at + compile time or a documented-infallible construction (AGENTS.md + convention 2 — no `unwrap` outside tests; the parse is statically + infallible, so a `const`/`SocketAddr::new` form is the faithful + translation). +- `Socks5Command::from_u8` returning `Option` + `ok_or` stays (the + unknown-command path maps to `SocksServerError::UnknownCommand` → + `CommandNotSupported`, RFC-faithful). +- Keep `#[allow(async_fn_in_trait)]` only where upstream had it and + the extracted region still needs it (the `DnsResolveHelper` trait is + cut; check whether any remaining trait needs the attribute — + `AuthMethod` is not async, so likely none). + +**The BIND note** (why carried change #1 does not appear here): BIND's +driving is wrapper-side code (POC #5 — "zero upstream ask for BIND +itself"): the typestate's `read_command()` already returns +`TCPBind`, `reply_success(sock_addr)` is reply#1's carrier, and the +reply#2 writer uses the published `new_reply`. The vendor carries the +*shape* (the published encoder + the typestate's existing surface); +the BIND phase-model driving itself is the producer task group's work +(ADR-007), not the vendor's. This task's obligation is only that the +surface BIND needs is present and public. + +## Acceptance Criteria + +- [ ] `src/vendor/server.rs` exists with the inventory above; + `cargo test` compiles +- [ ] `cargo check --target wasm32-unknown-unknown` clean +- [ ] No `run_tcp_proxy`/`run_udp_proxy*`/`udp_bind_random_port`/ + `handle_udp_*`/`transfer_udp`/`Socks5Server`/`Incoming`/ + `Socks5Socket`/`DnsResolveHelper`/`Config`/`Authentication`/ + `authenticate_callback` anywhere under `src/vendor/` +- [ ] No `anyhow`, `log`, `tokio::net`, `socket2`, `tokio-stream`, + `async-trait` anywhere under `src/vendor/` +- [ ] `new_reply` (or `reply`) is `pub` and produces byte-identical + output to upstream's private encoder (10 bytes v4 / 22 bytes v6, + VER/REP/RSV/ATYP/addr/port) — unit-tested +- [ ] `reply_error` contains no `unwrap`/`expect` (convention 2) +- [ ] `accept_no_auth` over an in-memory duplex completes a full + negotiation + `read_command` + `reply_success`/`reply_error` + round-trip against a first-party test client (tokio duplex + streams — no sockets), covering: version mismatch refusal, + no acceptable method (0xff reply), unknown command (0x07 reply), + success reply form, error reply form +- [ ] `cargo clippy --all-targets -- -D warnings` and + `cargo fmt --check` clean + +## References + +- docs/architecture/vendor.md (the extraction table row 1, the cut + table row 1, the carried changes) +- docs/architecture/decisions/011-inline-vendor.md §Amendment +- docs/architecture/decisions/002-protocol-base.md (the explicit + typestate API only) +- docs/architecture/decisions/007-bind-in-base.md (the BIND surface + the vendor must expose) +- docs/research/poc-bind-findings.md §Wire/API notes (`reply_success` + as reply#1 carrier; the `new_reply` finding) +- Upstream: `/workspace/fast-socks5/src/server.rs` +- POC #4 gate map: docs/research/poc-connect-wasm-findings.md + (server.rs row) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/vendor-wire-vocabulary.md b/tasks/implementation/vendor-wire-vocabulary.md new file mode 100644 index 0000000..6f61ca9 --- /dev/null +++ b/tasks/implementation/vendor-wire-vocabulary.md @@ -0,0 +1,134 @@ +--- +id: implementation/vendor-wire-vocabulary +name: Vendor — the wire vocabulary (consts, commands, codecs, errors, macros) +status: pending +depends_on: [implementation/vendor-crate-scaffolding] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Extract the pure wire vocabulary from fast-socks5 v1.0.0 into +`src/vendor/` — the RFC 1928 codecs and error enums that everything +else (typestate, client sessions, producer/consumer layers) consumes. +This is the "codecs" row of vendor.md's extraction table plus the +`lib.rs` non-net region. Copy from `/workspace/fast-socks5` (read-only +reference checkout) and apply the extraction-time adjustments; the +result must be wasm-clean by construction (no `tokio::net`, no +`anyhow`, no `log`). + +**What is extracted** (upstream file → in-tree target): + +1. **`src/lib.rs` → `src/vendor/lib.rs`** (new file — upstream's + `lib.rs` content minus the cut regions): + - `consts` module verbatim (all `SOCKS5_*` constants). + - `Socks5Command` enum + `as_u8`/`from_u8` (make both `pub` — the + producer's command dispatch and the consumer's request writer both + need them; upstream keeps them private with `#[allow(dead_code)]`). + - `AuthenticationMethod` enum + impls (consumed by the client + session's `use_stream` path; keep as-is). + - `ReplyError` enum + `as_u8`/`from_u8` verbatim. + - `UdpHeaderError` enum, `new_udp_header`, `parse_udp_request` + verbatim (the ASSOCIATE datagram codec — vendor.md's codecs row). + - `Result` type alias. + - **NOT extracted**: the net-heavy test module (upstream `lib.rs` + lines 345–520 — it binds real sockets; the crate's own test task + covers these edge cases), the `socks4` module declaration. +2. **`src/util/target_addr.rs` → `src/vendor/util/target_addr.rs`**: + - `AddrError`, `TargetAddr` (+ `is_ip`/`is_domain`/`to_be_bytes`/ + `into_string_and_port`), `ToSocketAddrs` impl, `Display` impl, + `ToTargetAddr` trait + all impls, `Addr` enum, `read_address` — + verbatim. + - **NOT extracted**: the `resolve_dns` method (POC #4's gate map — + it is the one net-touching member; `tokio::net::lookup_host` is + not in the wasm-clean graph). Delete the method; `AddrError:: + DNSResolutionFailed`/`NoDNSRecords` go with it (see adjustments). + - Drop the `debug!` log calls (upstream `log` → dropped or re-pointed + at `tracing`; these are per-octet debug traces — drop them). +3. **`src/util/stream.rs` → `src/vendor/util/stream.rs`**: + - `read_exact!` and `ready!` macros verbatim. **Adjustment**: they + are `#[macro_export]` upstream (crate-root exported, which would + pollute the alksocks public API); convert to plain `macro_rules!` + with `pub(crate) use` scoping inside the vendor module so they + stay internal (the wrapper's own code gets its own macros; the + vendor's are implementation detail). + - `ConnectError` enum + `to_reply_error` verbatim (the reply-code + mapping vocabulary — consumed by the dial seam's error mapping). + - **NOT extracted**: `tcp_connect`/`tcp_connect_with_timeout` (the + `tokio::net::TcpStream` dialers — POC #4's gate map; the crate's + local backend is first-party tokio code, ADR-008). +4. **`SocksError` (in `src/vendor/lib.rs`)** — the error vocabulary, + pruned to what the extracted region + the crate's error surface + actually consumes (vendor.md: "the vendored `SocksError`/ + `SocksServerError` stay the wire-level vocabulary"): + - Keep: `Io`, `AuthMethodUnacceptable`, `UnsupportedSocksVersion`, + `ExceededMaxDomainLen`, `AuthenticationRejected(String)`, + `ServerError(#[from] SocksServerError)`, + `UdpHeaderError(#[from] UdpHeaderError)`, + `AddrError(#[from] AddrError)`, `ReplyError(#[from] ReplyError)`. + - Drop: `Redaction`, `InvalidHeader` (dead upstream — no consumer in + the extracted region), `ConnectError(#[from] ConnectError)` + (its only producer was the cut `tcp_connect*`; `ConnectError` + itself stays — the dial seam maps it), `ReplySocks4Error` (socks4 + is cut), `ArgumentInputError` (no consumer), `Other(#[from] + anyhow::Error)` (anyhow is dropped — vendor.md's adjustments). + - Note: `AuthenticationRejected(String)` on `SocksError` is the + client-side variant (client.rs uses it with a message); + `SocksServerError::AuthenticationRejected` (unit) is the + server-side one — both stay, each in its own enum. + +**Extraction-time adjustments** (mechanical, per ADR-011 §Amendment + +vendor.md): drop `anyhow` wrapping, drop `log` calls, prune +error variants whose only producers/consumers were cut, keep +`thiserror` derives. No behavior change: every kept codec's wire +behavior is byte-identical to upstream v1.0.0. + +## Acceptance Criteria + +- [ ] `src/vendor/lib.rs`, `src/vendor/util/{mod,stream,target_addr}.rs` + exist with the inventory above; `cargo test` compiles +- [ ] `cargo check --target wasm32-unknown-unknown` clean +- [ ] `cargo check` (native, default features) clean +- [ ] No `anyhow`, `log`, `tokio::net`, `socket2`, `tokio-stream`, + `async-trait` anywhere under `src/vendor/` +- [ ] `Socks5Command::as_u8`/`from_u8` are `pub` (the wrapper needs + them; upstream had them private) +- [ ] `read_exact!`/`ready!` are not exported at the crate root (no + `#[macro_export]`); they resolve inside the vendor module +- [ ] `TargetAddr` has no `resolve_dns` method; `AddrError` has no + `DNSResolutionFailed`/`NoDNSRecords` variants +- [ ] `SocksError` has no `Other(anyhow::Error)`, no `Redaction`, no + `InvalidHeader`, no `ReplySocks4Error`, no `ArgumentInputError` + variants +- [ ] `cargo clippy --all-targets -- -D warnings` and + `cargo fmt --check` clean +- [ ] Unit tests for the codecs pass: `to_be_bytes`/`read_address` + round-trip (v4/v6/domain), `new_udp_header`/`parse_udp_request` + round-trip, `ReplyError::as_u8`/`from_u8` mapping table — ported + from upstream's non-net test regions where extractable + (vendor.md's "non-net test modules" row), adjusted to the + crate's test layout (no `env_logger`, no real sockets) + +## References + +- docs/architecture/vendor.md (the extraction table + the cut table) +- docs/architecture/decisions/011-inline-vendor.md §Amendment (the + mechanical adjustments) +- docs/architecture/decisions/002-protocol-base.md (the codecs are the + wire vocabulary; "no second codec, ever") +- Upstream: `/workspace/fast-socks5/src/lib.rs`, + `/workspace/fast-socks5/src/util/target_addr.rs`, + `/workspace/fast-socks5/src/util/stream.rs` +- POC #4 gate map: docs/research/poc-connect-wasm-findings.md + (lib.rs/util rows) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file