mirror of
https://github.com/ahmetb/kubectx.git
synced 2026-05-05 12:41:44 +00:00
Allow non-mutating POST endpoints and dry-run through readonly proxy
The readonly proxy now permits: - K8s "review" POST endpoints (SubjectAccessReview, TokenReview, etc.) that query auth state without persisting resources - Requests with ?dryRun=All for server-side validation Review endpoints are matched with anchored regexps pinned to authorization.k8s.io and authentication.k8s.io API groups, preventing spoofing via custom resources with the same name. Refactors the handler into small, independently tested filter functions (isUpgrade, isReadOnly, isNonMutatingPost, isDryRun) composed by a checkRequest commander. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ahmet Alp Balkan
@@ -10,6 +10,7 @@ import (
|
||||
"net/http/httputil"
|
||||
"net/url"
|
||||
"os"
|
||||
"regexp"
|
||||
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/client-go/rest"
|
||||
@@ -18,6 +19,18 @@ import (
|
||||
"github.com/ahmetb/kubectx/internal/env"
|
||||
)
|
||||
|
||||
// nonMutatingPostPatterns match Kubernetes "review" endpoints that use POST
|
||||
// but don't create persistent resources. Patterns are anchored to known API
|
||||
// groups to prevent spoofing via custom resource names.
|
||||
var nonMutatingPostPatterns = []*regexp.Regexp{
|
||||
regexp.MustCompile(`^/apis/authorization\.k8s\.io/[^/]+/selfsubjectaccessreviews$`),
|
||||
regexp.MustCompile(`^/apis/authorization\.k8s\.io/[^/]+/subjectaccessreviews$`),
|
||||
regexp.MustCompile(`^/apis/authorization\.k8s\.io/[^/]+/namespaces/[^/]+/localsubjectaccessreviews$`),
|
||||
regexp.MustCompile(`^/apis/authorization\.k8s\.io/[^/]+/selfsubjectrulesreviews$`),
|
||||
regexp.MustCompile(`^/apis/authentication\.k8s\.io/[^/]+/tokenreviews$`),
|
||||
regexp.MustCompile(`^/apis/authentication\.k8s\.io/[^/]+/selfsubjectreviews$`),
|
||||
}
|
||||
|
||||
var debugLog = func() *log.Logger {
|
||||
if _, ok := os.LookupEnv(env.EnvDebug); ok {
|
||||
return log.New(os.Stderr, "[readonly-proxy] ", log.Ltime)
|
||||
@@ -103,25 +116,71 @@ func NewHandler(target *url.URL, transport http.RoundTripper) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
debugLog.Printf(">> %s %s", r.Method, r.URL.Path)
|
||||
|
||||
// Block protocol upgrades (exec, cp, port-forward use SPDY/WebSocket).
|
||||
if r.Header.Get("Connection") == "Upgrade" || r.Header.Get("Upgrade") != "" {
|
||||
debugLog.Printf("<< %s %s -> 405 (upgrade blocked)", r.Method, r.URL.Path)
|
||||
writeBlockedResponse(w, r.Method, "[kubectx] readonly mode: operations like exec, cp, and port-forward are not allowed")
|
||||
if reason, ok := checkRequest(r); !ok {
|
||||
debugLog.Printf("<< %s %s -> 405 (%s)", r.Method, r.URL.Path, reason)
|
||||
writeBlockedResponse(w, r.Method,
|
||||
fmt.Sprintf("[kubectx] readonly mode: %s", reason))
|
||||
return
|
||||
}
|
||||
|
||||
switch r.Method {
|
||||
case http.MethodGet, http.MethodHead, http.MethodOptions:
|
||||
debugLog.Printf("<< %s %s -> proxied", r.Method, r.URL.Path)
|
||||
proxy.ServeHTTP(w, r)
|
||||
default:
|
||||
debugLog.Printf("<< %s %s -> 405 (blocked)", r.Method, r.URL.Path)
|
||||
writeBlockedResponse(w, r.Method,
|
||||
fmt.Sprintf("[kubectx] readonly mode: %s requests are not allowed", r.Method))
|
||||
}
|
||||
debugLog.Printf("<< %s %s -> proxied", r.Method, r.URL.Path)
|
||||
proxy.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
// checkRequest determines whether a request should be proxied or blocked.
|
||||
// Returns ("", true) if allowed, or (reason, false) if blocked.
|
||||
func checkRequest(r *http.Request) (reason string, allowed bool) {
|
||||
if isUpgrade(r) {
|
||||
return "operations like exec, cp, and port-forward are not allowed", false
|
||||
}
|
||||
if isReadOnly(r) {
|
||||
return "", true
|
||||
}
|
||||
if isNonMutatingPost(r) {
|
||||
return "", true
|
||||
}
|
||||
if isDryRun(r) {
|
||||
return "", true
|
||||
}
|
||||
return fmt.Sprintf("%s requests are not allowed", r.Method), false
|
||||
}
|
||||
|
||||
// isUpgrade returns true if the request is a protocol upgrade (SPDY/WebSocket).
|
||||
func isUpgrade(r *http.Request) bool {
|
||||
return r.Header.Get("Connection") == "Upgrade" || r.Header.Get("Upgrade") != ""
|
||||
}
|
||||
|
||||
// isReadOnly returns true for safe HTTP methods that never modify state.
|
||||
func isReadOnly(r *http.Request) bool {
|
||||
switch r.Method {
|
||||
case http.MethodGet, http.MethodHead, http.MethodOptions:
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// isNonMutatingPost returns true for Kubernetes "review" endpoints that use
|
||||
// POST but don't create persistent resources (e.g. SubjectAccessReview).
|
||||
// Patterns are anchored to known API groups to prevent spoofing.
|
||||
func isNonMutatingPost(r *http.Request) bool {
|
||||
if r.Method != http.MethodPost {
|
||||
return false
|
||||
}
|
||||
for _, re := range nonMutatingPostPatterns {
|
||||
if re.MatchString(r.URL.Path) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// isDryRun returns true if the request has ?dryRun=All, which means
|
||||
// the API server will validate but not persist the request.
|
||||
func isDryRun(r *http.Request) bool {
|
||||
return r.URL.Query().Get("dryRun") == "All"
|
||||
}
|
||||
|
||||
func writeBlockedResponse(w http.ResponseWriter, method, message string) {
|
||||
status := &metav1.Status{
|
||||
TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Status"},
|
||||
|
||||
@@ -28,6 +28,171 @@ func newTestHandler(t *testing.T) (http.Handler, *httptest.Server) {
|
||||
return handler, backend
|
||||
}
|
||||
|
||||
// --- Unit tests for individual filter functions ---
|
||||
|
||||
func TestIsUpgrade(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
connection string
|
||||
upgrade string
|
||||
want bool
|
||||
}{
|
||||
{"no headers", "", "", false},
|
||||
{"Connection: Upgrade", "Upgrade", "", true},
|
||||
{"Upgrade: SPDY", "", "SPDY/3.1", true},
|
||||
{"Upgrade: websocket", "", "websocket", true},
|
||||
{"both headers", "Upgrade", "websocket", true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
if tt.connection != "" {
|
||||
r.Header.Set("Connection", tt.connection)
|
||||
}
|
||||
if tt.upgrade != "" {
|
||||
r.Header.Set("Upgrade", tt.upgrade)
|
||||
}
|
||||
if got := isUpgrade(r); got != tt.want {
|
||||
t.Errorf("isUpgrade() = %v, want %v", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsReadOnly(t *testing.T) {
|
||||
tests := []struct {
|
||||
method string
|
||||
want bool
|
||||
}{
|
||||
{http.MethodGet, true},
|
||||
{http.MethodHead, true},
|
||||
{http.MethodOptions, true},
|
||||
{http.MethodPost, false},
|
||||
{http.MethodPut, false},
|
||||
{http.MethodPatch, false},
|
||||
{http.MethodDelete, false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.method, func(t *testing.T) {
|
||||
r := httptest.NewRequest(tt.method, "/api/v1/pods", nil)
|
||||
if got := isReadOnly(r); got != tt.want {
|
||||
t.Errorf("isReadOnly(%s) = %v, want %v", tt.method, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsNonMutatingPost(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
want bool
|
||||
}{
|
||||
{"selfsubjectaccessreviews", http.MethodPost,
|
||||
"/apis/authorization.k8s.io/v1/selfsubjectaccessreviews", true},
|
||||
{"subjectaccessreviews", http.MethodPost,
|
||||
"/apis/authorization.k8s.io/v1/subjectaccessreviews", true},
|
||||
{"localsubjectaccessreviews", http.MethodPost,
|
||||
"/apis/authorization.k8s.io/v1/namespaces/default/localsubjectaccessreviews", true},
|
||||
{"selfsubjectrulesreviews", http.MethodPost,
|
||||
"/apis/authorization.k8s.io/v1/selfsubjectrulesreviews", true},
|
||||
{"tokenreviews", http.MethodPost,
|
||||
"/apis/authentication.k8s.io/v1/tokenreviews", true},
|
||||
{"selfsubjectreviews", http.MethodPost,
|
||||
"/apis/authentication.k8s.io/v1/selfsubjectreviews", true},
|
||||
{"regular POST", http.MethodPost,
|
||||
"/api/v1/namespaces", false},
|
||||
{"GET to review path", http.MethodGet,
|
||||
"/apis/authorization.k8s.io/v1/selfsubjectaccessreviews", false},
|
||||
{"DELETE to review path", http.MethodDelete,
|
||||
"/apis/authorization.k8s.io/v1/selfsubjectaccessreviews", false},
|
||||
{"spoofed resource name", http.MethodPost,
|
||||
"/apis/evil.io/v1/selfsubjectaccessreviews", false},
|
||||
{"spoofed suffix in custom group", http.MethodPost,
|
||||
"/apis/custom.example.com/v1/namespaces/default/selfsubjectaccessreviews", false},
|
||||
{"review name as subresource", http.MethodPost,
|
||||
"/api/v1/namespaces/default/pods/selfsubjectaccessreviews", false},
|
||||
{"v1beta1 version allowed", http.MethodPost,
|
||||
"/apis/authorization.k8s.io/v1beta1/selfsubjectaccessreviews", true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := httptest.NewRequest(tt.method, tt.path, nil)
|
||||
if got := isNonMutatingPost(r); got != tt.want {
|
||||
t.Errorf("isNonMutatingPost(%s %s) = %v, want %v", tt.method, tt.path, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsDryRun(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
url string
|
||||
want bool
|
||||
}{
|
||||
{"dryRun=All", "/api/v1/namespaces?dryRun=All", true},
|
||||
{"no dryRun", "/api/v1/namespaces", false},
|
||||
{"dryRun=None", "/api/v1/namespaces?dryRun=None", false},
|
||||
{"dryRun empty", "/api/v1/namespaces?dryRun=", false},
|
||||
{"dryRun with other params", "/api/v1/namespaces?fieldManager=kubectl&dryRun=All", true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPost, tt.url, nil)
|
||||
if got := isDryRun(r); got != tt.want {
|
||||
t.Errorf("isDryRun(%s) = %v, want %v", tt.url, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// --- Unit tests for the commander ---
|
||||
|
||||
func TestCheckRequest(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
headers map[string]string
|
||||
allowed bool
|
||||
}{
|
||||
{"GET allowed", http.MethodGet, "/api/v1/pods", nil, true},
|
||||
{"POST blocked", http.MethodPost, "/api/v1/pods", nil, false},
|
||||
{"upgrade blocked", http.MethodGet, "/api/v1/pods/foo/exec",
|
||||
map[string]string{"Connection": "Upgrade", "Upgrade": "SPDY/3.1"}, false},
|
||||
{"review POST allowed", http.MethodPost,
|
||||
"/apis/authorization.k8s.io/v1/selfsubjectaccessreviews", nil, true},
|
||||
{"dry-run POST allowed", http.MethodPost,
|
||||
"/api/v1/namespaces?dryRun=All", nil, true},
|
||||
{"dry-run DELETE allowed", http.MethodDelete,
|
||||
"/api/v1/namespaces/foo?dryRun=All", nil, true},
|
||||
{"upgrade trumps dry-run", http.MethodGet, "/api/v1/pods?dryRun=All",
|
||||
map[string]string{"Connection": "Upgrade"}, false},
|
||||
{"upgrade trumps review", http.MethodPost,
|
||||
"/apis/authorization.k8s.io/v1/selfsubjectaccessreviews",
|
||||
map[string]string{"Connection": "Upgrade"}, false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := httptest.NewRequest(tt.method, tt.path, nil)
|
||||
for k, v := range tt.headers {
|
||||
r.Header.Set(k, v)
|
||||
}
|
||||
reason, ok := checkRequest(r)
|
||||
if ok != tt.allowed {
|
||||
t.Errorf("checkRequest() allowed=%v, want %v (reason=%q)", ok, tt.allowed, reason)
|
||||
}
|
||||
if !ok && reason == "" {
|
||||
t.Error("checkRequest() returned blocked with empty reason")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// --- Integration tests through the full handler ---
|
||||
|
||||
func TestHandler_AllowedMethods(t *testing.T) {
|
||||
handler, _ := newTestHandler(t)
|
||||
|
||||
@@ -108,6 +273,61 @@ func TestHandler_BlocksUpgrade(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandler_AllowsNonMutatingPOST(t *testing.T) {
|
||||
handler, _ := newTestHandler(t)
|
||||
|
||||
paths := []string{
|
||||
"/apis/authorization.k8s.io/v1/selfsubjectaccessreviews",
|
||||
"/apis/authorization.k8s.io/v1/subjectaccessreviews",
|
||||
"/apis/authorization.k8s.io/v1/namespaces/default/localsubjectaccessreviews",
|
||||
"/apis/authorization.k8s.io/v1/selfsubjectrulesreviews",
|
||||
"/apis/authentication.k8s.io/v1/tokenreviews",
|
||||
"/apis/authentication.k8s.io/v1/selfsubjectreviews",
|
||||
}
|
||||
|
||||
for _, path := range paths {
|
||||
t.Run(path, func(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodPost, path, nil)
|
||||
rr := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rr, req)
|
||||
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for POST %s, got %d", path, rr.Code)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandler_AllowsDryRun(t *testing.T) {
|
||||
handler, _ := newTestHandler(t)
|
||||
|
||||
for _, method := range []string{
|
||||
http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete,
|
||||
} {
|
||||
t.Run(method, func(t *testing.T) {
|
||||
req := httptest.NewRequest(method, "/api/v1/namespaces?dryRun=All", nil)
|
||||
rr := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rr, req)
|
||||
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for %s with dryRun=All, got %d", method, rr.Code)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandler_BlocksDryRunNone(t *testing.T) {
|
||||
handler, _ := newTestHandler(t)
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/namespaces?dryRun=None", nil)
|
||||
rr := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rr, req)
|
||||
|
||||
if rr.Code != http.StatusMethodNotAllowed {
|
||||
t.Errorf("expected 405 for dryRun=None, got %d", rr.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandler_GETResponsePassthrough(t *testing.T) {
|
||||
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
|
||||
Reference in New Issue
Block a user