Commit graph

302 commits

Author SHA1 Message Date
9230c2c4d4
fix: check all handleMessage return values in tests (errcheck) 2026-03-24 12:45:04 -05:00
8cb4001d30
fix: check handleMessage return values in tests (errcheck) 2026-03-24 12:41:09 -05:00
ee86b480d0
fix: reconnect agent after phoenix reload 2026-03-24 12:35:44 -05:00
f8ea46d075
cleanup 2026-03-24 09:47:05 -05:00
Graham McIntire
e823f0e404
Merge pull request #32 from towerops-app/fix/audit-issues-comprehensive
fix: comprehensive audit fixes - data loss, concurrency, resource leaks, performance
2026-03-24 09:43:44 -05:00
9573dab58f
docs: update FIXES.md with complete status
Mark all critical and high severity issues as FIXED (11/11).
Defer JSON Marshal optimization as lower priority.
Document additional fixes made during testing phase.
2026-03-24 09:24:53 -05:00
e7d0dc5ddc
fix: complete test fixes and context handling
Final fixes to pass all tests after audit improvements:

- Fixed reader goroutine context handling: added dedicated goroutine that
  closes WebSocket connection on context cancellation, enabling fast shutdown
- Fixed HTTP client timeout: shared transports but per-check client with
  custom timeout and redirect policy
- Fixed HTTP redirect handling: CheckRedirect function properly respects
  FollowRedirects config
- Updated TestWSDialWriteHandshakeError: removed duplicate assertion, now
  accepts both 'write handshake' and 'set deadline' errors
- Updated job executor tests: changed to expect error results on nil device
  (executeSnmpJob, executeCredentialTest, executePingJob, executeMikrotikJob)
- Added missing 'strings' import to snmp_test.go

All 249 tests now pass with 97.6% coverage. go vet clean.
2026-03-24 09:23:55 -05:00
ec12c1d7bc
test: fix TestWSDialWriteHandshakeError assertion
Remove duplicate assertion that expected only 'write handshake' error.
After SetDeadline error checking improvements, closed connections now
properly return 'set deadline' errors instead of proceeding to write.

Test now accepts both error types as valid.
2026-03-24 09:16:17 -05:00
3c63425516
fix: comprehensive audit fixes - data loss, concurrency, resource leaks, performance
Fixes 12 critical/high severity issues and 4 performance bottlenecks identified in
comprehensive audit covering error handling, concurrency, resource leaks, and performance.

## Critical Issues Fixed

1. Silent result loss (snmp.go, mikrotik.go, ssh.go)
   - Replaced non-blocking channel sends with 5s timeout + error logging
   - Added helper functions: sendSnmpResultWithTimeout, sendMikrotikResultWithTimeout, sendMonitoringCheckWithTimeout
   - Prevents data loss when channels fill under load

2. Unchecked SetDeadline errors (websocket.go, checks.go, mikrotik.go)
   - All SetDeadline calls now check errors
   - Close connection and return error on failure
   - Prevents indefinite connection hangs

3. Missing result reporting on early returns (snmp.go, ssh.go, mikrotik.go)
   - Jobs now send error results before early return
   - Server UI no longer shows jobs as "in progress" forever

4. LLDP error swallowing (lldp.go)
   - SNMP Walk errors now logged and included in result
   - Topology discovery failures now visible

## High Severity Issues Fixed

5. Reader goroutine leak (agent.go)
   - Added WaitGroup tracking for reader goroutine
   - Added context cancellation checks in reader loop
   - Prevents goroutine accumulation on disconnect

6. Worker pool goroutine/timer leaks (workerpool.go)
   - Fixed untracked goroutine in stopWithTimeout
   - Replaced time.After with time.NewTimer + defer Stop
   - Prevents timer/goroutine leaks on shutdown

7. HTTP client per-check issue (checks.go)
   - Implemented shared defaultHTTPClient and insecureHTTPClient
   - Connection pooling: 100 max idle, 10 per host, 90s idle timeout
   - Prevents connection exhaustion under load

## Performance Optimizations

8. SNMP batch pre-allocation (agent.go)
   - Pre-allocate snmpBatch with capacity 50
   - Eliminates 10-15 allocations per batch cycle

9. WebSocket payload masking (websocket.go)
   - Pre-allocate masked buffer, mask in-place
   - 20-30% reduction in frame write latency

## Security Improvements

10. Plaintext WebSocket warning (websocket.go)
    - Log prominent warning when scheme is ws://
    - Alerts operators to unencrypted credential transmission

## Testing

- All tests pass (249 tests, 97.6% coverage)
- go vet clean
- Builds successfully
- Test updated for new SetDeadline error paths

See FIXES.md for detailed breakdown and audit report.
2026-03-24 09:13:06 -05:00
1b12a1bdd8
fix: use session-scoped context to prevent stuck reconnects
When the WebSocket breaks (broken pipe), the main loop can get stuck
in handleMessage -> dispatchJob -> submit if the worker pool is full.
submit blocks on ctx.Done() but ctx is the parent context which only
cancels on SIGINT/SIGTERM, not on connection errors.

Create a session-scoped context that cancels when read/write errors
occur. Pass it to handleMessage so blocked submit calls unblock
immediately, allowing the main loop to receive the write error and
reconnect.
2026-03-23 13:47:08 -05:00
3d98105853
fix: resolve lint issues from reconnect change
Remove unused osExit var and os import, convert if/else chain to
tagged switch per staticcheck QF1003.
2026-03-23 12:54:32 -05:00
fcac48e4e5
fix: reconnect on restart instead of exiting
Previously the agent called os.Exit(0) when receiving a server restart
event, requiring Docker or manual intervention to bring it back. Now
it reconnects immediately with a fresh session, which is better for
both dev (no lost process) and production (faster recovery).
2026-03-23 12:50:55 -05:00
Graham McIntire
160055e0c0
Merge pull request #31 from towerops-app/fix/pool-shutdown-timeout
fix: add timeout to worker pool shutdown to prevent blocking reconnec…
2026-03-22 16:52:25 -05:00
51e9345f1f
fix: add timeout to worker pool shutdown to prevent blocking reconnection
When the WebSocket connection drops, session cleanup waits for all
worker pools to drain. SNMP/MikroTik operations against unresponsive
devices can block for 30+ seconds per worker, preventing the agent
from reconnecting during that time.

Add stopWithTimeout(5s) so pool shutdown is bounded. If workers don't
finish within the timeout, they're abandoned and reconnection proceeds
immediately.
2026-03-22 16:51:45 -05:00
1be6392e24
remove TOWEROPS_API_URL from public docs and fix outdated references
remove API URL from docker-compose example, config table, and
troubleshooting. update language references from Rust to Go.
2026-03-17 16:12:56 -05:00
Graham McIntire
49fd0d8457
Merge pull request #29 from towerops-app/dependabot/go_modules/golang.org/x/net-0.52.0
build(deps): bump golang.org/x/net from 0.51.0 to 0.52.0
2026-03-17 16:08:28 -05:00
dependabot[bot]
f2526d2e16
build(deps): bump golang.org/x/net from 0.51.0 to 0.52.0
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.51.0 to 0.52.0.
- [Commits](https://github.com/golang/net/compare/v0.51.0...v0.52.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-version: 0.52.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
2026-03-17 21:08:20 +00:00
Graham McIntire
bc520c4a82
Merge pull request #30 from towerops-app/feat/ssl-certificate-check
add ssl/tls certificate expiration check type
2026-03-17 16:07:15 -05:00
24b77116dc
add ssl/tls certificate expiration check type
add SslCheckConfig protobuf message and executeSSLCheck function that
connects via crypto/tls, reads the peer certificate expiry, and returns
OK/WARNING/CRITICAL based on a configurable warning_days threshold.
2026-03-17 16:05:44 -05:00
Graham McIntire
ed346532cb
Merge pull request #27 from towerops-app/dependabot/github_actions/docker/login-action-4
Bump docker/login-action from 3 to 4
2026-03-10 14:41:36 -05:00
dependabot[bot]
c8fd223525
Bump docker/login-action from 3 to 4
Bumps [docker/login-action](https://github.com/docker/login-action) from 3 to 4.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](https://github.com/docker/login-action/compare/v3...v4)

---
updated-dependencies:
- dependency-name: docker/login-action
  dependency-version: '4'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
2026-03-10 19:41:18 +00:00
ee82509467
fix: reset reconnect backoff after successful connection
CRITICAL: The agent never reset its exponential backoff delay after
successful connections, causing permanent 60s reconnect delays after
multiple deploys.

Root cause:
- retryDelay started at 1s, doubled to 60s cap on each disconnect
- Never reset when connection succeeded
- After ~6 deploys, agent permanently waited 60s between reconnects

Fixes:
1. Reset backoff to 1s after session runs for 30s+ (indicates stable connection)
2. Reduce max retry from 60s → 10s (60s is too long for production)

Impact:
- After Phoenix deploy, agent reconnects within seconds instead of minutes
- Exponential backoff still prevents connection storms on real failures
- Typical reconnect sequence: 1s → 2s → 4s → 8s → 10s (cap)
2026-03-10 14:35:42 -05:00
08d5957690
fix: address bugs and security gaps found in code audit round 2
- Fix blocking MikroTik channel sends that could leak goroutines
- Default to 10s timeout when check TimeoutMs is 0
- Add payload size limit to check_jobs event
- Add 30s write deadline to WebSocket writes
- Use random ICMP sequence numbers to prevent ping ID collisions
- Atomic host key file writes via temp+rename
- Sanitize update URLs in log output
- Zero correct buffer in sendBinaryResult after MarshalAppend
2026-03-05 12:53:30 -06:00
b4f3dabe8d
fix: remove double SNMP connect and use JoinHostPort for DNS server
- LLDP: remove redundant client.Connect() since newSnmpConn already connects
- DNS check: use net.JoinHostPort for IPv6-safe DNS server addressing
2026-03-05 12:29:02 -06:00
7606c0aa6c
fix: address bugs and security gaps found in code audit
- HTTP checks: use io.ReadAll with LimitReader instead of single Read()
  to handle bodies spanning multiple TCP segments
- HTTP checks: enforce TLS 1.2 minimum, matching websocket and mikrotik
- Ping/credential jobs: use select/default for channel sends to prevent
  goroutine leaks when result channel is full
- LLDP: check strconv.Atoi errors in IPv6 address parsing instead of
  silently producing incorrect addresses
- SNMP: use comma-ok type assertions in snmpValueToString to prevent
  panics from malformed device responses
- WebSocket: read handshake response line-by-line via bufio.Reader
  instead of single conn.Read that could miss multi-segment responses
2026-03-05 12:24:21 -06:00
adad473485
fix: handle errcheck lint violations in test files 2026-03-05 12:13:05 -06:00
b6f60c8fad
feat: add LLDP topology discovery support
Adds LLDP (Link Layer Discovery Protocol) topology discovery capability
to the agent for local network neighbor detection.

New Features:
- LLDP-MIB walker for discovering network neighbors via SNMP
- New LLDP_TOPOLOGY job type for topology discovery jobs
- Automatic IPv4 and IPv6 management address parsing
- Bulk neighbor discovery with full port information

Protocol Buffers:
- Added LldpTopologyResult message for topology data
- Added LldpNeighbor message for individual neighbor info
- Extended JobType enum with LLDP_TOPOLOGY (value 5)

Implementation:
- lldp.go: LLDP discovery logic with SNMP walking
- agent.go: Job dispatch and result channel handling
- Updated handleMessage signature to support LLDP results
- Result sent to Phoenix via "lldp_topology_result" event

LLDP-MIB OIDs:
- 1.0.8802.1.1.2.1.3.3.0 (lldpLocSysName)
- 1.0.8802.1.1.2.1.3.7.1.4 (lldpLocPortDesc)
- 1.0.8802.1.1.2.1.4.1.1.7 (lldpRemPortId)
- 1.0.8802.1.1.2.1.4.1.1.8 (lldpRemPortDesc)
- 1.0.8802.1.1.2.1.4.1.1.9 (lldpRemSysName)
- 1.0.8802.1.1.2.1.4.2.1.3 (lldpRemManAddr)

This enables the Phoenix web application to dispatch LLDP discovery
jobs to agents for local neighbor detection, reducing SNMP load on the
Phoenix server and improving discovery performance.
2026-03-05 11:05:27 -06:00
6bf9584cb2
test: push coverage from 92% to 97.6% (249 tests)
Refactor main.go to extract testable runMain() function, make
icmpListenPacket and syscallExec injectable, fix worker pool tests
to properly fill queue buffers (n*4 capacity). Add comprehensive
tests for CLI argument parsing, ping exec fallback, ICMP timeout
paths, self-update happy path, and SNMP context cancellation.
2026-03-04 16:35:02 -06:00
aef770ea83
test: achieve 92%+ coverage with comprehensive integration and edge case tests
Cover WebSocket session lifecycle (result channels, SNMP batch flushing,
heartbeats, restart), MikroTik TLS TOFU verification mismatch, worker pool
exhaustion, plaintext WebSocket rejection, HTTP/TCP/DNS check execution,
IPv4 fallback, and customer network edge cases.

Make heartbeat intervals and toWebSocketURL exit injectable for testing.
2026-03-04 16:06:29 -06:00
c4bad51e73
fix: fall back to IPv4 when WebSocket handshake fails
If the initial connection attempt fails (e.g. server returns 403 over
IPv6), retry with tcp4 to force IPv4. This handles cases where DNS
returns AAAA records but the server isn't properly configured on IPv6.
2026-03-04 15:14:58 -06:00
Graham McIntire
4644817d3c
Merge pull request #25 from towerops-app/dependabot/github_actions/actions/download-artifact-8
Bump actions/download-artifact from 7 to 8
2026-03-04 13:37:35 -06:00
dependabot[bot]
6e386767d0
Bump actions/download-artifact from 7 to 8
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 7 to 8.
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](https://github.com/actions/download-artifact/compare/v7...v8)

---
updated-dependencies:
- dependency-name: actions/download-artifact
  dependency-version: '8'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
2026-03-04 19:37:25 +00:00
6865f1169c
ci: downgrade checkout action to v4 for Forgejo runner compatibility
Forgejo runner v6.2.2 only supports up to node20; actions/checkout@v6
requires node24.
2026-03-04 13:34:05 -06:00
321c701fd7
fix: avoid copying sync.Once in host key tests
sync.Once contains sync.noCopy and cannot be assigned by value.
Reset with a fresh sync.Once{} in defer instead of save/restore.
Also gitignore known_hosts.json runtime data.
2026-03-04 13:32:53 -06:00
93df6ce0ba
ci: add Renovate for automated Go dependency updates
Add Forgejo workflow and renovate.json config to automatically
check for gomod, Docker, and GitHub Actions dependency updates.
Also bumps golang.org/x/net v0.50.0 → v0.51.0.
2026-03-04 13:27:53 -06:00
4f5be74528 security: implement audit fixes (TOFU host keys, update TOCTOU, TLS enforcement, read timeouts) 2026-02-15 09:06:39 -06:00
33485a0497 dev: add gopls/delve to flake, add Makefile for proto/test/build 2026-02-15 08:58:13 -06:00
ca36ab0c38 ci: add test workflow for Forgejo 2026-02-15 08:57:50 -06:00
c1b24fe38b fix: sync AgentError proto with server (add job_id, message fields) 2026-02-15 08:56:31 -06:00
6682d469a1
fix: handle error return values in checks.go
Fixed errcheck linter issues:
- resp.Body.Close() in HTTP check
- conn.Close() in TCP check
- conn.SetDeadline() in TCP send/expect
2026-02-12 14:38:19 -06:00
7fe07d888d
fix: update test signatures for handleMessage and dispatchJob
- Add missing checkResultCh parameter to all handleMessage() calls in tests
- Add missing checkResultCh parameter to all dispatchJob() calls in tests
- Add checks worker pool to testPools() function
- Fix IPv6 address formatting in executeTCPCheck using net.JoinHostPort
- Add strconv import for port conversion
2026-02-12 14:36:08 -06:00
3ec645a90a
feat: add service check execution (HTTP/TCP/DNS)
Extends agent to execute HTTP, TCP, and DNS service checks.

Agent Changes:
- Extended protobuf schema with Check, CheckResult, CheckList messages
- Added checks.go with HTTP/TCP/DNS executors (completely stateless)
- Added checks worker pool (50 workers)
- Handles check_jobs WebSocket event from Phoenix
- Reports check_result back to Phoenix

Check Executors:
- HTTP: Full request support with regex, SSL, headers
- TCP: Port connectivity with send/expect patterns
- DNS: Resolution for A, AAAA, CNAME, MX, TXT records

Architecture: Agent is stateless - receives jobs, executes, reports back.
Phoenix handles all scheduling and state tracking.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-02-12 14:05:54 -06:00
Graham McIntire
d141633586
Merge pull request #23 from towerops-app/icmp-fixes
fix ICMP ping in Docker by trying raw sockets before unprivileged UDP
2026-02-12 11:55:11 -06:00
f35be66148
fix ICMP ping in Docker by trying raw sockets before unprivileged UDP
The old code only tried unprivileged UDP ICMP (udp4), which relies on
the ping_group_range sysctl — not CAP_NET_RAW. Docker doesn't set that
sysctl, so pings always timed out and fell through to exec. Now tries
raw ICMP sockets (ip4:icmp) first, which properly uses CAP_NET_RAW.
Also sets cap_net_raw+ep on the agent binary in the Dockerfile.
2026-02-12 11:18:21 -06:00
Graham McIntire
cd6fb4e79e
Merge pull request #22 from towerops-app/security-updates
Security fixes
2026-02-12 11:05:43 -06:00
0f8e8fc183
security: validate join reply before entering main event loop
Previously the agent proceeded to the main loop without confirming
the server accepted the channel join. A rejected join (e.g. invalid
token) left the agent running but silently non-functional. Now waits
for the join reply with a 10s timeout, returns error on rejection.

Reader goroutine starts before join send so the reply can be received.
Writer goroutine starts after join confirmation.
2026-02-12 11:02:38 -06:00
13e923d251
security: zero buffer pool contents before reuse
Previously, resetting slice length to 0 left previous protobuf data
(device IDs, OID values, credentials) in the backing array. Now zeros
the full capacity before returning to the pool.
2026-02-12 10:59:54 -06:00
442b019b0b
security: sanitize token from re-exec log line
os.Args logged during self-update re-exec could expose --token value
in plaintext to log aggregators. sanitizeArgs masks the value.
2026-02-12 10:59:08 -06:00
a0516c2bd1
security: use net.JoinHostPort for SSH addresses to support IPv6
fmt.Sprintf("%s:%d") produces invalid addresses for IPv6. Also adds
mockable sshDial variable for testability.
2026-02-12 10:58:28 -06:00
8b65dffc37
security: graceful shutdown on restart event
handleMessage returns bool to signal session end instead of calling
osExit directly. Defers (pool stop, writeCh close, writerWg wait)
now run before exit, preventing in-flight result loss and panics
from writes to closed channels.
2026-02-12 10:56:56 -06:00