feat(auth): add HMAC-SHA256 authenticated ingest channel (Issue #8) by SacrilegeTx · Pull Request #11 · kiosvantra/metronous

SacrilegeTx · 2026-04-08T13:57:03Z

Summary

Add HMAC-SHA256 authentication to /ingest endpoint to prevent telemetry spoofing
Plugin generates and reads shared secret from {dataDir}/ingest.key
Server supports both plain token and HMAC signature verification
Use hmac.Equal() for timing-safe comparison to prevent timing attacks
Add tests for 1MB payload limit and file-based token loading

Changes

metronous-plugin.ts

Generate/read shared secret from {dataDir}/ingest.key (64-char hex)
Sign request body with HMAC-SHA256
Send X-Metronous-Auth: sha256:<signature> header

internal/mcp/server.go

Read token from METRONOUS_INGEST_TOKEN env or {dataDir}/ingest.key file
Support both plain token and HMAC signature (sha256:... format)
Use timing-safe comparison (hmac.Equal()) to prevent attacks
Reject requests with 401 if auth invalid

internal/mcp/server_test.go

Add 4 auth sub-tests (no auth -> 401, plain token -> 200, HMAC -> 200, invalid -> 401)
Add test for 1MB payload limit (413 response)
Add test for file-based token loading

Testing

All tests pass (25 tests including new auth tests)
Reviewed with Judgment Day (two independent judges)

…install - Add --force flag to allow reinstall when service already exists - Add validateDataDir() to check data directory is writable - Add validateBinary() to verify binary location - Add validateBinaryPath() as alias for compatibility - Add checkPortConflict() to detect port 8844 conflicts - Add validatePermissions() to verify service installation permissions - Add getDefaultPort() returning 8844 - Fix race condition: wait 2s after Stop() and 2s after Uninstall() before proceeding This enables 'metronous install --force' for reinstall scenarios on Windows.

- Add logging to mcp_shim_windows.go ensureDaemonRunning function - Add logging to server.go to show serve mode selection - Add detailed logging to metronous-plugin.ts readPortFile and waitForServer This helps diagnose why the OpenCode plugin cannot connect to the MCP server.

- TestShimPortFilePath: verifies correct port file path format - TestReadShimPortSuccess: tests reading a valid port file - TestReadShimPortInvalid: tests error on invalid port content - TestReadShimPortOutOfRange: tests port validation (1-65535) - TestReadShimPortMissing: tests error on missing file

- Replace fixed 2s sleeps with polling for service status (waitForServiceStop, waitForServiceUninstalled) - Replace checkPortConflict() (useless - checked hardcoded port 8844 that daemon never uses) with checkDaemonRunning() that reads mcp.port file and performs real health check - Add handleServiceExists() for idempotent install without --force flag - Add cleanupServiceFiles() to remove mcp.port, daemon.lock, daemon.pid after uninstall - Daemon uses dynamic ports via net.Listen('tcp', '127.0.0.1:0'), not fixed 8844

- These are user-specific development rules - Not all developers share the same environment (Windows/PowerShell vs Linux) - Each developer can have their own AGENTS.md without affecting the repo

…antra#8) - Add HMAC-SHA256 authentication to /ingest endpoint - Plugin generates and signs shared secret from {dataDir}/ingest.key - Server supports both plain token and HMAC signature verification - Use hmac.Equal() for timing-safe comparison to prevent attacks - Add tests for 1MB payload limit and file-based token loading - Two-pass review with Judgment Day

Copilot

Pull request overview

Adds an HMAC-SHA256 authenticated channel for the /ingest HTTP endpoint so telemetry submissions can be verified and spoofing reduced.

Changes:

Plugin generates/loads a shared secret from {dataDir}/ingest.key and signs ingest request bodies with X-Metronous-Auth: sha256:<signature>.
Server verifies ingest auth using either a plain token or HMAC signature, and can load the secret from env or ingest.key.
Adds/updates tests for auth behaviors, 1MB payload rejection, and file-based token loading.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 7 comments.

File	Description
metronous-plugin.ts	Generates/loads shared secret and signs `/ingest` requests via HMAC-SHA256 header.
internal/mcp/server.go	Implements token/file loading and HMAC/legacy token verification for `/ingest`.
internal/mcp/server_test.go	Adds subtests for auth modes, payload-size rejection, and ingest.key-based token loading.
.gitignore	Ignores user-specific rule files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T14:02:02Z

 func (s *Server) ingestHandler(ctx context.Context) http.HandlerFunc {
+	// Try to get token from env, fall back to reading from ingest.key file
 	expectedToken := os.Getenv(ingestAuthEnvVar)
+	if expectedToken == "" {
+		// Try to read from ingest.key file
+		keyPath := s.ingestKeyPath()
+		if data, err := os.ReadFile(keyPath); err == nil {
+			expectedToken = strings.TrimSpace(string(data))
+		}
+	}
+
 	return func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodPost {
 			http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
 			return
 		}



expectedToken is captured once when the handler is registered. If METRONOUS_INGEST_TOKEN is unset and {dataDir}/ingest.key is created/rotated after server start (which is likely since the plugin lazily creates it on first request), the server will continue running with expectedToken == "" (auth bypass) or a stale secret (auth failures). Resolve the token inside the returned handler per-request (or implement safe caching with reload) and consider validating the file format (e.g., 64 hex chars) before enabling auth.

Suggested change

func (s *Server) ingestHandler(ctx context.Context) http.HandlerFunc {

// Try to get token from env, fall back to reading from ingest.key file

expectedToken := os.Getenv(ingestAuthEnvVar)

if expectedToken == "" {

// Try to read from ingest.key file

keyPath := s.ingestKeyPath()

if data, err := os.ReadFile(keyPath); err == nil {

expectedToken = strings.TrimSpace(string(data))

}

}

return func(w http.ResponseWriter, r *http.Request) {

if r.Method != http.MethodPost {

http.Error(w, "method not allowed", http.StatusMethodNotAllowed)

return

}

func (s *Server) resolveIngestToken() string {

expectedToken := os.Getenv(ingestAuthEnvVar)

if expectedToken != "" {

return expectedToken

}

keyPath := s.ingestKeyPath()

data, err := os.ReadFile(keyPath)

if err != nil {

return ""

}

expectedToken = strings.TrimSpace(string(data))

if expectedToken == "" {

return ""

}

if len(expectedToken) != 64 {

s.logger.Warn("ignoring invalid ingest key file",

zap.String("path", keyPath),

zap.Int("length", len(expectedToken)))

return ""

}

if _, err := hex.DecodeString(expectedToken); err != nil {

s.logger.Warn("ignoring invalid ingest key file",

zap.String("path", keyPath),

zap.Error(err))

return ""

}

return expectedToken

}

func (s *Server) ingestHandler(ctx context.Context) http.HandlerFunc {

return func(w http.ResponseWriter, r *http.Request) {

if r.Method != http.MethodPost {

http.Error(w, "method not allowed", http.StatusMethodNotAllowed)

return

}

expectedToken := s.resolveIngestToken()

Copilot · 2026-04-08T14:02:02Z

+//     token in the X-Metronous-Auth header (exact match).
+//   - If the token is not set, requests are accepted without authentication (legacy mode).
+//
+// The header value can be either:
+//   - Plain token (backward compatibility): exact match against METRONOUS_INGEST_TOKEN


The authentication doc comment says auth is only required when METRONOUS_INGEST_TOKEN is set, but the implementation also enables auth when {dataDir}/ingest.key exists. Update the comment to describe the file-based secret behavior so it matches the actual logic.

Suggested change

// token in the X-Metronous-Auth header (exact match).

// - If the token is not set, requests are accepted without authentication (legacy mode).

//

// The header value can be either:

// - Plain token (backward compatibility): exact match against METRONOUS_INGEST_TOKEN

// token in the X-Metronous-Auth header.

// - Otherwise, if {dataDir}/ingest.key exists and contains a token, requests MUST

// include that matching token in the X-Metronous-Auth header.

// - If neither source provides a token, requests are accepted without authentication

// (legacy mode).

//

// The header value can be either:

// - Plain token (backward compatibility): exact match against the configured secret

Copilot · 2026-04-08T14:02:02Z

+				// Compute expected HMAC
+				mac := hmac.New(sha256.New, []byte(expectedToken))
+				mac.Write([]byte(bodyStr))
+				expectedSig := hex.EncodeToString(mac.Sum(nil))
+				if hmac.Equal([]byte(sig), []byte(expectedSig)) {


HMAC verification compares hex strings via hmac.Equal([]byte(sig), []byte(expectedSig)) without validating/decoding sig. This rejects equivalent uppercase hex and doesn’t ensure the signature is the correct length/hex. Decode sig with hex.DecodeString, verify it’s 32 bytes, and compare raw MAC bytes with hmac.Equal.

Suggested change

// Compute expected HMAC

mac := hmac.New(sha256.New, []byte(expectedToken))

mac.Write([]byte(bodyStr))

expectedSig := hex.EncodeToString(mac.Sum(nil))

if hmac.Equal([]byte(sig), []byte(expectedSig)) {

providedSig, err := hex.DecodeString(sig)

if err != nil || len(providedSig) != sha256.Size {

s.logger.Warn("invalid HMAC signature in ingest request",

zap.String("remote_addr", r.RemoteAddr))

http.Error(w, "invalid authentication signature", http.StatusUnauthorized)

return

}

// Compute expected HMAC

mac := hmac.New(sha256.New, []byte(expectedToken))

mac.Write([]byte(bodyStr))

expectedSig := mac.Sum(nil)

if hmac.Equal(providedSig, expectedSig) {

Copilot · 2026-04-08T14:02:03Z

+				bodyBytes, err := io.ReadAll(http.MaxBytesReader(w, r.Body, 1024*1024))
+				if err != nil {
+					http.Error(w, "failed to read request body", http.StatusBadRequest)
+					return


When http.MaxBytesReader hits the limit, it returns *http.MaxBytesError. Right now this path returns 400, but callers/tests expect a 413 for oversized payloads (especially for chunked requests where ContentLength is -1). Detect *http.MaxBytesError here and return StatusRequestEntityTooLarge.

Copilot · 2026-04-08T14:02:03Z

+	// Set token via file
+	tokenPath := dataDir + "/ingest.key"
+	token := "test-token-64-chars-xxxx-xxxx-xxxx-xxxx-xxxx-xxxx-xxxx-xxxx"
+	_ = os.WriteFile(tokenPath, []byte(token), 0600)
+


Avoid ignoring the os.WriteFile error in tests; if it fails, the test may pass/fail for the wrong reason. Also prefer filepath.Join(dataDir, "ingest.key") over string concatenation for portability.

Copilot · 2026-04-08T14:02:03Z

+	// Write token to file instead of env var
+	tokenPath := dataDir + "/ingest.key"
+	token := "file-based-token-64-chars-xxxx-xxxx-xxxx-xxxx-xxxx-xxxx"
+	_ = os.WriteFile(tokenPath, []byte(token), 0600)
+


Avoid ignoring the os.WriteFile error in tests; if it fails, the test may pass/fail for the wrong reason. Also prefer filepath.Join(dataDir, "ingest.key") over string concatenation for portability.

Copilot · 2026-04-08T14:02:03Z

+		if err != context.Canceled {
+			t.Logf("ServeWithHealth error: %v", err)


This test now logs unexpected ServeWithHealth errors instead of failing. That can mask real regressions where the server exits early or returns a non-cancelation error. Keep the assertion strict (fail on unexpected errors) as before, or at least fail on any non-nil error other than context.Canceled.

Suggested change

if err != context.Canceled {

t.Logf("ServeWithHealth error: %v", err)

if err != nil && !errors.Is(err, context.Canceled) {

t.Fatalf("ServeWithHealth returned unexpected error: %v", err)

kiosvantra

Blocking issue: the server resolves METRONOUS_INGEST_TOKEN / ingest.key once when ingestHandler is created, not per request. The plugin creates ingest.key lazily on first HTTP send, so a fresh daemon starts with expectedToken == "" and will keep accepting unauthenticated requests for the lifetime of that process. Authentication only starts working after a daemon restart, which defeats the security goal of this PR. Please reload the secret at request time or guarantee the key exists before the handler is registered.

The plugin creates ingest.key lazily on first HTTP send. The daemon was caching the token at handler registration time, so a fresh daemon would accept unauthenticated requests until restart. Now reads token at request time (env first, then file) to properly handle the lazy key creation pattern.

kiosvantra

The per-request token reload fixes the lazy ingest.key bootstrap issue I flagged earlier. My blocking concern is resolved. I still agree with the non-blocking follow-ups about returning 413 on MaxBytesError in the HMAC path and keeping the ServeWithHealth test assertions strict, but those do not block this PR for me.

- Extract resolveIngestToken() method: per-request token resolution with validation (64 hex chars, valid hex decode) - Decode HMAC signature to raw bytes before comparison instead of comparing hex strings, verify sig is exactly 32 bytes (sha256.Size) - Return 413 for MaxBytesError in both HMAC and JSON decode paths - Update doc comment to document file-based auth source - Fix tests: check os.WriteFile errors, use filepath.Join, strict ServeWithHealth assertions (fail on non-canceled errors) - Use valid 64-char hex tokens in tests

SacrilegeTx · 2026-04-09T03:57:39Z

Thanks for the thorough review! All feedback addressed in commit 13843a4:

Per-request token resolution — Extracted
esolveIngestToken() method that reads token on each request, handles lazy key creation correctly
Token format validation —
esolveIngestToken() validates 64 hex chars and valid hex decode, rejects malformed keys with a warning log
HMAC signature decoded to raw bytes — hex.DecodeString() first, verify len == sha256.Size, then compare raw MAC bytes with hmac.Equal() (no more hex string comparison)
413 for MaxBytesError — Both HMAC path and JSON decode path detect *http.MaxBytesError and return 413
Doc comment updated — Documents file-based auth source alongside env var
Test quality — Check os.WriteFile errors, use ilepath.Join instead of string concat, strict ServeWithHealth assertions (fail on non-canceled errors), valid 64-char hex tokens

All tests passing, go vet clean.

SacrilegeTx added 12 commits April 1, 2026 09:53

Merge remote-tracking branch 'upstream/main'

1c52b7a

Merge remote-tracking branch 'upstream/main'

a988ecf

chore: add AGENTS.md and CLAUDE.md to gitignore

66a97e0

- These are user-specific development rules - Not all developers share the same environment (Windows/PowerShell vs Linux) - Each developer can have their own AGENTS.md without affecting the repo

Merge remote-tracking branch 'upstream/main'

0fa2db5

Merge remote-tracking branch 'upstream/main'

28f1843

Merge remote-tracking branch 'upstream/main'

24b2145

Merge remote-tracking branch 'upstream/main'

52de841

Copilot AI review requested due to automatic review settings April 8, 2026 13:57

Copilot started reviewing on behalf of SacrilegeTx April 8, 2026 13:57 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

kiosvantra requested changes Apr 8, 2026

View reviewed changes

kiosvantra approved these changes Apr 9, 2026

View reviewed changes

kiosvantra merged commit 1c5f216 into kiosvantra:main Apr 9, 2026
1 check passed

Navigation Menu

feat(auth): add HMAC-SHA256 authenticated ingest channel (Issue #8) - #11

feat(auth): add HMAC-SHA256 authenticated ingest channel (Issue #8)#11
kiosvantra merged 14 commits into
kiosvantra:mainfrom
SacrilegeTx:main

SacrilegeTx commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

kiosvantra left a comment

Uh oh!

kiosvantra left a comment

Uh oh!

SacrilegeTx commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-//     token in the X-Metronous-Auth header (exact match).
-//   - If the token is not set, requests are accepted without authentication (legacy mode).
-//
-// The header value can be either:
-//   - Plain token (backward compatibility): exact match against METRONOUS_INGEST_TOKEN
+//     token in the X-Metronous-Auth header.
+//   - Otherwise, if {dataDir}/ingest.key exists and contains a token, requests MUST
+//     include that matching token in the X-Metronous-Auth header.
+//   - If neither source provides a token, requests are accepted without authentication
+//     (legacy mode).
+//
+// The header value can be either:
+//   - Plain token (backward compatibility): exact match against the configured secret

		if err != context.Canceled {
		t.Logf("ServeWithHealth error: %v", err)

Conversation

SacrilegeTx commented Apr 8, 2026

Summary

Changes

metronous-plugin.ts

internal/mcp/server.go

internal/mcp/server_test.go

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

kiosvantra left a comment

Choose a reason for hiding this comment

Uh oh!

kiosvantra left a comment

Choose a reason for hiding this comment

Uh oh!

SacrilegeTx commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants