diff --git a/internal/web/connections_version_control_test.go b/internal/web/connections_version_control_test.go
new file mode 100644
index 0000000..1743fe6
--- /dev/null
+++ b/internal/web/connections_version_control_test.go
@@ -0,0 +1,130 @@
+package web_test
+
+// Regression test for the synthetic "version_control" connector-tab
+// filter that groups github + gitlab cards (and existing github/gitlab
+// connections) under one /connections sub-page. Lives next to the
+// parallel /policies?scope=version_control test so a future refactor
+// can't silently drop either branch of the filter.
+
+import (
+ "net/http"
+ "strings"
+ "testing"
+
+ "github.com/trilitech/Sieve/internal/connector"
+ githubconn "github.com/trilitech/Sieve/internal/connectors/github"
+ gitlabconn "github.com/trilitech/Sieve/internal/connectors/gitlab"
+ "github.com/trilitech/Sieve/internal/connectors/httpproxy"
+)
+
+// TestConnectionsPage_VersionControlTab asserts:
+// - /connections?type=version_control surfaces both the github and
+// gitlab connector catalog cards.
+// - It also surfaces existing connections whose ConnectorType is
+// "github" or "gitlab", and excludes other connections.
+// - The narrower ?type=github tab only shows GitHub (no GitLab).
+func TestConnectionsPage_VersionControlTab(t *testing.T) {
+ handler, env := newTestWebServer(t)
+
+ // Register the real github + gitlab connector metas so the catalog
+ // loop has cards to render. The mock connector seeded by
+ // newTestWebServer is registered under "mock", which is fine —
+ // it's the negative control.
+ env.Registry.Register(githubconn.Meta(), func(raw map[string]any) (connector.Connector, error) {
+ return githubconn.Factory()(raw)
+ })
+ env.Registry.Register(gitlabconn.Meta(), func(raw map[string]any) (connector.Connector, error) {
+ return gitlabconn.Factory()(raw)
+ })
+ // http_proxy is needed for the "forged category" negative control below.
+ env.Registry.Register(httpproxy.Meta, httpproxy.Factory)
+
+ // Seed one connection per connector type. Config payloads are
+ // intentionally empty: the filter under test keys off
+ // ConnectorType + display name, not the connector's typed config
+ // schema, and a too-specific seed would tie the test to a
+ // schema that may evolve.
+ if err := env.Connections.Add("my-github", "github", "My GitHub", map[string]any{}); err != nil {
+ t.Fatalf("add github connection: %v", err)
+ }
+ if err := env.Connections.Add("my-gitlab", "gitlab", "My GitLab", map[string]any{}); err != nil {
+ t.Fatalf("add gitlab connection: %v", err)
+ }
+ if err := env.Connections.Add("my-mock", "mock", "My Mock", map[string]any{}); err != nil {
+ t.Fatalf("add mock connection: %v", err)
+ }
+ // Negative control: an http_proxy whose config.category is set to
+ // "github" must NOT appear under ?type=version_control. The filter
+ // keys off ConnectorType (immutable) rather than the per-connection
+ // category override, so this stays excluded.
+ if err := env.Connections.Add("fake-vcs-proxy", "http_proxy", "Fake VCS Proxy", map[string]any{
+ "target_url": "https://example.com",
+ "category": "github",
+ }); err != nil {
+ t.Fatalf("add fake-vcs proxy: %v", err)
+ }
+
+ t.Run("version_control tab shows both catalog cards + both connections", func(t *testing.T) {
+ rec := getRequest(handler, env, "/connections?type=version_control")
+ if rec.Code != http.StatusOK {
+ t.Fatalf("expected 200, got %d (body: %s)", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+ // Catalog cards: the page renders each connector card by
+ // emitting both the connector's Meta().Name (in an h4) and its
+ // Meta().Description (in a sibling p). Pin both Name strings —
+ // the Name is the load-bearing label and is stable text.
+ // "GitLab" alone is too soft a probe (it also appears in the
+ // connection-row display name), so we also pin a Description
+ // substring per connector so a regression that removed the
+ // catalog cards while leaving the connection list intact would
+ // fail.
+ for _, want := range []string{
+ "My GitHub", // existing connection display name
+ "My GitLab", // existing connection display name
+ "PAT or GitHub App", // GitHub Meta() Description substring
+ "merge requests", // GitLab Meta() Description substring
+ } {
+ if !strings.Contains(body, want) {
+ t.Errorf("version_control tab missing %q", want)
+ }
+ }
+ if strings.Contains(body, "My Mock") {
+ t.Errorf("version_control tab incorrectly includes non-VCS connection 'My Mock'")
+ }
+ // An http_proxy with config.category="github" must NOT slip
+ // into the version_control tab — the filter keys off the
+ // immutable ConnectorType, not the per-connection override.
+ if strings.Contains(body, "Fake VCS Proxy") {
+ t.Errorf("version_control tab incorrectly includes an http_proxy whose config.category is forged to 'github'")
+ }
+ })
+
+ t.Run("github tab excludes gitlab", func(t *testing.T) {
+ rec := getRequest(handler, env, "/connections?type=github")
+ if rec.Code != http.StatusOK {
+ t.Fatalf("expected 200, got %d", rec.Code)
+ }
+ body := rec.Body.String()
+ if !strings.Contains(body, "My GitHub") {
+ t.Errorf("github tab missing 'My GitHub' connection")
+ }
+ if strings.Contains(body, "My GitLab") {
+ t.Errorf("github tab incorrectly includes 'My GitLab' connection")
+ }
+ })
+
+ t.Run("gitlab tab excludes github", func(t *testing.T) {
+ rec := getRequest(handler, env, "/connections?type=gitlab")
+ if rec.Code != http.StatusOK {
+ t.Fatalf("expected 200, got %d", rec.Code)
+ }
+ body := rec.Body.String()
+ if !strings.Contains(body, "My GitLab") {
+ t.Errorf("gitlab tab missing 'My GitLab' connection")
+ }
+ if strings.Contains(body, "My GitHub") {
+ t.Errorf("gitlab tab incorrectly includes 'My GitHub' connection")
+ }
+ })
+}
diff --git a/internal/web/policies_version_control_test.go b/internal/web/policies_version_control_test.go
new file mode 100644
index 0000000..fe49b35
--- /dev/null
+++ b/internal/web/policies_version_control_test.go
@@ -0,0 +1,100 @@
+package web_test
+
+// Regression test for the synthetic "version_control" policy scope that
+// groups github + gitlab policies under one sidebar tab. The filter is
+// implemented in server.go's handlePolicies; this test pins the
+// inclusion rule so a future refactor can't silently drop github/gitlab
+// from the group or accidentally widen it to other scopes.
+
+import (
+ "net/http"
+ "strings"
+ "testing"
+
+ "github.com/trilitech/Sieve/internal/testing/testenv"
+)
+
+// TestPoliciesPage_VersionControlScope asserts:
+// - ?scope=version_control includes policies tagged github and gitlab.
+// - That synthetic scope does NOT include policies for unrelated
+// connectors (slack, gmail).
+// - The narrower ?scope=github tab shows only github (legacy/empty
+// scopes also show, but explicitly-scoped gitlab policies do not).
+func TestPoliciesPage_VersionControlScope(t *testing.T) {
+ handler, env := newTestWebServer(t)
+
+ // Seed four policies, three of which carry an explicit scope.
+ mustSeedPolicy(t, env, "github-allow", "github")
+ mustSeedPolicy(t, env, "gitlab-allow", "gitlab")
+ mustSeedPolicy(t, env, "slack-allow", "slack")
+ mustSeedPolicy(t, env, "legacy-no-scope", "") // empty scope — shows under all tabs
+ // Hand-crafted "version_control"-scoped policy stands in for a
+ // DB row that slipped past the create-time block in
+ // handlePolicyCreate. The version_control filter MUST NOT
+ // surface it (scope is a browse-only filter, not a real scope).
+ mustSeedPolicy(t, env, "vcs-stamped", "version_control")
+
+ t.Run("version_control includes github + gitlab, excludes slack", func(t *testing.T) {
+ rec := getRequest(handler, env, "/policies?scope=version_control")
+ if rec.Code != http.StatusOK {
+ t.Fatalf("expected 200, got %d (body: %s)", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+ for _, want := range []string{"github-allow", "gitlab-allow", "legacy-no-scope"} {
+ if !strings.Contains(body, want) {
+ t.Errorf("version_control scope missing policy %q", want)
+ }
+ }
+ if strings.Contains(body, "slack-allow") {
+ t.Errorf("version_control scope incorrectly includes slack policy")
+ }
+ if strings.Contains(body, "vcs-stamped") {
+ t.Errorf("version_control scope incorrectly includes a policy literally stamped scope=version_control (this scope is browse-only and such a policy is nonsense)")
+ }
+ })
+
+ t.Run("github scope excludes gitlab", func(t *testing.T) {
+ rec := getRequest(handler, env, "/policies?scope=github")
+ if rec.Code != http.StatusOK {
+ t.Fatalf("expected 200, got %d", rec.Code)
+ }
+ body := rec.Body.String()
+ if !strings.Contains(body, "github-allow") {
+ t.Errorf("github scope missing github-allow policy")
+ }
+ if strings.Contains(body, "gitlab-allow") {
+ t.Errorf("github scope incorrectly includes gitlab-allow")
+ }
+ })
+
+ t.Run("slack scope excludes both vcs policies", func(t *testing.T) {
+ rec := getRequest(handler, env, "/policies?scope=slack")
+ if rec.Code != http.StatusOK {
+ t.Fatalf("expected 200, got %d", rec.Code)
+ }
+ body := rec.Body.String()
+ for _, unwanted := range []string{"github-allow", "gitlab-allow"} {
+ if strings.Contains(body, unwanted) {
+ t.Errorf("slack scope incorrectly includes %q", unwanted)
+ }
+ }
+ })
+}
+
+// mustSeedPolicy creates a minimal rules-evaluator policy carrying the
+// given scope (empty string = unscoped). The rules body is intentionally
+// trivial — we're testing the scope filter, not the evaluator.
+func mustSeedPolicy(t *testing.T, env *testenv.Env, name, scope string) {
+ t.Helper()
+ cfg := map[string]any{
+ "rules": []any{
+ map[string]any{"action": "allow"},
+ },
+ }
+ if scope != "" {
+ cfg["scope"] = scope
+ }
+ if _, err := env.Policies.Create(name, "rules", cfg); err != nil {
+ t.Fatalf("seed policy %q: %v", name, err)
+ }
+}
diff --git a/internal/web/server.go b/internal/web/server.go
index a7c91d0..dc0bb36 100644
--- a/internal/web/server.go
+++ b/internal/web/server.go
@@ -554,6 +554,17 @@ func (s *Server) handleConnections(w http.ResponseWriter, r *http.Request) {
if cat == "http_proxy" || c.ConnectorType == "mcp_proxy" {
conns = append(conns, c)
}
+ } else if connType == "version_control" {
+ // "version_control" tab groups github + gitlab.
+ // Match the immutable ConnectorType directly so an
+ // http_proxy that happens to carry config.category =
+ // "github" / "gitlab" (a legitimate generic-LLM /
+ // generic-proxy override) does NOT slip into the VCS
+ // filter. `cat` reflects the http_proxy override and
+ // would do that here; ConnectorType cannot be forged.
+ if c.ConnectorType == "github" || c.ConnectorType == "gitlab" {
+ conns = append(conns, c)
+ }
} else if cat == connType {
conns = append(conns, c)
}
@@ -573,6 +584,10 @@ func (s *Server) handleConnections(w http.ResponseWriter, r *http.Request) {
if connType == "proxy" {
match = m.Type == "http_proxy" || m.Type == "mcp_proxy"
}
+ // "version_control" tab shows both github and gitlab connectors
+ if connType == "version_control" {
+ match = m.Type == "github" || m.Type == "gitlab"
+ }
if match {
filtered[category] = append(filtered[category], m)
}
@@ -1434,10 +1449,22 @@ func (s *Server) handlePolicies(w http.ResponseWriter, r *http.Request) {
// Filter policies by scope. A policy's scope is stored in its config.
// Policies without a scope (legacy/presets) show under all tabs.
+ // The synthetic "version_control" scope is strictly defined as
+ // github + gitlab + unscoped. A policy literally stamped with
+ // scope=version_control would be nonsense (creation is blocked in
+ // handlePolicyCreate); excluding it here keeps the filter honest
+ // against any hand-crafted DB row that slipped through, and keeps
+ // the handlePolicyCreate comment accurate.
var pols []policies.Policy
for _, p := range allPols {
pScope, _ := p.PolicyConfig["scope"].(string)
- if scope == "" || pScope == "" || pScope == scope {
+ var match bool
+ if scope == "version_control" {
+ match = pScope == "" || pScope == "github" || pScope == "gitlab"
+ } else {
+ match = scope == "" || pScope == "" || pScope == scope
+ }
+ if match {
pols = append(pols, p)
}
}
@@ -1475,6 +1502,22 @@ func (s *Server) handlePolicyCreate(w http.ResponseWriter, r *http.Request) {
policyConfig = make(map[string]any)
}
+ // "version_control" is a synthetic browse-only scope used in the
+ // sidebar grouping; it isn't a real connector scope. The
+ // /policies?scope=version_control list-filter (handlePolicies)
+ // pulls policies whose stored scope is github or gitlab (plus
+ // unscoped legacies). Persisting scope=version_control on a new
+ // policy would orphan it from both the github and gitlab tabs
+ // while only matching the synthetic group — almost certainly an
+ // accident, so refuse it loudly. The policies.html template
+ // hides the create form under this scope; this server-side
+ // reject is defence-in-depth against a hand-crafted POST or an
+ // out-of-date browser tab.
+ if sc, _ := policyConfig["scope"].(string); sc == "version_control" {
+ http.Error(w, "scope \"version_control\" is a browse-only filter; pick github or gitlab for a real policy", http.StatusBadRequest)
+ return
+ }
+
// Validate the policy rules against known operations.
if errs := s.validatePolicyRules(policyConfig); len(errs) > 0 {
http.Error(w, "Policy validation errors:\n"+strings.Join(errs, "\n"), http.StatusBadRequest)
diff --git a/internal/web/server_status_test.go b/internal/web/server_status_test.go
index 5aecabb..80fc779 100644
--- a/internal/web/server_status_test.go
+++ b/internal/web/server_status_test.go
@@ -48,6 +48,23 @@ func authedPost(t *testing.T, env *testenv.Env, path string) (*http.Request, *ht
return req, httptest.NewRecorder()
}
+// getRequest performs an authenticated GET against an admin handler and
+// returns the recorder. Shared by the web_test package's GET-and-assert
+// tests so a single canonical "GET with session cookie" path lives here
+// instead of being re-implemented in each test file. (internal/web/slack_test.go
+// has a same-named helper that lives in the internal `package web` for
+// the internal-package tests; this one is its external-package
+// counterpart.)
+func getRequest(handler http.Handler, env *testenv.Env, path string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodGet, path, nil)
+ if c := env.SessionCookie(); c != nil {
+ req.AddCookie(c)
+ }
+ rec := httptest.NewRecorder()
+ handler.ServeHTTP(rec, req)
+ return rec
+}
+
// TestServer_DisableConnection_RejectsAgentToken verifies that an
// agent bearer token cannot disable a connection through the admin UI.
// The requireOperatorSession middleware inspects the Authorization
diff --git a/internal/web/templates/connections.html b/internal/web/templates/connections.html
index ff0d56b..b7863ba 100644
--- a/internal/web/templates/connections.html
+++ b/internal/web/templates/connections.html
@@ -201,34 +201,22 @@
{{.Name}}
{{else if eq .Type "github"}}
-
+
Two ways to connect. Pick what matches your access pattern.
Acts as the App (not a real user), better for audit.
-
Requires your Sieve host to be reachable from GitHub.
-
+
Paste a token; done. Scoped to specific repos under one owner. Acts as you (or a service-account user). Tokens expire and need re-pasting.
+
+
+
+ GitHub App install
+
Org-wide install with finer-grained per-repo scopes. Ephemeral 1-hour tokens — Sieve refreshes automatically. Acts as the App (better audit). Requires your Sieve host to be reachable from GitHub for the callback.
-
Use a PAT for a single repo or personal/dev access; install the App when multiple repos in an org need long-lived, auto-rotating access.
+
Use a PAT for a single repo or personal/dev access; install the App when multiple repos in an org need long-lived, auto-rotating access.
@@ -164,13 +201,23 @@
function toggleSub(id) {
var el = document.getElementById(id);
var chevron = document.getElementById(id + '-chevron');
- if (el.classList.contains('hidden')) {
+ var nowOpen = el.classList.contains('hidden');
+ if (nowOpen) {
el.classList.remove('hidden');
if (chevron) chevron.style.transform = 'rotate(180deg)';
} else {
el.classList.add('hidden');
if (chevron) chevron.style.transform = '';
}
+ // Keep the disclosure button's aria-expanded in sync with the
+ // submenu's visibility so screen-reader announcements match what's
+ // on screen. Only buttons that declared aria-controls={id} on the
+ // surrounding markup are updated; older toggles that haven't been
+ // migrated remain a no-op here.
+ var btns = document.querySelectorAll('button[aria-controls="' + id + '"]');
+ for (var i = 0; i < btns.length; i++) {
+ btns[i].setAttribute('aria-expanded', nowOpen ? 'true' : 'false');
+ }
}
// Auto-expand sections that have active sub-items.
@@ -178,6 +225,9 @@
var active = "{{.Active}}";
if (active.startsWith("connections-")) {
toggleSub('conn-sub');
+ if (active === "connections-github" || active === "connections-gitlab" || active === "connections-version_control") {
+ toggleSub('conn-vcs-sub');
+ }
}
if (active.startsWith("policies-")) {
toggleSub('policy-sub');
@@ -190,7 +240,274 @@
toggleSub('aws-sub');
}
}
+ if (active === "policies-github" || active === "policies-gitlab" || active === "policies-version_control") {
+ toggleSub('policy-vcs-sub');
+ }
+ }
+})();
+
+
+
+
“Version Control” groups existing GitHub and GitLab policies under one tab for review. New policies are authored per-connector because each has its own operation namespace.
“Version Control” is a browse filter that lists GitHub and GitLab policies together. Each connector exposes a different operation namespace, so rule authoring happens per-connector: