[nightshift] Tech debt classification: Microck/tailstick

[nightshift-micr]

🏗️ Tech Debt Classification: Microck/tailstick

Task: tech-debt-classify
Category: options
Date: 2026-04-18
Analyzed by: Nightshift v3 (GLM 5.1)
Scope: Go codebase (~170KB), 14 source files across 8 packages

---

Summary

Tailstick is a well-structured Go application for managing ephemeral Tailscale device leases. The codebase demonstrates solid engineering practices: clean package boundaries, good test coverage for core packages, proper error handling with %w wrapping, and defense-in-depth crypto (scrypt + AES-GCM). Tech debt is primarily in architectural patterns, not code quality.

---

Classification by Category

🏛️ Architecture Debt

P1 — Monolithic workflow.go (755 lines)

• File: internal/app/workflow.go
• Description: The workflow.go file contains the entire lease lifecycle (enroll, agent, cleanup, reconciliation, agent installation, secret management) in a single file. The Manager struct has 15+ methods spanning unrelated concerns.
• Recommendation: Split into focused files: enroll.go, agent.go, cleanup.go, secrets.go. The Manager struct can remain the orchestrator but each file should handle one lifecycle phase.
• Effort: Medium (refactoring with no behavior change)

P2 — Duplicated CLI flag definitions

• Files: internal/app/cli.go:42-59, cli.go:105-114, cli.go:148-156
• Description: Runtime paths (configPath, statePath, auditPath, logPath, dryRun) are declared as flags in runEnroll, runAgent, and runCleanup identically. Three separate NewManager() calls duplicate the Runtime construction.
• Recommendation: Extract a commonFlags(fs *flag.FlagSet, rt Runtime) helper that returns a Runtime, reducing boilerplate from ~12 lines per command to ~3.
• Effort: Low (1 helper function + 3 call sites)

P2 — No dependency injection for tailscale.Client

• File: internal/app/workflow.go:77
• Description: tailscale.Client{Runner: runner} is constructed inline in NewManager. This makes unit testing of the Manager difficult (tests must use the real Runner or manually inject).
• Recommendation: Accept tailscale.Client as a parameter to NewManager (or use a functional option pattern), enabling test doubles.
• Effort: Low

🔒 Security Debt

P1 — Config file uses os.ExpandEnv on entire JSON

• File: internal/config/config.go:31
• Description: os.ExpandEnv(string(b)) is applied to the raw config file before JSON parsing. This means ANY ${...} pattern in the config (including inside JSON string values that aren't meant to be env vars) gets expanded. If a password or API key contains $ followed by a valid env var name, it will be silently replaced.
• Recommendation: Use targeted env var expansion only for known fields (authKeyEnv, apiKeyEnv, operatorPasswordEnv) rather than blanket ExpandEnv. This is the intended pattern anyway — the *Env fields exist precisely for this.
• Effort: Medium

P2 — GUI error responses leak internal error messages

• File: internal/gui/server.go:174
• Description: err.Error() is returned directly to the HTTP client on enrollment failure. Internal errors (file paths, system state) could leak to the browser.
• Recommendation: Return sanitized error messages to the client, log the full error server-side.
• Effort: Low

P2 — GUI has no CSRF protection

• File: internal/gui/server.go:132-184
• Description: The /api/enroll POST endpoint accepts requests from any origin. Since the GUI listens on 127.0.0.1, this is lower risk, but any local web page could trigger lease creation.
• Recommendation: Add a CSRF token or Origin header check. For a localhost-only service, checking Origin/Referer headers is sufficient.
• Effort: Low

⚡ Performance Debt

P3 — AgentRun loads state file on every iteration

• File: internal/app/workflow.go:267-270
• Description: After every AgentOnce() call (which itself loads the state file), AgentRun loads the state file again just to check hasActiveManagedLeases. This doubles I/O per iteration.
• Recommendation: Have AgentOnce return whether active leases remain, or cache the result.
• Effort: Low

P3 — No file locking on state.json

• File: internal/state/store.go:37-52
• Description: State file reads and writes have no file locking. If two agent processes run concurrently (e.g., systemd timer race), state corruption is possible.
• Recommendation: Use flock or fcntl advisory locking before read-modify-write cycles. The atomic rename write pattern mitigates partial writes but not lost updates.
• Effort: Medium

🧪 Test Debt

P2 — No integration tests for crypto package

• File: internal/crypto/secret.go
• Description: The crypto package has only a basic test (secret_test.go, 675B ≈ ~20 lines). Given the sensitivity of the encryption (scrypt + AES-GCM for auth keys), this deserves round-trip tests, edge case tests (empty input, max-length input, unicode), and failure mode tests.
• Recommendation: Add tests for: empty plaintext, large plaintext, wrong password, corrupted envelope, missing fields.
• Effort: Low

P2 — No tests for gui package

• File: internal/gui/server.go
• Description: server_test.go (3411B) exists but the GUI server has no endpoint tests. The /api/enroll endpoint has validation logic (mode, channel, days checks) that should be tested.
• Recommendation: Add httptest-based tests for the three endpoints (/, /api/presets, /api/enroll) covering validation edge cases.
• Effort: Medium

P3 — cli.go has no tests

• File: internal/app/cli.go
• Description: CLI command routing, flag parsing, and exit code handling are untested.
• Recommendation: Test RunCLI with various argument combinations to verify correct routing and exit codes.
• Effort: Low-Medium

📝 Documentation Debt

P3 — Version is hardcoded constant, not derived from build

• File: internal/app/cli.go:16
• Description: const Version = "1.0.0" is a hardcoded string. It won't reflect the actual build version unless manually updated.
• Recommendation: Use go build -ldflags "-X ..." to inject the version at build time (the CI already does goreleaser).
• Effort: Low

---

Debt Summary

Priority	Category	Item	Effort
P1	Architecture	Monolithic workflow.go (755 lines)	Medium
P1	Security	os.ExpandEnv on entire config JSON	Medium
P2	Architecture	Duplicated CLI flag definitions	Low
P2	Architecture	No DI for tailscale.Client	Low
P2	Security	GUI error responses leak internals	Low
P2	Security	GUI has no CSRF protection	Low
P2	Test	No integration tests for crypto package	Low
P2	Test	No tests for GUI endpoints	Medium
P3	Performance	AgentRun double-loads state file	Low
P3	Performance	No file locking on state.json	Medium
P3	Test	cli.go has no tests	Low-Medium
P3	Documentation	Hardcoded version constant	Low

Recommended Remediation Order

1. Security first: Fix os.ExpandEnv blanket expansion (P1) + GUI error leaks (P2)
2. Architecture: Split workflow.go into focused files (P1)
3. Testing: Add crypto round-trip tests (P2) + GUI endpoint tests (P2)
4. Hardening: Add CSRF check to GUI (P2) + file locking on state (P3)

---

This analysis was generated automatically by Nightshift v3. Review and prioritize findings at your discretion.

[Microck]

Closing this generated tech-debt classification report as superseded by the targeted fixes now merged in #43.