mirror of
https://github.com/hanzoai/kms.git
synced 2026-10-17 11:18:16 +00:00
the secret URL carries no org, because the server does not read one there
The HTTP path addressed /v1/kms/orgs/{org}/secrets. The server mounts
/v1/kms/secrets and takes the org from the credential the request is made with,
so every HTTP-transport call reached nothing — measured against the live edge,
where the mounted path answers a JSON 403 and the org path a bare 404.
Reading the tenant from the credential is also the only place it can be trusted
from: a segment is a name the caller picks, and one identity naming another
org's secrets should not be a request this client can compose.
The four tests that covered these calls asserted the org path against a mock
server, which answers whatever path it is sent — so they were green the whole
time the transport was broken. They now pin what the server mounts, and a test
beside them fails if an org ever steers the URL again.
Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
+14
-16
@@ -289,9 +289,14 @@ func (c *Client) Close() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// secretPath joins org/path/name into the canonical HTTP URL:
|
||||
// secretPath joins path/name into the canonical HTTP URL:
|
||||
//
|
||||
// {endpoint}/v1/kms/orgs/{org}/secrets/{path}/{name}
|
||||
// {endpoint}/v1/kms/secrets/{path}/{name}
|
||||
//
|
||||
// The org is NOT a segment. The server reads it from the credential the request
|
||||
// is made with, which is the only place it can be trusted from: a segment is a
|
||||
// name the caller picks, and one identity naming another org's secrets is the
|
||||
// request this client should not be able to compose.
|
||||
func (c *Client) secretPath(path, name string) string {
|
||||
p := strings.Trim(path, "/")
|
||||
n := strings.Trim(name, "/")
|
||||
@@ -303,9 +308,8 @@ func (c *Client) secretPath(path, name string) string {
|
||||
for i, s := range segs {
|
||||
segs[i] = url.PathEscape(s)
|
||||
}
|
||||
return fmt.Sprintf("%s/v1/kms/orgs/%s/secrets/%s",
|
||||
return fmt.Sprintf("%s/v1/kms/secrets/%s",
|
||||
c.endpoint,
|
||||
url.PathEscape(c.org),
|
||||
strings.Join(segs, "/"),
|
||||
)
|
||||
}
|
||||
@@ -320,7 +324,7 @@ func (c *Client) secretURL(path, name string) string {
|
||||
|
||||
// Get fetches a single secret value by path and name.
|
||||
//
|
||||
// On the HTTP path: GET /v1/kms/orgs/{org}/secrets/{path}/{name}.
|
||||
// On the HTTP path: GET /v1/kms/secrets/{path}/{name}.
|
||||
// On the ZAP path: OpSecretGet (0x0040).
|
||||
func (c *Client) Get(ctx context.Context, path, name string) (string, error) {
|
||||
if c.transport == "zap" {
|
||||
@@ -383,7 +387,7 @@ func (c *Client) GetJSON(ctx context.Context, path, name string, dst any) error
|
||||
// List returns "path/name" strings for all secrets visible to the
|
||||
// caller under the given prefix.
|
||||
//
|
||||
// On the HTTP path: GET /v1/kms/orgs/{org}/secrets?prefix=…
|
||||
// On the HTTP path: GET /v1/kms/secrets?prefix=…
|
||||
// On the ZAP path: OpSecretList (0x0042) — names only, no path
|
||||
// information is surfaced; we prepend the requested prefix so callers
|
||||
// see identical output across transports.
|
||||
@@ -411,10 +415,7 @@ func (c *Client) httpList(ctx context.Context, pathPrefix string) ([]string, err
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("kmsclient: auth: %w", err)
|
||||
}
|
||||
u := fmt.Sprintf("%s/v1/kms/orgs/%s/secrets",
|
||||
c.endpoint,
|
||||
url.PathEscape(c.org),
|
||||
)
|
||||
u := c.endpoint + "/v1/kms/secrets"
|
||||
if pathPrefix != "" {
|
||||
u += "?prefix=" + url.QueryEscape(pathPrefix)
|
||||
}
|
||||
@@ -457,7 +458,7 @@ func (c *Client) httpList(ctx context.Context, pathPrefix string) ([]string, err
|
||||
|
||||
// Put creates or updates a secret.
|
||||
//
|
||||
// On the HTTP path: POST /v1/kms/orgs/{org}/secrets (upsert).
|
||||
// On the HTTP path: POST /v1/kms/secrets (upsert).
|
||||
// On the ZAP path: OpSecretPut (0x0041).
|
||||
//
|
||||
// Requires admin role on the respective auth path.
|
||||
@@ -473,10 +474,7 @@ func (c *Client) httpPut(ctx context.Context, path, name, value string) error {
|
||||
if err != nil {
|
||||
return fmt.Errorf("kmsclient: auth: %w", err)
|
||||
}
|
||||
u := fmt.Sprintf("%s/v1/kms/orgs/%s/secrets",
|
||||
c.endpoint,
|
||||
url.PathEscape(c.org),
|
||||
)
|
||||
u := c.endpoint + "/v1/kms/secrets"
|
||||
// env is REQUIRED by the server on writes (no silent "default"): send
|
||||
// c.env explicitly so the write lands in the same env-keyed record that
|
||||
// project/env/path readers resolve. Historically this body omitted env
|
||||
@@ -509,7 +507,7 @@ func (c *Client) httpPut(ctx context.Context, path, name, value string) error {
|
||||
|
||||
// Delete removes a secret.
|
||||
//
|
||||
// On the HTTP path: DELETE /v1/kms/orgs/{org}/secrets/{path}/{name}.
|
||||
// On the HTTP path: DELETE /v1/kms/secrets/{path}/{name}.
|
||||
// On the ZAP path: OpSecretDelete (0x0043).
|
||||
func (c *Client) Delete(ctx context.Context, path, name string) error {
|
||||
if c.transport == "zap" {
|
||||
|
||||
@@ -85,8 +85,9 @@ func TestGet_WithMockServer(t *testing.T) {
|
||||
iam := mockIAM(t, "test-token-123")
|
||||
defer iam.Close()
|
||||
|
||||
// Server: implements canonical GET /v1/kms/orgs/{org}/secrets/{path}/{name}.
|
||||
const wantPath = "/v1/kms/orgs/hanzo/secrets/providers/alpaca/dev/api_key"
|
||||
// Server: implements canonical GET /v1/kms/secrets/{path}/{name}. The org is
|
||||
// not a segment — the server reads it from the credential.
|
||||
const wantPath = "/v1/kms/secrets/providers/alpaca/dev/api_key"
|
||||
kms := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Header.Get("Authorization") != "Bearer test-token-123" {
|
||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
||||
@@ -217,8 +218,8 @@ func TestPut_SendsCanonicalPayload(t *testing.T) {
|
||||
if err := c.Put(context.Background(), "providers/square/dev", "access_token", "sq_abc"); err != nil {
|
||||
t.Fatalf("Put: %v", err)
|
||||
}
|
||||
if gotPath != "/v1/kms/orgs/hanzo/secrets" {
|
||||
t.Errorf("path = %q, want /v1/kms/orgs/hanzo/secrets", gotPath)
|
||||
if gotPath != "/v1/kms/secrets" {
|
||||
t.Errorf("path = %q, want /v1/kms/secrets", gotPath)
|
||||
}
|
||||
if gotBody["path"] != "providers/square/dev" || gotBody["name"] != "access_token" || gotBody["value"] != "sq_abc" {
|
||||
t.Errorf("body = %+v, want path=providers/square/dev name=access_token value=sq_abc", gotBody)
|
||||
@@ -310,8 +311,8 @@ func TestList_UsesCanonicalPath(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("List: %v", err)
|
||||
}
|
||||
if gotPath != "/v1/kms/orgs/hanzo/secrets" {
|
||||
t.Errorf("path = %q, want /v1/kms/orgs/hanzo/secrets", gotPath)
|
||||
if gotPath != "/v1/kms/secrets" {
|
||||
t.Errorf("path = %q, want /v1/kms/secrets", gotPath)
|
||||
}
|
||||
if gotQuery != "prefix=providers%2Fsquare" {
|
||||
t.Errorf("query = %q, want prefix=providers%%2Fsquare", gotQuery)
|
||||
@@ -339,8 +340,8 @@ func TestDelete_UsesCanonicalPath(t *testing.T) {
|
||||
if gotMethod != http.MethodDelete {
|
||||
t.Errorf("method = %q, want DELETE", gotMethod)
|
||||
}
|
||||
if gotPath != "/v1/kms/orgs/hanzo/secrets/providers/square/dev/access_token" {
|
||||
t.Errorf("path = %q, want canonical orgs path", gotPath)
|
||||
if gotPath != "/v1/kms/secrets/providers/square/dev/access_token" {
|
||||
t.Errorf("path = %q, want the canonical secrets path", gotPath)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -348,7 +349,7 @@ func TestSecretPath_EscapesSegments(t *testing.T) {
|
||||
c := &Client{endpoint: "http://kms:8443", org: "liq/uid"}
|
||||
got := c.secretPath("foo bar/baz", "k+q")
|
||||
// Segments individually escaped; "/" preserved as separator. Org escaped once.
|
||||
want := "http://kms:8443/v1/kms/orgs/liq%2Fuid/secrets/foo%20bar/baz/k+q"
|
||||
want := "http://kms:8443/v1/kms/secrets/foo%20bar/baz/k+q"
|
||||
if got != want {
|
||||
t.Errorf("secretPath = %q, want %q", got, want)
|
||||
}
|
||||
@@ -364,8 +365,8 @@ func TestFetchEnv(t *testing.T) {
|
||||
}
|
||||
|
||||
kms := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// Path: /v1/kms/orgs/org/secrets/<path>/<name>
|
||||
const prefix = "/v1/kms/orgs/org/secrets/"
|
||||
// Path: /v1/kms/secrets/<path>/<name>
|
||||
const prefix = "/v1/kms/secrets/"
|
||||
if !strings.HasPrefix(r.URL.Path, prefix) {
|
||||
http.Error(w, "nf", http.StatusNotFound)
|
||||
return
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
// Copyright 2026 Hanzo AI, Inc.
|
||||
// SPDX-License-Identifier: MIT OR Apache-2.0
|
||||
|
||||
package kmsclient
|
||||
|
||||
import "testing"
|
||||
|
||||
// The server mounts /v1/kms/secrets and reads the org from the credential. A URL
|
||||
// carrying an org segment reaches nothing — and would be asking the server to
|
||||
// take the caller's word for the tenant, which is the one thing it must not do.
|
||||
func TestSecretURLCarriesNoOrgSegment(t *testing.T) {
|
||||
c := &Client{endpoint: "https://api.hanzo.ai", org: "hanzo"}
|
||||
|
||||
got := c.secretPath("", "OPENROUTER_API_KEY")
|
||||
if want := "https://api.hanzo.ai/v1/kms/secrets/OPENROUTER_API_KEY"; got != want {
|
||||
t.Fatalf("secretPath = %q, want %q", got, want)
|
||||
}
|
||||
if nested := c.secretPath("ci", "DB"); nested != "https://api.hanzo.ai/v1/kms/secrets/ci/DB" {
|
||||
t.Fatalf("a nested path should ride under secrets, got %q", nested)
|
||||
}
|
||||
}
|
||||
|
||||
// The org travels in the credential, so naming a different one changes nothing
|
||||
// about where the request goes. If this ever fails, the client has started
|
||||
// letting a caller address another tenant by writing its name down.
|
||||
func TestTheOrgDoesNotSteerTheURL(t *testing.T) {
|
||||
mine := (&Client{endpoint: "https://api.hanzo.ai", org: "hanzo"}).secretPath("", "K")
|
||||
theirs := (&Client{endpoint: "https://api.hanzo.ai", org: "someone-else"}).secretPath("", "K")
|
||||
if mine != theirs {
|
||||
t.Fatalf("the org steered the URL: %q vs %q", mine, theirs)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user