1.12: reliability & security pass#609
Merged
Merged
Conversation
- race keepalive SendRequest against a timer (default: keepalive interval, override with CHISEL_PING_TIMEOUT) and close the ssh conn on timeout so the client reconnect loop kicks in - replace the time.Sleep ping loop with a stoppable ticker that exits when the connection closes - add unit tests (mock ssh.Conn) and an e2e blackhole-link test, adapted from PR #581 with its broken tcpEcho assertion fixed Closes md task 1 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- watch the parent directory filtered by filename so the watch survives vim tmp+rename, truncate+write, and remove+recreate - detect kubernetes configmap updates via EvalSymlinks retarget check, since those never emit events for the file path itself - debounce reloads (100ms) to collapse event bursts and avoid loading half-written files; invalid JSON keeps the last-good users (existing parse-before-swap behavior, now tested) - supersedes PR #587 (same dir-watch + debounce approach, plus symlink handling, gofmt-clean, and tests) Closes md task 2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- subject socks channels to the ACL using the well-known token "socks" instead of skipping the check, closing the hole where a modified client with --socks5 + --authfile got unrestricted egress - make Remote.UserAddr() return "socks" for socks remotes (was ":") so authfiles can express socks access at config time too - document the authfile migration in --help and README: socks now requires an entry matching "socks"; wildcard "" still matches - add e2e tests: channel denied/allowed by ACL (incl. real SOCKS5 handshake) and config-time socks remote validation Closes md task 4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- flows beyond the conn cap are now marked write-only and swept once idle for UDP_DEADLINE, so the map drains below the cap and new flows regain read slots (was: stored forever, all subsequent flows permanently blackholed, memory unbounded) - make the cap configurable via UDP_MAX_CONNS (default 100) - close conns when read goroutines exit (fd leak: removed from the map but never closed, so closeAll could no longer reach them) - make per-flow write errors non-fatal so an ICMP unreachable or a conn closed by its reader drops one flow, not the whole channel - sweep runs in the single write goroutine, avoiding the timer races in PR #515 which could kill the channel mid-write - unit test for sweep selection + e2e cap/recovery test Closes md task 3 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- closed 11 superseded dependabot PRs (#448 #449 #450 #451 #467 #470 #478 #496 #513 #516 #517) — deps updated via #568/#578 - closed duplicate keepalive PRs #488/#442 with thanks (#581's approach won, implemented in task 1) - closed keepalive issues #445/#560 (fixed by task 1, ships next release); #585/#541 were already closed - left #504/#550/#485 for task 22 and #561/#498 for task 23, as those tasks deliver the fixes that close them - filed task 35: decide dependabot grouped updates vs renovate Closes md task 21 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- replace permissions: write-all with top-level contents: read; release job alone gets contents: write + packages: write - scope push triggers to master and v* tags, ending duplicate test runs on PR branches - merge release_binaries/release_docker into one goreleaser job (pinned v2.12.7) publishing multi-arch images to GHCR + Hub - Dockerfile now copies the goreleaser-built binary, so images carry the correct stamped version (fixes the #417 version mismatch from git-describe in a tagless checkout); WORKDIR /app kept for volume-mount compat - adopted from PR #584 with the above trigger/permission fixes; verified via goreleaser check + local image build (--version inside the container matches the stamped snapshot version) Closes md task 20 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cio.Pipe now propagates half-closes: clean EOF in one direction CloseWrites the destination while the other direction keeps flowing; copy errors still tear down both ends, and Pipe closes both conns before returning (no caller changes needed) - rwcConn delegates CloseWrite to the underlying stream so the SOCKS path half-closes through ssh channels (from PR #548) - exit side dials the target before accepting the ssh channel and rejects it on failure (ssh.ConnectionFailed), so inbound clients see a prompt close instead of a live conn to a dead target - dial is context-bound (tunnel teardown cancels it) with a 30s default timeout, tunable via CHISEL_DIAL_TIMEOUT - adapted from PRs #536/#548/#538; dropped #538's accept-loop serialization and error-text-into-stream reset behavior - e2e: half-close echo (fails on old Pipe, verified), channel rejection for dead targets, prompt local-conn close; unit test for rwcConn CloseWrite delegation Closes md task 19 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- InterruptContext also catches SIGTERM (docker stop, kubernetes) and registers the handler before returning, closing the startup window where SIGTERM took the default kill path; a second signal forces immediate exit - GoServe context cancellation now runs http.Server.Shutdown with a grace period (SHUTDOWN_GRACE, default 5s) draining in-flight requests before force-closing; hijacked websocket tunnels are unaffected and close with their owners - drop dead listenErr field; explicit Server.Close() (tests, chserver.Close) stays immediate - supersedes PR #564 (same SIGTERM idea, minus its premature "shutdown complete" logging) - tests: SIGTERM cancels the context (unix), in-flight request drains during shutdown while new conns are refused Closes md task 18 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- SetReadLimit inside NewWebSocketConn so both server and client are covered at every call site; default 64KB (ssh packets are <= ~35KB), tunable via WS_READ_LIMIT, 0 disables - oversized frames now error the read and drop the connection instead of buffering unbounded attacker data before ssh auth - unit tests for limit/normal reads; e2e: unauthenticated peer sending a 5MB frame is disconnected mid-frame Closes md task 17 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- BuildVersion falls back to debug.ReadBuildInfo Main.Version, so 'go install github.com/jpillora/chisel@latest' reports v1.x.y instead of 0.0.0-src; ldflags-stamped builds are unchanged and source builds keep the 0.0.0-src marker - skip the client/server version-mismatch log when either side is a dev/unknown build, ending the recurring 0.0.0-src confusion - unit tests for the fallback rules Closes md task 22 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add Proxy.Close() releasing the tcp/udp listener for proxies which were bound but never Run - BindRemotes closes proxies[0..i-1] before returning a bind error, so a failed (reverse) client config no longer orphans server ports for the process lifetime - test: first port is rebindable after a partial bind failure Closes md task 5 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- authUser returns the username via ssh.Permissions.Extensions and handleWebsocket re-resolves it, deleting the sessions side-map: no more entries leaked by aborted handshakes or config timeouts, and no panic when an authfile reload races the Len()==0 check - a user removed by a reload mid-handshake now gets a logged close instead of a panic or stale access - constant-time password comparison (crypto/subtle) - fix never-formatted literal %s in the auth error message - unit tests for authUser (valid/wrong/unknown/allow-all) Closes md task 6 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- connectionLoop returns an error when --max-retry-count is exhausted so the process exits non-zero (systemd/scripts can react); ctx-cancelled shutdown still exits 0 - backoff Min raised 100ms -> 1s and made configurable via new --min-retry-interval flag (adopted from PR #537, sans jitter); no more reconnect stampede when a popular server restarts - explicit sub-second --max-retry-interval values are now honored instead of silently raised to 5 minutes; max below min is raised to min with a warning; 5m remains the unset default - guard err != nil before err.Error(); pass error text as %s args rather than format strings (three sites) - tests: interval defaults/explicit/inversion, give-up error Closes md task 10 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- auth strings without a colon are now a fatal config error on both server and client; previously they silently disabled auth (and e2e TestAuth had been running authless this whole time by passing a file path as --auth — now fixed to real creds) - --auth user is pinned in the UserIndex and survives authfile reloads instead of being dropped by the first Reset; pinned users win name clashes with file users - per-channel ACL re-resolves the user from the live index, so authfile reloads apply to connected clients' new tunnels and removed users are denied immediately (#549); established tunnels are not interrupted - documented in --help and README - tests: pinned user survives reload, invalid auth (both sides), e2e channel denial after user removal Closes md task 8 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- session open logs user, remote addr and declared remotes at info level; close logs user, addr, duration and error - connected clients are now visible without debug mode (#530, #501) - failed logins log at info with the source address (#521) - the optional /metrics endpoint from the task is intentionally deferred: metric/flag naming is long-term API surface Closes md task 29 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- L4Proto lowercases the returned proto to match its case-insensitive regex; '1.1.1.1:53/UDP' previously decoded fine then failed at listen time with 'unknown local proto' - add uppercase suffix case to the remote decode table test Closes md task 12 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- NewServer's four key paths (read file, chisel-key convert, seed generate, ssh parse) now return errors; main.go already fatals on them, and library embedders (#542) keep their process - the invalid-key error no longer echoes raw key material - drop the now-unused log import; test both error paths Closes md task 14 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- help: remote-host default corrected to 127.0.0.1, --keyfile inline-key example fixed (#498 — keygen output needs no extra base64), 'fallsback' typo, --fingerprint tab indentation, --backend documents its --proxy alias, client --proxy documents URL-encoded credentials (#396) - README rendered via md-tmpl from the fixed help texts - demo section: dead Heroku app (#561) replaced with self-hosted fly.io instructions using example/fly.toml - install one-liner verified working today (PR #562 not needed); microbadger badge link and dead Google App Engine tracker link replaced - new guides: TLS setup (#533), reverse SOCKS with an authfile (#518, ACL token verified as R:127.0.0.1:1080), CDN/Cloudflare fronting notes (#490) - new section documenting all CHISEL_* environment knobs Closes md task 23 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- socks5:// joins socks:// and socks5h:// (same SOCKS5 dialer); it was the most common spelling yet the only one rejected (#474) - help/README note the schemes are equivalent: DNS is always resolved by the proxy - test covers all accepted schemes, http CONNECT, and rejection Closes md task 24 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- server_listen: move the LetsEncrypt port-443 warning into the hasDomains branch where it was intended (it was dead under hasKeyCert) and drop the always-true err==nil (PR #546) - fix user-facing 'forwaring' error (PRs #588/#575) and README .akp->.apk (PR #528); comment typos: represent, received, acquired, extracts, fairly straightforward, successfully, that's, doesn't - add codespell job to CI with .codespellrc (PR #430 idea), verified clean locally with codespell 2.4.2 Closes md task 25 Co-authored-by: celestix <celestix@users.noreply.github.com> Co-authored-by: edoardottt <edoardottt@users.noreply.github.com> Co-authored-by: iMarble <iMarble@users.noreply.github.com> Co-authored-by: mjtrangoni <mjtrangoni@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- legacy MD5 fingerprints must now match the full 16-octet colon form; the old prefix match let a truncated fingerprint verify against ~1 in 65k keys - authfile loads log a warning for unanchored address patterns (anchoring by default would be breaking) - --help/README now state loudly that patterns are unanchored, show an anchored example, and note "" matches everything Closes md task 26 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Makefile: linux/windows now CGO_ENABLED=0 matching releases; dep installs tools via 'go install tool@latest'; build dir is an order-only target instead of mkdir on every parse - migrate //+build tags to //go:build (cos signal/pprof files) - drop deprecated net.Error.Temporary() from the udp read loop - client_test: replace log.Fatal with t.Fatal and stop calling t.Fatal from the http handler goroutine (record + assert on the test goroutine instead) - remove dead pipeVis code from cio.Pipe - new Client.Ready(ctx) readiness API (via Tunnel.Ready) replaces the e2e 50ms startup sleep; setup logs rather than fails so negative tests (rejected client certs) still run their asserts Closes md task 27 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- client.Ready covers the handshake but reverse remotes bind on the server concurrently afterwards; the removed 50ms sleep had been masking that window, making TestReverse flaky under -race - setup now polls each tcp tunnel listener until it accepts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- modern Go sets the floor (Windows 10/Server 2016, macOS 12, Linux 3.2, FreeBSD 12.2); point older systems (#576, Win7) at v1.8.1, the last release built before Go dropped them - partial progress on md task 34 (remaining items are feature decisions, parked) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- tasks 28/30/31/32/33/34/35 are inprogress awaiting decisions (announced individually); all executable tasks are closed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- summarizes the reliability/security pass: keepalive timeouts, live authfile reloads, socks ACL (breaking note), half-close, SIGTERM drain, UDP flow fixes, ws read caps, client exit codes, version fallback, goreleaser docker images Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- features: reconnect bullet covers keepalive dead-conn detection and retry tuning; docker section lists GHCR alongside Docker Hub - security: legacy MD5 full-form requirement, pre-auth ws size cap - authentication: live reload semantics, unanchored-pattern warning, breaking socks token migration, fatal invalid auth strings, pinned --auth user - socks5 guide cross-references the authfile socks token - signals help (rendered into README) documents SIGINT/SIGTERM graceful shutdown with second-signal force exit Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- golang.org/x/crypto 0.48.0 -> 0.53.0 - golang.org/x/net 0.50.0 -> 0.56.0 - golang.org/x/sync 0.19.0 -> 0.21.0 - github.com/fsnotify/fsnotify 1.9.0 -> 1.10.1 (indirect: x/sys 0.46.0, x/text 0.38.0) - actions/checkout v5 -> v7 - docker/setup-qemu-action, setup-buildx-action, login-action v3 -> v4 Covers the eight applicable open dependabot PRs (598-605); the ninth (597, docker/build-push-action) is moot since the goreleaser CI rework dropped that action. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A denied channel was only visible with -v, which makes the 1.12 socks-token authfile migration hard to diagnose server-side: operators saw nothing while end users saw generic SOCKS/connect failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add 'Upgrading to 1.12' section consolidating the four migration items (socks grant, full fingerprints, auth colon, exit codes) - changelog: list the fingerprint and auth-colon breaking changes alongside the socks one - help text (main.go + README copy): document that legacy MD5 fingerprints are accepted only in full 16-octet form - env table: note PING_TIMEOUT is inert when --keepalive is 0 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Post-rebase follow-up to ba4dac1, matching the current open dependabot PRs: - golang.org/x/crypto 0.53.0 -> 0.54.0 (#605, #604) - golang.org/x/net 0.56.0 -> 0.57.0 (#604; supersedes #607's 0.55.0) - golang.org/x/sync 0.21.0 -> 0.22.0 (#603, #604) (indirect: x/sys 0.46.0 -> 0.47.0, x/text 0.38.0 -> 0.40.0) With this and the rebase pulling in #606 (crypto 0.52.0 to master), all open dependabot PRs (597-607) are now covered or superseded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR is the unreleased 1.12 line: a broad reliability/security hardening pass across the tunnel transport, auth/ACL enforcement, shutdown behavior, version reporting, CI/release packaging, and documentation—while keeping the wire protocol compatible (chisel-v3).
Changes:
- Hardens tunnel behavior (keepalive ping timeouts, TCP half-close propagation, pre-accept dial w/ timeout, UDP flow-cap recovery).
- Tightens security/auth surfaces (WS pre-auth read limit, SOCKS ACL gating via
sockstoken,--authvalidation, constant-time password compare, live authfile reload resilience). - Refreshes tooling/docs (version stamping fallback, Go module bumps, CI permissions + codespell, GoReleaser multi-arch Docker images, README upgrade guidance).
Reviewed changes
Copilot reviewed 56 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/ws_limit_test.go | E2E coverage for pre-auth WebSocket message size cap. |
| test/e2e/udp_cap_test.go | E2E coverage for UDP flow-cap recovery via idle sweep. |
| test/e2e/setup_test.go | Replaces fixed sleeps with readiness + listener availability waits. |
| test/e2e/keepalive_test.go | E2E coverage for reconnect after dead-path keepalive timeout. |
| test/e2e/halfclose_test.go | E2E coverage for TCP half-close + dial-failure propagation. |
| test/e2e/auth_test.go | Updates auth test to reflect --auth validation behavior. |
| test/e2e/acl_socks_test.go | E2E coverage for SOCKS ACL enforcement (channel + config). |
| test/e2e/acl_reload_test.go | E2E coverage for per-channel ACL re-resolution after user removal. |
| TASKS.md | Adds/updates task log documenting reliability/security work items. |
| share/version.go | Adds module-version fallback when ldflags stamping absent. |
| share/version_test.go | Unit tests for version fallback logic. |
| share/tunnel/tunnel.go | Adds Ready() and keepalive ping timeout behavior; unbind-on-failure. |
| share/tunnel/tunnel_out_ssh.go | Enforces ACL on socks token; dial-before-accept w/ dial timeout. |
| share/tunnel/tunnel_out_ssh_udp.go | Adds UDP conn cap configurability and idle sweep for write-only flows. |
| share/tunnel/tunnel_out_ssh_udp_test.go | Unit test for UDP write-only sweep behavior. |
| share/tunnel/tunnel_keepalive_test.go | Unit tests for keepalive timeout vs healthy ping behavior. |
| share/tunnel/tunnel_in_proxy.go | Adds Proxy.Close to unbind listeners on partial bind failure. |
| share/tunnel/tunnel_in_proxy_udp.go | Fixes spelling + removes deprecated Temporary() usage. |
| share/tunnel/tunnel_bind_test.go | Unit test verifying BindRemotes cleans up after partial failure. |
| share/settings/users.go | Improves authfile watching (dir watch, debounce, symlink swap detection) + pinned users + unanchored regex warnings. |
| share/settings/users_watch_test.go | Tests for authfile watcher behaviors (rename, symlink swap, invalid JSON, pinned users). |
| share/settings/remote.go | Normalizes /UDP suffix to lowercase proto; SOCKS UserAddr token. |
| share/settings/remote_test.go | Adds coverage for uppercase /UDP remote parsing. |
| share/cos/signal.go | Updates build tags + fixes log spelling. |
| share/cos/signal_windows.go | Updates build tags for Windows variant. |
| share/cos/pprof.go | Updates build tags for pprof build. |
| share/cos/common.go | Adds SIGTERM support + second-signal forced exit path. |
| share/cos/common_test.go | Tests SIGTERM cancels InterruptContext (non-Windows). |
| share/cnet/http_server.go | Adds graceful HTTP shutdown drain with configurable grace period. |
| share/cnet/http_server_test.go | Tests graceful drain behavior and refusal of new connections post-cancel. |
| share/cnet/conn_ws.go | Adds WebSocket read limit (pre-auth memory-DoS hardening). |
| share/cnet/conn_ws_test.go | Tests WebSocket read limit and normal-sized message handling. |
| share/cnet/conn_rwc.go | Adds CloseWrite propagation for half-close support. |
| share/cnet/conn_rwc_test.go | Tests CloseWrite delegation behavior. |
| share/cio/pipe.go | Reworks Pipe to propagate half-closes instead of full teardown on EOF. |
| server/server.go | Removes log.Fatal key loading; validates --auth; pins auth user; constant-time password compare; removes session map by using ssh.Permissions. |
| server/server_test.go | Tests NewServer returns key-load errors instead of log.Fatal. |
| server/server_listen.go | Fixes LetsEncrypt warning placement + removes redundant err check. |
| server/server_handler.go | Resolves user from ssh permissions; live per-channel ACL resolution; improved version mismatch logic; info-level open/close logging. |
| server/server_auth_test.go | Tests authUser behavior, allow-all behavior, and invalid auth strings. |
| README.md | Major docs refresh: 1.12 upgrade notes, env knobs, authfile semantics, SOCKS ACL token, Docker publishing, etc. |
| Makefile | Aligns CGO defaults with release builds; modernizes dep tool installs; ensures dist dir target. |
| main.go | Updates help text for new behavior/options and corrected wording. |
| go.sum | Updates sums for bumped dependencies. |
| go.mod | Updates dependencies (fsnotify + x/*) and indirects. |
| example/users.json | Adds SOCKS token example to authfile sample. |
| client/client.go | Adds retry interval min/max defaults; validates auth string; tightens legacy fingerprint match; supports socks5:// proxy; adds Ready(). |
| client/client_test.go | Fixes incorrect test patterns (t.Fatal in handler goroutine, log.Fatal usage). |
| client/client_retry_test.go | Tests retry interval defaults/overrides and non-zero exit on give-up. |
| client/client_proxy_test.go | Tests accepted proxy schemes and rejection of unsupported ones. |
| client/client_fingerprint_test.go | Tests rejection of truncated legacy MD5 fingerprint prefixes. |
| client/client_connect.go | Adds backoff Min; ensures exhausted retries return error; fixes unsafe err handling and formatting. |
| client/client_auth_test.go | Tests client rejects invalid auth string without colon. |
| .github/workflows/ci.yml | Scopes CI permissions, adds codespell, upgrades actions, consolidates release with GoReleaser + multi-registry logins. |
| .github/goreleaser.yml | Adds multi-arch Docker image buildx + manifest publishing. |
| .github/Dockerfile | Switches to GoReleaser-provided binary; simplifies build stages. |
| .codespellrc | Configures codespell skips/ignores. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+54
to
+61
| //CloseWrite propagates half-closes to the underlying | ||
| //connection when supported (e.g. ssh.Channel) | ||
| func (c *rwcConn) CloseWrite() error { | ||
| if cw, ok := c.ReadWriteCloser.(closeWriter); ok { | ||
| return cw.CloseWrite() | ||
| } | ||
| return nil | ||
| } |
Comment on lines
112
to
127
| func (u *UserIndex) addWatchEvents() error { | ||
| watcher, err := fsnotify.NewWatcher() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if err := watcher.Add(u.configFile); err != nil { | ||
| configPath, err := filepath.Abs(u.configFile) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| //watch the parent directory instead of the file itself, so the watch | ||
| //survives editors and orchestrators which replace the file rather | ||
| //than write to it (vim tmp+rename, truncate+write, kubernetes | ||
| //configmap symlink swaps) | ||
| if err := watcher.Add(filepath.Dir(configPath)); err != nil { | ||
| return err | ||
| } |
The 1.12 line's user-facing review — breaking changes, quiet behavior changes, library-API surface, mixed-version compatibility, and dependency posture. Also published at https://jpillora.com/chisel-112-ux-review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rwcConn always defines CloseWrite(), so it satisfies the closeWriter interface cio.Pipe uses to choose half-close vs full-close. When the wrapped stream had no CloseWrite the method returned nil (a no-op), so Pipe never took its full-close fallback and the peer never saw EOF — the opposite io.Copy could block forever. Fall back to Close() instead. Flagged by the PR #609 review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two authfile-watcher fixes from the PR #609 review: - fsnotify's directory watch does not deliver kubelet-style symlink swap events on every platform (notably macOS/kqueue), so the ConfigMap-swap reload silently failed there (TestWatchSymlinkSwap failed on macOS CI). Reconcile the resolved path on a 1s ticker as a fallback. - addWatchEvents leaked the fsnotify watcher (fd + goroutine) when filepath.Abs or watcher.Add failed after NewWatcher succeeded; close it on those error paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When a client disconnects between the SSH handshake and its config request, the closed request channel yields a nil *ssh.Request and the handler panicked on r.Type (recovered by net/http, killing the handler and orphaning its goroutines). Guard the nil and close the connection instead. Regression test reproduces the panic via a handshake-then-close client and watches the global logger for net/http's recovered-panic report. Fixes the critical half of #608; the keepalive CPU-spin half was already fixed on this branch by the ping-timeout rework. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stage 1 (existing release job, on v* tag after merging to master): goreleaser creates a draft GitHub release and pushes version-tagged multi-arch Docker images. The floating manifests (latest / X / X.Y) are removed from goreleaser so nothing user-facing moves at tag time. Stage 2 (new promote.yml, on publishing the draft release): retags the already-pushed version manifests to latest / X / X.Y on GHCR and Docker Hub via 'docker buildx imagetools create' — a manifest copy, no rebuild. Prereleases are skipped; workflow_dispatch with a tag input is the manual escape hatch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…allback Two low-severity items from the pre-release review: goreleaser now replaces an existing draft when the release job is re-run for the same tag (instead of stacking a second draft), and rwcConn.CloseWrite's full-close fallback documents its terminate-vs-hang trade-off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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
This is the unreleased 1.12 line: a broad reliability and security pass across the client, server, and tunnel surfaces, plus a documentation refresh and a dependency update. 31 commits, +3,104 / −314 across 57 files, including ~1,000 lines of new end-to-end tests.
The wire protocol is unchanged (
chisel-v3) and no handshake changed, so 1.12 mixes cleanly with v1.11.x peers in both directions (details below).📄 Full user-facing review — breaking changes, quiet behavior changes, library-API surface, and mixed-version compatibility, each verified against the code: https://jpillora.com/chisel-112-ux-review
Three for existing users — all now flagged in the README changelog and the new "Upgrading to 1.12" section:
--socks5+--authfilenow requires an authfile entry matching the tokensocks(wildcard""still works). Enforcement itself shipped in v1.11.7; this branch adds the previously-missing documentation (server/client help, auth section, SOCKS guide, changelog). Denials now log at info asDenied connection to <dest> (ACL), so post-upgrade breakage is self-diagnosing without-v.--fingerprint— truncated legacy MD5 fingerprints are rejected; must be the full SHA256 form (the full 16-octet MD5 colon form is still accepted but deprecated). Closes a ~1-in-65k prefix-match spoofing window.--authwithout a colon is now a fatal startup error (invalid auth string, expected <user>:<pass>) instead of silently disabling authentication.Plus one change that is silent for automation:
chisel clientwith--max-retry-countnow exits non-zero when attempts are exhausted (Ctrl-C / ctx-cancel still exit 0). Scripts checking$?and systemdRestart=on-failureunits will notice. It is in the changelog.Reliability
CHISEL_PING_TIMEOUT, default = keepalive interval), so sleep/wake, NAT timeouts, and server restarts reconnect promptly instead of sitting in 15–60 min of kernel-retransmit limbo.CHISEL_DIAL_TIMEOUT, 30s);shutdown(SHUT_WR)propagates through the tunnel, fixing netcat-style pipelines and rsync. Falls back to full-close against old peers.CHISEL_SHUTDOWN_GRACE, 5s); a second signal force-exits.CHISEL_UDP_DEADLINE, 15s) instead of being blackholed past 100 concurrent flows.--min-retry-interval(default 1s).--authuser is pinned across reloads.BindRemotesfailure unbinds the listeners it already opened.Security
CHISEL_WS_READ_LIMIT, 64KB; 0 disables) — guards against an unauthenticated memory-DoS.--authstring validation,--authuser pinned across reloads and wins name clashes, auth-session map removed.log.Fatalremoved from key loading —NewServerreturns errors instead (also a win for embedders); bad--keyfileno longer echoes raw key material into logs.Observability
Open (user=… addr=… remotes=…)/Close (… duration=…)andLogin failed for user X (ip)(finally fail2ban-able). Heads-up: log parsers keyed on the old debugClosed connectionline need updating.Smaller fixes & niceties
socks5://accepted for--proxy; uppercase3000/UDPparses;go installbuilds report their real version.New env-var knobs
CHISEL_PING_TIMEOUT--keepalive 0)CHISEL_DIAL_TIMEOUTCHISEL_SHUTDOWN_GRACECHISEL_UDP_DEADLINECHISEL_WS_READ_LIMIT--min-retry-intervalDependencies & CI
x/crypto 0.54.0,x/net 0.57.0,x/sync 0.22.0,fsnotify 1.10.1(indirectx/sys 0.47.0,x/text 0.40.0).go build,go vet, andgo mod verifypass.actions/checkout v7, dockersetup-qemu/setup-buildx/login-actionv4); scoped workflow permissions; releases now ship goreleaser-built multi-arch Docker images to GHCR and Docker Hub with correctly stamped versions.masterand is included here via rebase.Mixed-version compatibility
socks5://is client-local).Testing
New end-to-end suites cover the load-bearing changes: keepalive/ping-timeout, half-close, SOCKS ACL enforcement + live reload, UDP flow cap, WS read limit, and auth. Full suite +
go vet+-raceare green.🤖 Generated with Claude Code