From be1ff4839b0189960b28f9eb3f6b8e397837d770 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torjus=20H=C3=A5kestad?= <torjus@usit.uio.no>
Date: Tue, 3 Feb 2026 19:10:31 +0100
Subject: [PATCH] security: validate revision parameter to prevent Nix
 injection

The revision parameter was interpolated directly into a Nix expression,
allowing potential injection of arbitrary Nix code. An attacker could
craft a revision string like:
  "; builtins.readFile /etc/passwd; "

This adds ValidateRevision() which ensures revisions only contain safe
characters (alphanumeric, hyphens, underscores, dots) and are at most
64 characters long.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 internal/nixos/indexer.go      | 25 +++++++++++++++++
 internal/nixos/indexer_test.go | 51 ++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/internal/nixos/indexer.go b/internal/nixos/indexer.go
index 460443e..f1b6cf8 100644
--- a/internal/nixos/indexer.go
+++ b/internal/nixos/indexer.go
@@ -11,12 +11,18 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 
 	"git.t-juice.club/torjus/labmcp/internal/database"
 )
 
+// revisionPattern validates revision strings to prevent injection attacks.
+// Allows: alphanumeric, hyphens, underscores, dots (for channel names like "nixos-24.11"
+// and git hashes). Must be 1-64 characters.
+var revisionPattern = regexp.MustCompile(`^[a-zA-Z0-9._-]{1,64}$`)
+
 // Indexer handles indexing of nixpkgs revisions.
 type Indexer struct {
 	store      database.Store
@@ -42,10 +48,24 @@ type IndexResult struct {
 	AlreadyIndexed bool // True if revision was already indexed (skipped)
 }
 
+// ValidateRevision checks if a revision string is safe to use.
+// Returns an error if the revision contains potentially dangerous characters.
+func ValidateRevision(revision string) error {
+	if !revisionPattern.MatchString(revision) {
+		return fmt.Errorf("invalid revision format: must be 1-64 alphanumeric characters, hyphens, underscores, or dots")
+	}
+	return nil
+}
+
 // IndexRevision indexes a nixpkgs revision by git hash or channel name.
 func (idx *Indexer) IndexRevision(ctx context.Context, revision string) (*IndexResult, error) {
 	start := time.Now()
 
+	// Validate revision to prevent injection attacks
+	if err := ValidateRevision(revision); err != nil {
+		return nil, err
+	}
+
 	// Resolve channel names to git refs
 	ref := resolveRevision(revision)
 
@@ -116,6 +136,11 @@ func (idx *Indexer) IndexRevision(ctx context.Context, revision string) (*IndexR
 
 // ReindexRevision forces re-indexing of a revision, deleting existing data first.
 func (idx *Indexer) ReindexRevision(ctx context.Context, revision string) (*IndexResult, error) {
+	// Validate revision to prevent injection attacks
+	if err := ValidateRevision(revision); err != nil {
+		return nil, err
+	}
+
 	ref := resolveRevision(revision)
 
 	// Delete existing revision if present
diff --git a/internal/nixos/indexer_test.go b/internal/nixos/indexer_test.go
index 44b3a88..7b5c4b5 100644
--- a/internal/nixos/indexer_test.go
+++ b/internal/nixos/indexer_test.go
@@ -12,6 +12,57 @@ import (
 // TestNixpkgsRevision is the revision from flake.lock used for testing.
 const TestNixpkgsRevision = "e6eae2ee2110f3d31110d5c222cd395303343b08"
 
+// TestValidateRevision tests the revision validation function.
+func TestValidateRevision(t *testing.T) {
+	tests := []struct {
+		name     string
+		revision string
+		wantErr  bool
+	}{
+		// Valid cases
+		{"valid git hash", "e6eae2ee2110f3d31110d5c222cd395303343b08", false},
+		{"valid short hash", "e6eae2e", false},
+		{"valid channel name", "nixos-unstable", false},
+		{"valid channel with version", "nixos-24.11", false},
+		{"valid underscore", "some_branch", false},
+		{"valid mixed", "release-24.05_beta", false},
+
+		// Invalid cases - injection attempts
+		{"injection semicolon", "foo; rm -rf /", true},
+		{"injection quotes", `"; builtins.readFile /etc/passwd; "`, true},
+		{"injection backticks", "foo`whoami`", true},
+		{"injection dollar", "foo$(whoami)", true},
+		{"injection newline", "foo\nbar", true},
+		{"injection space", "foo bar", true},
+		{"injection slash", "foo/bar", true},
+		{"injection backslash", "foo\\bar", true},
+		{"injection pipe", "foo|bar", true},
+		{"injection ampersand", "foo&bar", true},
+		{"injection redirect", "foo>bar", true},
+		{"injection less than", "foo<bar", true},
+		{"injection curly braces", "foo{bar}", true},
+		{"injection parens", "foo(bar)", true},
+		{"injection brackets", "foo[bar]", true},
+
+		// Edge cases
+		{"empty string", "", true},
+		{"too long", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", true},
+		{"just dots", "...", false}, // dots are allowed, path traversal is handled elsewhere
+		{"single char", "a", false},
+		{"max length 64", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", false},
+		{"65 chars", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateRevision(tt.revision)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ValidateRevision(%q) error = %v, wantErr %v", tt.revision, err, tt.wantErr)
+			}
+		})
+	}
+}
+
 // BenchmarkIndexRevision benchmarks indexing a full nixpkgs revision.
 // This is a slow benchmark that requires nix to be installed.
 // Run with: go test -bench=BenchmarkIndexRevision -benchtime=1x -timeout=30m ./internal/nixos/...