From df1232e09a38fb474239afd91434e6d01d68e400 Mon Sep 17 00:00:00 2001
From: Rian Stockbower <rstockbower@gmail.com>
Date: Tue, 19 May 2026 09:41:49 -0400
Subject: [PATCH] feat(cache): allow underscore in instance keys and resource
 names
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Second §5 first-port generalization (MON-5369). cli-common/cache's
component guard rejected "_", but the first consumer (jtk) imposed NO
charset limit on resource names — a resource like "also_ok" was valid
there and must stay valid through the facade. Underscore is
filesystem-safe and not a separator, so widening the allowed set is
purely permissive: no previously-valid input becomes invalid, and the
traversal / path-separator / trailing-dot guards are unchanged
(regression-tested). Benefits every future consumer whose resource or
instance identifiers use "_".

[MON-5369]
---
 cache/cache_test.go | 28 ++++++++++++++++++++++++++++
 cache/locator.go    | 12 ++++++++----
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/cache/cache_test.go b/cache/cache_test.go
index bf99877..96bb5ca 100644
--- a/cache/cache_test.go
+++ b/cache/cache_test.go
@@ -329,3 +329,31 @@ func TestWriteResource_StillStampsFetchedAt(t *testing.T) {
 		t.Fatalf("WriteResource must stamp a non-zero FetchedAt")
 	}
 }
+
+func TestUnderscoreComponentsAreValid(t *testing.T) {
+	// Resource/instance names legitimately use underscores; this must
+	// round-trip (the first consumer, jtk, imposed no charset limit on
+	// resource names). Purely permissive widening.
+	loc := cache.Locator{Root: t.TempDir(), InstanceKey: "host_1"}
+	if err := cache.WriteResource(loc, "also_ok", "1h", payload{Name: "u", N: 9}); err != nil {
+		t.Fatalf("WriteResource with underscores: %v", err)
+	}
+	got, err := cache.ReadResource[payload](loc, "also_ok")
+	if err != nil {
+		t.Fatalf("ReadResource with underscores: %v", err)
+	}
+	if got.Data.N != 9 || got.Instance != "host_1" || got.Resource != "also_ok" {
+		t.Fatalf("underscore round-trip wrong: %+v", got)
+	}
+}
+
+func TestUnderscoreWideningStillRejectsUnsafe(t *testing.T) {
+	// Regression: widening to allow "_" must not weaken the traversal /
+	// separator / trailing-dot guards.
+	root := t.TempDir()
+	for _, bad := range []string{"a/b", `a\b`, "..", "a..b", "../x", "trailing.", "_leading"} {
+		if err := cache.WriteResource(cache.Locator{Root: root, InstanceKey: "ok"}, bad, "1h", payload{}); !errors.Is(err, cache.ErrInvalidName) {
+			t.Fatalf("name %q: err = %v, want ErrInvalidName", bad, err)
+		}
+	}
+}
diff --git a/cache/locator.go b/cache/locator.go
index 3fcb723..e0a4db3 100644
--- a/cache/locator.go
+++ b/cache/locator.go
@@ -20,10 +20,14 @@ var (
 
 // safeComponent bounds instance keys and resource names to the subset that is
 // safe to compose into a filesystem path: letters, digits, dot, hyphen,
-// starting alphanumeric. Path separators, whitespace, and control characters
-// are rejected rather than trusted (the values are caller-supplied — e.g. a
-// hostname derived from config).
-var safeComponent = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9.\-]*$`)
+// underscore, starting alphanumeric. Path separators, whitespace, and control
+// characters are rejected rather than trusted (the values are caller-supplied
+// — e.g. a hostname derived from config, or a resource name). Underscore is
+// included because resource names legitimately use it (the first consumer,
+// jtk, imposed no charset limit on resource names); it is filesystem-safe and
+// not a separator, so widening to allow it is purely permissive (no
+// previously-valid input becomes invalid).
+var safeComponent = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._\-]*$`)
 
 // validComponent also rejects a ".." substring (traversal) and a trailing dot.
 // A trailing dot matters cross-OS: Windows (NTFS/FAT) silently strips it, so