Bug fixes, structured logging, config validation and CI

[ayebrian]

Targets main directly so the whole main → dev → feat/zrle chain lands as one PR instead of a stack. Everything on dev and feat/zrle is already contained here (verified with git merge-base --is-ancestor), so both branches become redundant once this merges.

The first six commits are the pre-existing dev + feat/zrle work — the 2.0 config format, image rotation and ZRLE encoding. Everything below is new on top of that.

CI is green on all five checks, and --check, log rotation, the banner and each handshake rejection path were also exercised against a running server.

Bugs fixed

The release pipeline was broken end to end

• install.sh ran go build -o fictusvnc main.go, but the package spans eight files — it failed with undefined: appVersion and seven more. It now builds the package. FLAGS=(-ldflags=-s -w) was also expanded as $FLAGS, silently dropping -w.
• The release workflow pinned Go 1.24 while go.mod required 1.25.0, so every tagged build failed. It also copied a config.toml that is not in the repo, with the failure swallowed by || true, shipping archives without a config — and the binary exits 1 on first run without one. It now packages config.example.toml.
• Tagged releases embedded whatever version was hardcoded in the source rather than the tag.

Resource exhaustion

• No write deadline was ever set. A client that stopped reading (TCP zero-window) pinned a goroutine forever, holding a full per-connection framebuffer copy. Every send is now bounded.
• Incremental FramebufferUpdateRequests were answered with a full frame. Real clients re-request as soon as each update lands, so this spun at 100% CPU and saturated the link redrawing an image that never changes. The first update is still unconditional, so a client never faces a blank screen; later incremental ones are ignored, as a real server does.
• ClientCutText trusted a uint32 length. int(n) goes negative on the 386 targets build.sh produces, making io.CopyN return immediately and desync the stream. Capped at 1 MiB.
• No connection limit existed. New max_connections (default 512, 0 disables) is process-wide, since the memory it protects is shared across listeners.
• The banner was built on connect — a framebuffer copy (~8 MB at 1080p) plus, with rDNS, a DNS query — before a single protocol byte was exchanged. At the default cap that was ~4 GB worst case for clients that might never request a frame. It is now built on the first update request, so scanners that connect and vanish cost neither.

Correctness

• The client IP overlay split RemoteAddr on ":", yielding [ for an IPv6 peer.
• Image paths resolved against the working directory while the config path defaulted to the executable's directory, so a service started outside its install dir found its config but not its images. Images now resolve against the config file's directory.
• With no server able to start, select {} left the process alive and silent forever — systemd saw a healthy service listening on nothing. Listeners bind synchronously, a zero-listener start exits non-zero, and SIGINT/SIGTERM shut down cleanly.
• getSequential did a non-atomic load-then-increment, handing the same image to concurrent connections.
• An unknown message type drained one byte and kept parsing a stream it could no longer interpret.
• The client's RFB version and chosen security type were read and discarded. A malformed greeting is now refused instead of being parsed as protocol, RFB 3.3 is refused explicitly (it puts the server in charge of the security type over a different flow), and a client selecting an unoffered type gets a proper RFB 3.8 failure result.

Observability

Structured logging on log/slog — stdlib, no new dependency. A connection used to produce six unrelated lines behind an unparseable [Acme - Reception] prefix; it now produces one event carrying the whole session:

{"time":"...","level":"INFO","msg":"connection","server":"Reception","listen":"0.0.0.0:5900",
 "peer_ip":"198.51.100.42","peer_port":48280,"handshake":true,"outcome":"client_eof",
 "duration_ms":1049,"bytes_sent":6407,"client_version":"RFB 003.008","security_type":1,
 "image":"default.png","updates":1,"encodings":[16,0,-239],"encoding_used":"zrle"}

Three of those fields were already being read and thrown away. The encoding list in the client's own order is the best available fingerprint of which VNC software is on the other end — the whole point of running a honeypot. outcome is a small stable vocabulary so it groups cleanly, and handshake separates real clients from probes that open a socket and vanish.

New [logging] section: level, format (json/text) and output (stdout, stderr or a file path). Shipping to Elasticsearch or Loki needs no code here — JSON on stdout is what every collector consumes. A file sink is reopened on SIGHUP so logrotate works.

Config

Renamed for clarity; the 2.0 spellings still load and emit a deprecation notice:

Old	New	Note
show_ip	show_client_ip	same meaning
no_brand	branding	inverted, defaults to true
server_name	name	matches the global key

The global name is now actually used as the branding prefix — it was previously unreachable, because the per-server fallback to the section id always won and section ids are never empty.

Banner flags are per-server. show_client_ip, show_rdns and show_time are global defaults that any server can override. Server fields are *bool so three states are distinguishable: inherit, force on, force off — a plain bool cannot express "switch a globally enabled line back off".

Typos are reported. A mistyped key used to be dropped in silence, so the option simply looked broken; a mistyped section name produced a misleading "check listen addresses and image paths". Both are caught now, at every nesting level, including keys inside inline image tables. Settings that silently override one another are flagged too — image with images, a port in listen with a port range, an unrecognised rotation_mode.

--check parses the config, loads every image and prints a summary without binding a port, so it is safe to run on a live host. It exits non-zero for anything that would stop a server starting, including two servers claiming the same address — which previously only surfaced as a bind failure at startup.

Banner rendering

Was a hardcoded 360×22 box in a fixed 13px bitmap face: unreadable on a wallpaper-sized image, and either clipping the text or leaving a wide empty bar. It is now measured from its contents — Go Mono at a size proportional to the image, a box sized to the widest line plus proportional padding, shrinking until it fits the image width, so a full IPv6 address or a long hostname no longer runs off the edge. Optional rDNS and timestamp lines were added. Go Mono and opentype ship inside golang.org/x/image, so this adds no module.

Build and release

• CI (ci.yml, new): gofmt, go vet, go test -race, a go mod tidy diff, and the full build.sh matrix on every PR and push to main. None of this ran before — the only workflow triggered on v* tags, so nothing was checked until release time.
• Rolling dev pre-releases (dev-release.yml, new): every push to main publishes the full platform matrix under a moving dev tag, versioned 2.1.0-dev.g<sha>. The tag moves rather than accumulating one release per commit.
• Packaging moved out of release.yml into a shared package.sh instead of being duplicated between workflows.
• Go 1.26.5; golang.org/x/image 0.41 → 0.44; actions/checkout and setup-go v4 → v7; action-gh-release v2 → v3.

Tests

57 tests, all with -race. Beyond the ZRLE round-trip that already existed: config loading, key deprecation, typo detection, per-server banner inheritance, --check verdicts, listen-address expansion, banner geometry, incremental-request suppression, cut-text limits, unknown messages, every handshake rejection path, IPv6 peer parsing, sequential rotation under concurrency, log rotation across a rename, and connection-record contents.

Several were checked against the pre-fix code and fail there — the rotation race (62–63 distinct images out of 64), the three handshake rejections, and the lazy-overlay test.

Still open

Reported but deliberately not fixed here: the 24bpp colour converter ignores RShift/GShift/BShift and endianness, the 32bpp path masks rather than scales when RMax < 255, there is no /metrics endpoint, and there is no buffered writer.