fix: remediate audit findings across desktop, sync, agent, discovery, relay, net, core - #51
Merged
Merged
Conversation
- mailbox: drain an item queued between a failed pop and the last sender's drop instead of closing over it (close/pop race). - local down-pump: track encoded/event tap liveness independently so a dead tap cannot report Finished while its sibling is still live. - session: file SetBitrate into a requested slot the pacing task drains and steers into the controller — a viewer target survives past one 250ms tick instead of being overwritten by the next adapt step. - session: bound input injection by the frame deadline and drop a wedged worker instead of stalling the whole control plane on a hung X server. - client: bound spawn_blocking decode by the frame-stream deadline; a decoder that never returns re-baselines on a fresh chain + IDR instead of stalling uni demux for every service on the connection. - capture/x11: refuse root depths without a 32bpp pixmap format at construction and fail honestly on a GetImage byte-count mismatch. - input/x11: drop the dead static-sink inject() footgun; scroll fan-out reuses one pointer_on_screen check instead of per-click re-queries. - codec/openh264: IDR detection now requires a real Annex-B start code. - run_desktop_client: headless builds print that decoding is absent instead of looking inert.
- push_chunks: the receiver's Done can legitimately arrive while chunk streams still finish; consume it as an early ack and validate the root through one shared recv_done path instead of reporting a spurious "unexpected control frame" abort. - Transfer::serve: a failed control.finish must not mask the transfer error that triggered it — warn and return the original cause. - session_open: a HelloAck with a wrong transfer ID now reports the ID mismatch rather than a phantom version failure. - JournalSink: the disk-job permit rides with each queued chunk instead of the sink's lifetime — N concurrent receives no longer pin N of the process's 32 blocking slots while waiting on the network. - Offer/Need/Done waits now use a 900s phase bound: they gate on the peer's heavy local work (manifest hashing, journal walk, assemble) rather than wire speed; the session deadline still bounds a dead peer.
- Refusal HelloAck writes on the greeting path are wrapped in the documented deadlines (hello for policy/scope refusals, authz for authorization-path answers) and explicitly finished — a stalled peer can no longer park the task mid-refusal. - AgentPolicy::validate now checks authz and shutdown timeouts inside the same 1..=3600s contract as handshake/hello. - Agent::serve runs the same policy preflight as run — programmatic callers previously bypassed it entirely. - TCP target parse and permit checks move to preflight, before a service slot is consumed — a refused connect no longer holds a data lane while its refusal is written. - ResourceGate: the first sample no longer depends on subtracting an interval from Instant::now (fresh-process underflow); max_rss_mb saturates at the byte ceiling instead of silently disabling the gate. - macOS fd accounting uses proc_pidinfo(PROC_PIDLISTFDS) — the true fd table without /dev/fd's fdesc-mount dependency or the read_dir fd skew. - sync_paths scope entries normalize by splitting on both separators, matching check_scope_path's admitted spellings — a `a\b` scope can now actually match the `a/b` requests it was minted for. - Docs clarify Ping/Info are never gated by policy but remain subject to a grant's service scope.
- Record/DeleteRequest verify now peeks the leading schema version after signature check but before committing to a typed decode — a payload minted by another schema reports "unsupported record version N" instead of an opaque serde failure (the root cause of the mixed- deployment `400 record malformed` incident). - EndpointKey::from_str accepts 64-digit hex alongside base32, matching rds_core::EndpointId's dual spelling — `--directory-allow` no longer rejects the hex that `rds id` prints. - Remove Client::metrics: it called `/v1/metrics`, a route the service dropped when metrics moved to the admin listener — it could only ever return 404. - Connection and worker task groups now count panicked tasks into a new rds_directory_task_panics_total metric instead of silently reaping them (maintenance already accounted failures via gc_failures). - decide() retirement semantics reviewed: the retired revision floor is deliberate and correct — same-revision+same-digest on a retired entry reports Expired, conflicts report Stale.
…rate The iroh relay backend admitted every endpoint when --allow was empty, while the owned backend required entries or an explicit dev-open opt-in. Copied production configs silently ran open relays. Both backends now share one admission contract: --allow entries or --development-open-relay. Allowlist keys are validated like directory enrollment — weak ed25519 keys and duplicates are rejected at startup instead of silently holding dead entries. The iroh backend also gains the owned relay's per-client envelope via upstream's implemented client_rx knob (64 MiB/s + 4 MiB burst); upstream exposes no connection-count cap, which the docs now state. Direct serve() callers get an explicit open-access warning. An allowlist e2e test proves the full path: a registered identity reaches its home relay while an unlisted one reports auth_denied_reason and never connects. Shared runtime fixtures opt into development-open where they exercise lifecycle or TLS validation rather than admission, and the former dev-open rejection case becomes the dev-open/allowlist conflict. Docs: relay-runtime backend table and deployment admission section describe the unified fail-closed contract.
…e opens Wire-error hygiene: refusal reasons crossing the stream now use fixed vocabulary — tcp connect failures report io::ErrorKind, sync refusals report coarse causes, and desktop unavailability no longer leaks capability-probe internals. Real errors still reach the local log via the propagating bail. Structural config checks: rds --registry-epoch is Option<NonZeroU64> (requires --registry-key, so an epoch can no longer be silently ignored), the managed-session rejection now tests presence rather than the 1 sentinel, and the agent schema validator rejects zero authority epochs from either flags or config file — Authority::new caught them only at bind time. effective_services is now the single honest service source: an explicit services list containing desktop on a headless build was reported as enabled while Info and the directory record disagreed; the explicit arm now filters Desktop by the build feature like the implicit arm does. File opens: read_rotations, --directory-ca and relay PEM reads used plain opens that block forever on FIFOs and never checked file type. All three now use NONBLOCK opens plus the regular-file check, matching the posture agent config and grant files already had. Tickets: parse_target accepted unbounded base32 bodies; anything over 4 KiB is refused before decode allocates an unbounded address set.
- Incoming::remote_address returns Option: the transient state between accepting a raw attempt and owning its handshake previously answered 0.0.0.0:0, which masquerades as a peer address - parse_target surfaces the real ticket decode error for rds1-prefixed input instead of shadowing it with an endpoint-id complaint - regression tests cover malformed, oversized and non-ticket targets Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- rustls/tokio-rustls lose the tls12 feature: every TLS peer here is our own rustls binary and QUIC is 1.3-only; 1.2 has no legitimate client - remove the unused workspace async-trait declaration and the never enabled, never gated rds-core/desktop feature - compact the redb record store on open so expired/deleted records reclaim pages instead of growing the file monotonically; a failed compact is maintenance, not a fatal open error - document the exact russh pin rationale in the manifest itself - semver-compatible lockfile refresh Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…n docs - vector.toml safe transform admitted only 12 of the 16 event names the pipeline emits — session_opened, session_closed, path_migrated and request_refused were silently dropped by the assert; add them plus the typed reason field they carry, with allowlist tests - rds-server.service still described --allow as "empty = open relay"; since the relay now fails closed, document the required allowlist and the --development-open-relay opt-in - rds-agent.service gains MemoryDenyWriteExecute for parity with the server unit (no JIT anywhere in the binary) - observability.md field table gains reason; deployment.md records that --directory-allow also accepts lowercase hex keys Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- effective_services filter used a literal-bool match that newer clippy folds into matches! - sys.rs open_fds qualified std::mem::size_of, which is prelude in edition 2024 (the arm is macOS-cfg so only the macOS lane saw it) Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Nine-wave remediation of the full-estate audit (10-agent review of all
crates, docs and ops surfaces). Each commit is atomic per wave:
SetBitrate, input deadline, decode timeout, X11 depth
session_open ID mismatch, journal permit per queued store, 900s phase
bounds
validate() gaps, serve() preflight order, macOS FD probing
EndpointKey acceptance, dead metrics() removal
--development-open-relay opt-in, iroh client RX bound (64 MiB/s,
4 MiB burst), weak/duplicate allowlist rejection, allowlist E2E
malformed-ticket error preservation, special-file opens
(NOFOLLOW|NONBLOCK), NonZero authority epochs
rds1 ticket errors no longer shadowed by endpoint-id errors
rds-core/desktop feature, redb compaction on open, lockfile refresh
reason field (was silently dropping 4), stale open-relay unit comment,
agent MemoryDenyWriteExecute parity
Test plan
Generated with Devin