From 48b466ed1f5281289e011946f7fefaeea88e0025 Mon Sep 17 00:00:00 2001 From: "Customer.io Open Source Bot" Date: Thu, 3 Sep 2026 16:43:58 +0530 Subject: [PATCH] Add Customer.io CLI source CioCliPublicExport-RevId: b54d02deffe56b9d88e71489b66ea7ee5f6e8f3a --- cmd/api.go | 97 +++++++++ cmd/api_preflight_test.go | 343 ++++++++++++++++++++++++++++++ cmd/auth_test.go | 4 + cmd/helpers.go | 49 +++++ cmd/prime_context.md | 12 ++ cmd/schema.go | 30 +-- internal/routes/pathindex.go | 315 +++++++++++++++++++++++++++ internal/routes/pathindex_test.go | 263 +++++++++++++++++++++++ 8 files changed, 1087 insertions(+), 26 deletions(-) create mode 100644 cmd/api_preflight_test.go create mode 100644 internal/routes/pathindex.go create mode 100644 internal/routes/pathindex_test.go diff --git a/cmd/api.go b/cmd/api.go index eefc188..62ab533 100644 --- a/cmd/api.go +++ b/cmd/api.go @@ -48,6 +48,7 @@ Examples: func init() { apiCmd.Flags().StringP("method", "X", "", "HTTP method (default: GET, or POST if --json or --file is provided)") apiCmd.Flags().StringArray("file", nil, "Send the request as multipart/form-data with a file part: --file @path, or --file field=@path to name the part (repeatable). --json then supplies the request's other form fields") + apiCmd.Flags().Bool("no-preflight", false, "Send the request even if the path is absent from the API spec") rootCmd.AddCommand(apiCmd) } @@ -123,6 +124,10 @@ func runAPI(cmd *cobra.Command, args []string) error { return err } + if err := preflightPath(cmd, c, httpMethod, resolvedPath); err != nil { + return err + } + // Dry run. if GetDryRun(cmd) { apiURL, _ := cmd.Flags().GetString("api-url") @@ -210,6 +215,98 @@ func requestBody(jsonBody json.RawMessage, fileParts []client.FilePart) (*client return client.NewMultipartBody(fileParts, fields) } +// preflightPath rejects a path the API spec does not describe, before the +// request is sent. +// +// An unknown path on the API host does not answer 404: the web app's catch-all +// serves it a 200 and an HTML page. A mistyped or invented path therefore comes +// back looking like an ambiguous empty result rather than a mistake, which +// invites trying more variants of it. Failing here instead names the problem +// and points at `cio schema`, and costs no request. +// +// It is a gate, not a lookup: an error means the request must not be sent, and +// nil means let it through. Callers get nil both when the path matches a route +// and when the check cannot be trusted to judge it — no spec available, or a +// path outside the trees the spec documents. The API stays the authority on +// what exists; this only catches paths already known to be absent. +func preflightPath(cmd *cobra.Command, c *client.Client, httpMethod, resolvedPath string) error { + if skip, _ := cmd.Flags().GetBool("no-preflight"); skip { + return nil + } + + // Whatever the spec cache already holds, read without a lock or a + // download: this sits in front of every request, so it must not be able to + // block on another process or wait on the network. A cold cache means the + // check has no opinion, and `cio schema` is what fills it. + idx, err := routes.LoadPathIndexFromCache(specCacheOptions(c)) + if err != nil { + // Fail open: an absent or unparseable spec must never stop a caller + // from reaching an endpoint that does exist. + return nil + } + + // The route exists: nothing to block, so let the request proceed. The + // matched template is of no use here — the request keeps the caller's path. + if _, matched := idx.Lookup(httpMethod, resolvedPath); matched { + return nil + } + + // The path did not match, which is only grounds to reject it if it sits + // inside a scope the spec describes. Outside one, the spec has nothing to + // say: real endpoints are omitted from it, and whole trees may postdate it. + if !idx.Covers(resolvedPath) { + return nil + } + + // The shape exists but not for this verb — a different mistake, and one + // the caller fixes by changing -X rather than the path. + if allowed := idx.MethodsFor(resolvedPath); len(allowed) > 0 { + // Name the fix, not just the verb: the method is usually implicit here + // (GET unless a body was passed), so the caller needs to be told to + // set it rather than left to infer that from the list. + fix := fmt.Sprintf("the path accepts %s", strings.Join(allowed, ", ")) + if len(allowed) == 1 { + fix = fmt.Sprintf("retry with -X %s", allowed[0]) + } + err := fmt.Errorf( + "%s is not allowed on %s (%s); request not sent", + httpMethod, resolvedPath, fix) + output.PrintError(output.CodeValidationError, err.Error(), map[string]any{ + "method": httpMethod, + "path": resolvedPath, + "allowed_methods": allowed, + "skip_check_flag": "--no-preflight", + }) + return err + } + + suggestions := idx.Suggest(resolvedPath, 5) + hint := "run 'cio schema' to list resources" + if len(suggestions) > 0 && suggestions[0].Resource != "" { + hint = fmt.Sprintf("run 'cio schema %s' to list that resource's endpoints", suggestions[0].Resource) + } + // The spec omits a few real endpoints, and for those `cio schema` cannot + // list what it does not describe — so the way past a wrong rejection has to + // be in the message, not only in the details below. + hint += ", or resend with --no-preflight if you know the endpoint exists" + + closest := make([]string, 0, len(suggestions)) + for _, s := range suggestions { + closest = append(closest, s.String()) + } + + err = fmt.Errorf( + "%s %s is not an endpoint in the API spec, so the request was not sent; %s", + httpMethod, resolvedPath, hint) + output.PrintError(output.CodeValidationError, err.Error(), map[string]any{ + "method": httpMethod, + "path": resolvedPath, + "closest_endpoints": closest, + "skip_check_flag": "--no-preflight", + }) + return err +} + // resolveMethod determines the HTTP method from the flag or defaults. func resolveMethod(flag string, hasBody bool) string { if flag != "" { diff --git a/cmd/api_preflight_test.go b/cmd/api_preflight_test.go new file mode 100644 index 0000000..b5c2ffb --- /dev/null +++ b/cmd/api_preflight_test.go @@ -0,0 +1,343 @@ +package cmd + +import ( + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" +) + +// preflightSpec is a Journeys OpenAPI fixture with enough of the campaigns +// resource to exercise the pre-send path check: a collection with two verbs and +// a single-campaign read. +func preflightSpec() string { + return `{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/v1/environments/{environment_id}/campaigns": { + "get": {"summary": "List campaigns"}, + "post": {"summary": "Create campaign"} + }, + "/v1/environments/{environment_id}/campaigns/{campaign_id}": { + "get": {"summary": "Get campaign"} + }, + "/v1/environments/{environment_id}/campaigns/{campaign_id}/duplicate": { + "post": {"summary": "Duplicate campaign"} + } + } + }` +} + +// preflightServer serves the spec and echoes anything else, recording every +// path it was asked for — API paths so a test can assert a request never left +// the CLI, spec paths so a test can assert the check itself fetches nothing. +type preflightServer struct { + *httptest.Server + mu sync.Mutex + apiPaths []string + specPaths []string +} + +func (p *preflightServer) requested() []string { + p.mu.Lock() + defer p.mu.Unlock() + return append([]string(nil), p.apiPaths...) +} + +func (p *preflightServer) specFetches() []string { + p.mu.Lock() + defer p.mu.Unlock() + return append([]string(nil), p.specPaths...) +} + +func (p *preflightServer) forget() { + p.mu.Lock() + defer p.mu.Unlock() + p.apiPaths = nil + p.specPaths = nil +} + +// newPreflightServer starts the server without priming the spec cache, for +// tests that care about a cold cache. +func newPreflightServer(t *testing.T) *preflightServer { + t.Helper() + t.Setenv("HOME", t.TempDir()) + t.Setenv("CIO_TOKEN", "sa_live_test123") + t.Setenv("CIO_ACCESS_TOKEN", "") + + p := &preflightServer{} + p.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/service_accounts/oauth/token": + _, _ = w.Write([]byte(`{"access_token":"jwt-test-session","token_type":"Bearer","expires_in":3600}`)) + return + case "/v1/openapi.json", "/cdp/api/openapi.json": + p.mu.Lock() + p.specPaths = append(p.specPaths, r.URL.Path) + p.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + if r.URL.Path == "/v1/openapi.json" { + _, _ = w.Write([]byte(preflightSpec())) + } else { + _, _ = w.Write([]byte(`{"openapi":"3.1.0","info":{"title":"CDP","version":"1.0.0"},"paths":{}}`)) + } + return + } + + p.mu.Lock() + p.apiPaths = append(p.apiPaths, r.URL.Path) + p.mu.Unlock() + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + t.Cleanup(p.Close) + return p +} + +// setupPreflightTest primes the spec cache the way a session does — the check +// reads the cache and never downloads — then forgets the setup traffic. +func setupPreflightTest(t *testing.T) *preflightServer { + t.Helper() + p := newPreflightServer(t) + if _, _, err := executeCommand("schema", "--api-url", p.URL); err != nil { + t.Fatalf("priming the spec cache via schema: %v", err) + } + p.forget() + return p +} + +func TestAPIPreflight_BlocksUnknownSubPathWithoutSending(t *testing.T) { + server := setupPreflightTest(t) + + _, _, err := executeCommand("api", "/v1/environments/456/campaigns/48/actions", + "--api-url", server.URL) + if err == nil { + t.Fatal("expected an invented sub-path to be rejected before sending") + } + if !strings.Contains(err.Error(), "is not an endpoint in the API spec") { + t.Errorf("error should name the cause, got: %s", err.Error()) + } + if !strings.Contains(err.Error(), "request was not sent") { + t.Errorf("error should say the request was not sent, got: %s", err.Error()) + } + // The prescription matters as much as the diagnosis: a caller that only + // learns "that failed" tries a variant instead of reading the schema. + if !strings.Contains(err.Error(), "cio schema campaigns") { + t.Errorf("error should point at the resource's schema, got: %s", err.Error()) + } + if got := server.requested(); len(got) != 0 { + t.Errorf("no API request should have been made, got %v", got) + } +} + +func TestAPIPreflight_AllowsKnownPaths(t *testing.T) { + server := setupPreflightTest(t) + + for _, path := range []string{ + "/v1/environments/456/campaigns", + "/v1/environments/456/campaigns/48", + } { + if _, _, err := executeCommand("api", path, "--api-url", server.URL); err != nil { + t.Fatalf("expected %s to be allowed, got: %v", path, err) + } + } + + if got := server.requested(); len(got) != 2 { + t.Errorf("expected both requests to reach the API, got %v", got) + } +} + +// The template form must pass the check too: --params fills it in before the +// path is judged. +func TestAPIPreflight_AllowsTemplateFormWithParams(t *testing.T) { + server := setupPreflightTest(t) + + _, _, err := executeCommand("api", "/v1/environments/{environment_id}/campaigns", + "--api-url", server.URL, + "--params", `{"environment_id": "456"}`) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := server.requested(); len(got) != 1 || got[0] != "/v1/environments/456/campaigns" { + t.Errorf("expected the resolved path to be requested, got %v", got) + } +} + +func TestAPIPreflight_ReportsAllowedMethodsForWrongVerb(t *testing.T) { + server := setupPreflightTest(t) + + _, _, err := executeCommand("api", "/v1/environments/456/campaigns", + "--api-url", server.URL, + "-X", "DELETE") + if err == nil { + t.Fatal("expected DELETE on a GET/POST collection to be rejected") + } + if !strings.Contains(err.Error(), "DELETE is not allowed") { + t.Errorf("error should name the rejected verb, got: %s", err.Error()) + } + if !strings.Contains(err.Error(), "GET, POST") { + t.Errorf("error should list the accepted verbs, got: %s", err.Error()) + } + if got := server.requested(); len(got) != 0 { + t.Errorf("no API request should have been made, got %v", got) + } +} + +// The method is usually implicit — GET unless a body is passed — so a route +// that exists for one other verb must name the flag that fixes the call. +func TestAPIPreflight_NamesTheMethodFlagForASingleAllowedVerb(t *testing.T) { + server := setupPreflightTest(t) + + _, _, err := executeCommand("api", "/v1/environments/456/campaigns/48/duplicate", + "--api-url", server.URL) + if err == nil { + t.Fatal("expected an implicit GET on a POST-only route to be rejected") + } + if !strings.Contains(err.Error(), "retry with -X POST") { + t.Errorf("error should name the flag that fixes it, got: %s", err.Error()) + } + if got := server.requested(); len(got) != 0 { + t.Errorf("no API request should have been made, got %v", got) + } + + // And with the method set, the same path goes through. + if _, _, err := executeCommand("api", "/v1/environments/456/campaigns/48/duplicate", + "--api-url", server.URL, "-X", "post"); err != nil { + t.Fatalf("expected -X post (case-insensitive) to be allowed, got: %v", err) + } + if got := server.requested(); len(got) != 1 { + t.Errorf("expected the POST to reach the API, got %v", got) + } +} + +func TestAPIPreflight_NoPreflightFlagSendsAnyway(t *testing.T) { + server := setupPreflightTest(t) + + _, _, err := executeCommand("api", "/v1/environments/456/campaigns/48/actions", + "--api-url", server.URL, + "--no-preflight") + if err != nil { + t.Fatalf("--no-preflight should bypass the check, got: %v", err) + } + if got := server.requested(); len(got) != 1 || got[0] != "/v1/environments/456/campaigns/48/actions" { + t.Errorf("expected the request to be sent unchecked, got %v", got) + } +} + +// A dry run must not report a path as valid when the spec does not describe it. +func TestAPIPreflight_DryRunRejectsUnknownPath(t *testing.T) { + server := setupPreflightTest(t) + + stdout, _, err := executeCommand("api", "/v1/environments/456/campaigns/48/actions", + "--api-url", server.URL, + "--dry-run") + if err == nil { + t.Fatal("expected a dry run to reject an unknown path") + } + if strings.Contains(stdout, `"valid": true`) { + t.Errorf("dry run must not claim an unknown path is valid, got: %s", stdout) + } +} + +// Paths too shallow to sit inside a documented scope cannot be judged absent. +// "/v1/login" is the case that matters: it is a real endpoint the spec omits, +// so a check that policed everything under "/v1" would block a working call. +func TestAPIPreflight_AllowsPathsOutsideSpecScopes(t *testing.T) { + for _, path := range []string{"/health", "/v1/login", "/v2/environments/456/campaigns"} { + t.Run(path, func(t *testing.T) { + server := setupPreflightTest(t) + + if _, _, err := executeCommand("api", path, "--api-url", server.URL); err != nil { + t.Fatalf("%s must not be blocked, got: %v", path, err) + } + if got := server.requested(); len(got) != 1 || got[0] != path { + t.Errorf("expected %s to be requested, got %v", path, got) + } + }) + } +} + +// A resource invented alongside real ones is inside a documented scope, so it +// is judged — this is the other half of the observed failure mode. +func TestAPIPreflight_BlocksInventedSiblingResource(t *testing.T) { + server := setupPreflightTest(t) + + _, _, err := executeCommand("api", "/v1/environments/456/attribute_names", + "--api-url", server.URL) + if err == nil { + t.Fatal("expected an invented sibling resource to be rejected") + } + if got := server.requested(); len(got) != 0 { + t.Errorf("no API request should have been made, got %v", got) + } +} + +// The check reads the spec cache and must never fetch: EnsureSpecs takes an +// uninterruptible cache lock and can sit through two spec downloads, which is +// not something a per-request check may put in the caller's way. +func TestAPIPreflight_NeverFetchesTheSpecItself(t *testing.T) { + server := setupPreflightTest(t) + + // Rejects from the primed cache, with no spec request of its own. + if _, _, err := executeCommand("api", "/v1/environments/456/campaigns/48/actions", + "--api-url", server.URL); err == nil { + t.Fatal("expected the cached spec to reject an invented sub-path") + } + if got := server.specFetches(); len(got) != 0 { + t.Errorf("the check must not download specs, got %v", got) + } +} + +// A cold cache means no opinion: the call proceeds, and still nothing is +// fetched to form one. +func TestAPIPreflight_ColdCacheIsInertAndSilent(t *testing.T) { + server := newPreflightServer(t) + + if _, _, err := executeCommand("api", "/v1/environments/456/campaigns/48/actions", + "--api-url", server.URL); err != nil { + t.Fatalf("a cold cache must not block the call, got: %v", err) + } + if got := server.requested(); len(got) != 1 { + t.Errorf("expected the request to be sent, got %v", got) + } + if got := server.specFetches(); len(got) != 0 { + t.Errorf("the check must not download specs to populate itself, got %v", got) + } +} + +// When the spec cannot be loaded the check has nothing to judge against, and +// must not stand between the caller and an endpoint that does exist. +func TestAPIPreflight_FailsOpenWhenSpecUnavailable(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("CIO_TOKEN", "sa_live_test123") + t.Setenv("CIO_ACCESS_TOKEN", "") + + var mu sync.Mutex + var apiPaths []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/service_accounts/oauth/token": + _, _ = w.Write([]byte(`{"access_token":"jwt-test-session","token_type":"Bearer","expires_in":3600}`)) + case "/v1/openapi.json", "/cdp/api/openapi.json": + http.Error(w, "spec unavailable", http.StatusInternalServerError) + default: + mu.Lock() + apiPaths = append(apiPaths, r.URL.Path) + mu.Unlock() + _, _ = w.Write([]byte(`{"ok":true}`)) + } + })) + defer server.Close() + + if _, _, err := executeCommand("api", "/v1/environments/456/campaigns", "--api-url", server.URL); err != nil { + t.Fatalf("expected the call to proceed without a spec, got: %v", err) + } + + mu.Lock() + got := append([]string(nil), apiPaths...) + mu.Unlock() + if len(got) != 1 || got[0] != "/v1/environments/456/campaigns" { + t.Errorf("expected the request to be sent, got %v", got) + } +} diff --git a/cmd/auth_test.go b/cmd/auth_test.go index b693750..faef651 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -53,6 +53,10 @@ func executeCommand(args ...string) (stdout, stderr string, err error) { if f, ok := apiCmd.Flags().Lookup("file").Value.(pflag.SliceValue); ok { _ = f.Replace(nil) } + if f := apiCmd.Flags().Lookup("no-preflight"); f != nil { + _ = apiCmd.Flags().Set("no-preflight", "false") + f.Changed = false + } // Reset auth login flags; Changed must clear too, or the // mutually-exclusive-flags check sees stale state from earlier tests. diff --git a/cmd/helpers.go b/cmd/helpers.go index ff68457..c1e51c2 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -9,9 +9,58 @@ import ( "github.com/customerio/cli/internal/client" "github.com/customerio/cli/internal/output" + "github.com/customerio/cli/internal/routes" "github.com/spf13/cobra" ) +// specLoadOptions builds the spec-download options shared by the commands that +// read the route registry, so `cio api`'s pre-send check and `cio schema` +// always resolve the same spec — and the same cache entry — for one identity. +func specLoadOptions(cmd *cobra.Command, c *client.Client) routes.LoadRegistryOptions { + var opts routes.LoadRegistryOptions + if c == nil { + return opts + } + + opts.BaseURL = c.BaseURL() + switch { + case c.ServiceAccountToken() != "": + jwt, err := c.EnsureAccessToken(cmd.Context()) + if err == nil { + opts.AccessToken = jwt + opts.CacheKey = c.ServiceAccountToken() + } + case c.AccessToken() != "": + // Pre-exchanged JWT (e.g. CIO_ACCESS_TOKEN, as the in-product agent uses) + // — send it so the server returns the plan-filtered spec, not the full one. + opts.AccessToken = c.AccessToken() + opts.CacheKey = opts.AccessToken + } + return opts +} + +// specCacheOptions locates the spec cache for the current identity without +// acquiring a token for it. Only a download needs credentials — reading what +// the cache already holds needs the cache key alone, and asking for a token +// would mean an exchange over the network, which is far more than a local +// lookup should cost. The key matches the one specLoadOptions uses, so both +// commands read and write the same directory. +func specCacheOptions(c *client.Client) routes.LoadRegistryOptions { + var opts routes.LoadRegistryOptions + if c == nil { + return opts + } + + opts.BaseURL = c.BaseURL() + switch { + case c.ServiceAccountToken() != "": + opts.CacheKey = c.ServiceAccountToken() + case c.AccessToken() != "": + opts.CacheKey = c.AccessToken() + } + return opts +} + // doPageAll runs auto-pagination and writes NDJSON to stdout. func doPageAll(cmd *cobra.Command, c *client.Client, path string, params map[string]string, startPage, limit int) error { jq := GetJQFlag(cmd) diff --git a/cmd/prime_context.md b/cmd/prime_context.md index b64a80d..eff8666 100644 --- a/cmd/prime_context.md +++ b/cmd/prime_context.md @@ -193,6 +193,18 @@ Errors are structured JSON on stderr: Read the `message` field — it usually tells you what's wrong (missing field, wrong format, etc.). +### Paths the spec does not describe + +`cio api` checks a path against the API spec before sending it. A path the spec +does not describe is rejected without a request, and the error names the closest +real endpoints plus the `cio schema` command that lists them — so treat it as +"this endpoint does not exist", not as an empty result, and look the resource up +rather than trying another spelling. A path that exists only for another method +reports that method and the `-X` flag to use. + +A few real endpoints are absent from the spec. If you know the path exists, +resend it with `--no-preflight`. + ## Retry Behavior Automatic retries on HTTP 429 and 5xx with exponential backoff and jitter. Default: 3 retries. Respects `Retry-After` headers. diff --git a/cmd/schema.go b/cmd/schema.go index 3fb0aca..8cea6d0 100644 --- a/cmd/schema.go +++ b/cmd/schema.go @@ -39,36 +39,14 @@ func runSchema(cmd *cobra.Command, args []string) error { refresh, _ := cmd.Flags().GetBool("refresh") compact, _ := cmd.Flags().GetBool("compact") - var baseURL string - var accessToken string - var cacheKey string - if c := clientFromCmd(cmd); c != nil { - baseURL = c.BaseURL() - switch { - case c.ServiceAccountToken() != "": - jwt, err := c.EnsureAccessToken(cmd.Context()) - if err == nil { - accessToken = jwt - cacheKey = c.ServiceAccountToken() - } - case c.AccessToken() != "": - // Pre-exchanged JWT (e.g. CIO_ACCESS_TOKEN, as the in-product agent uses) - // — send it so the server returns the plan-filtered spec, not the full one. - accessToken = c.AccessToken() - cacheKey = accessToken - } - } + opts := specLoadOptions(cmd, clientFromCmd(cmd)) + opts.ForceRefresh = refresh if GetDryRun(cmd) { - return schemaDryRun(cmd, baseURL, accessToken) + return schemaDryRun(cmd, opts.BaseURL, opts.AccessToken) } - reg, err := routes.LoadRegistry(routes.LoadRegistryOptions{ - BaseURL: baseURL, - AccessToken: accessToken, - CacheKey: cacheKey, - ForceRefresh: refresh, - }) + reg, err := routes.LoadRegistry(opts) if err != nil { output.PrintError(output.CodeGeneralError, fmt.Sprintf("failed to load routes: %v", err), nil) return err diff --git a/internal/routes/pathindex.go b/internal/routes/pathindex.go new file mode 100644 index 0000000..9bd3974 --- /dev/null +++ b/internal/routes/pathindex.go @@ -0,0 +1,315 @@ +package routes + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strings" +) + +// PathIndex is a lightweight view of the API surface: the HTTP method and path +// template of every route, without the request and response schemas a Registry +// resolves. Decoding one costs a few milliseconds against a cached spec where +// building a Registry costs closer to a second, which is what makes it usable +// as a pre-send check on every call rather than only in `cio schema`. +type PathIndex struct { + entries []indexEntry +} + +type indexEntry struct { + httpMethod string + path string + segments []string +} + +// Suggestion is a route offered as a near miss for a path that did not match. +type Suggestion struct { + HTTPMethod string + Path string + // Resource is the CLI resource name, suitable for `cio schema `. + Resource string +} + +// String renders the suggestion as "GET /v1/environments/{environment_id}/campaigns". +func (s Suggestion) String() string { + return s.HTTPMethod + " " + s.Path +} + +// pathItemVerbs are the OpenAPI path-item keys that describe an operation. A +// path item also carries non-operation keys (parameters, summary, servers, +// $ref), which must not be read as HTTP methods. +var pathItemVerbs = map[string]bool{ + "get": true, "put": true, "post": true, "delete": true, + "options": true, "head": true, "patch": true, "trace": true, +} + +// LoadPathIndexFromCache decodes the path templates out of whatever specs the +// cache already holds. It reads files and nothing else: no download, no cache +// lock, no metadata. +// +// EnsureSpecs, which LoadRegistry goes through, is the wrong tool in front of a +// request. It takes an exclusive lock on the cache directory before reading +// anything, and that lock is a plain flock — it cannot be bounded or +// cancelled — so concurrent callers serialize behind whichever one holds it, +// and the holder may sit through two sequential spec downloads first. A check +// worth ~25ms must not be able to cost a minute, nor make parallel calls queue. +// +// Reading without the lock is safe because cached specs are replaced by rename, +// so a reader sees one whole version or the previous one, never a torn file. +// The cost is that this reports what the cache knows rather than what the +// server currently serves: a caller that needs freshness (`cio schema`) keeps +// going through EnsureSpecs, and a caller checking a path treats a cold cache +// as "no opinion". +func LoadPathIndexFromCache(opts LoadRegistryOptions) (*PathIndex, error) { + cacheDir, err := opts.resolveCacheDir() + if err != nil { + return nil, err + } + + idx := &PathIndex{} + for _, src := range defaultSpecSources { + data, readErr := os.ReadFile(filepath.Join(cacheDir, src.Name+".json")) + if readErr != nil { + continue + } + // A spec that will not parse is skipped rather than fatal: the other + // one still describes its own tree, and a tree with no routes indexed + // simply goes unjudged. + _ = idx.add(data) + } + + if len(idx.entries) == 0 { + return nil, fmt.Errorf("no cached API spec describes any route") + } + return idx, nil +} + +// add indexes one spec document, accepting either an OpenAPI document or the +// walked-routes JSON that LoadRegistryFromData reads. +func (idx *PathIndex) add(data []byte) error { + if IsOpenAPISpec(data) { + var doc struct { + Paths map[string]map[string]json.RawMessage `json:"paths"` + } + if err := json.Unmarshal(data, &doc); err != nil { + return fmt.Errorf("parsing OpenAPI paths: %w", err) + } + for path, pathItem := range doc.Paths { + for key := range pathItem { + if !pathItemVerbs[strings.ToLower(key)] { + continue + } + idx.push(strings.ToUpper(key), path) + } + } + return nil + } + + var walked []walkedRoute + if err := json.Unmarshal(data, &walked); err != nil { + return fmt.Errorf("parsing walked routes JSON: %w", err) + } + for _, wr := range walked { + // Normalize :param to {param}, matching LoadRegistryFromData. + idx.push(strings.ToUpper(wr.Method), colonParamRegex.ReplaceAllString(wr.Path, "{${1}}")) + } + return nil +} + +func (idx *PathIndex) push(httpMethod, path string) { + idx.entries = append(idx.entries, indexEntry{ + httpMethod: httpMethod, + path: path, + segments: splitPathSegments(path), + }) +} + +// Len reports how many method/path pairs the index holds. +func (idx *PathIndex) Len() int { + return len(idx.entries) +} + +// Lookup returns the path template matching the given method and path, which +// may be given in either template form ("/v1/environments/{environment_id}/campaigns") +// or resolved form ("/v1/environments/123/campaigns"). +func (idx *PathIndex) Lookup(httpMethod, path string) (string, bool) { + reqSegs := splitPathSegments(path) + for i := range idx.entries { + e := &idx.entries[i] + if e.httpMethod != strings.ToUpper(httpMethod) { + continue + } + if segmentsMatch(e.segments, reqSegs) { + return e.path, true + } + } + return "", false +} + +// MethodsFor returns the HTTP methods the index holds for the given path +// shape, sorted. An empty result means no route has that shape at all, which +// distinguishes an unknown path from a path called with the wrong method. +func (idx *PathIndex) MethodsFor(path string) []string { + reqSegs := splitPathSegments(path) + seen := make(map[string]bool) + var methods []string + for i := range idx.entries { + e := &idx.entries[i] + if !segmentsMatch(e.segments, reqSegs) || seen[e.httpMethod] { + continue + } + seen[e.httpMethod] = true + methods = append(methods, e.httpMethod) + } + sort.Strings(methods) + return methods +} + +// CoverageDepth is how many leading segments a path must share with a +// documented route before this index is willing to call the path absent. +// +// Three is the length of a scope prefix — "/v1/environments/{environment_id}", +// "/v1/accounts/{account_id}", "/cdp/api/workspaces/{workspace_id}" — so a +// path is judged only once it is inside a scope the spec describes. Matching +// less than that (the first segment alone, say) polices every path under "/v1", +// including real endpoints the spec deliberately omits, while still leaving a +// whole undocumented tree such as "/v2/..." unchecked. Neither is this index's +// business: it exists to catch a wrong turn inside a documented neighbourhood, +// not to police the shape of the API. +// +// Two limits come with the choice. A real endpoint the spec omits from inside +// a documented scope is still called absent, and reaching it needs the check +// turned off. And a bogus segment sitting where a template expects an id +// ("…/customers/search" against "…/customers/{customer_id}") matches by shape, +// so the server, not this index, is what rejects it. +const CoverageDepth = 3 + +// Covers reports whether the path sits deep enough inside a documented scope +// for its absence from the index to mean something. A path that shares less +// than CoverageDepth leading segments with every known route — "/health", +// "/v1/login", anything under a tree the spec does not describe — cannot be +// called absent on the strength of this index alone. +func (idx *PathIndex) Covers(path string) bool { + reqSegs := splitPathSegments(path) + if len(reqSegs) < CoverageDepth { + return false + } + for i := range idx.entries { + if sharedLeadingSegments(idx.entries[i].segments, reqSegs) >= CoverageDepth { + return true + } + } + return false +} + +// Suggest returns up to limit routes that share the longest leading run of +// segments with path, nearest first. Routes sharing fewer than two leading +// segments are dropped: at that distance the suggestion is noise. +func (idx *PathIndex) Suggest(path string, limit int) []Suggestion { + const minSharedSegments = 2 + + reqSegs := splitPathSegments(path) + type scored struct { + entry *indexEntry + score int + } + + var candidates []scored + for i := range idx.entries { + e := &idx.entries[i] + score := sharedLeadingSegments(e.segments, reqSegs) + if score < minSharedSegments { + continue + } + candidates = append(candidates, scored{entry: e, score: score}) + } + + sort.SliceStable(candidates, func(i, j int) bool { + if candidates[i].score != candidates[j].score { + return candidates[i].score > candidates[j].score + } + // Prefer the shape closest in length to what was asked for, then a + // stable alphabetical order so output does not vary between runs. + di := absDiff(len(candidates[i].entry.segments), len(reqSegs)) + dj := absDiff(len(candidates[j].entry.segments), len(reqSegs)) + if di != dj { + return di < dj + } + if candidates[i].entry.path != candidates[j].entry.path { + return candidates[i].entry.path < candidates[j].entry.path + } + return candidates[i].entry.httpMethod < candidates[j].entry.httpMethod + }) + + var out []Suggestion + seen := make(map[string]bool) + for _, c := range candidates { + if len(out) >= limit { + break + } + key := c.entry.httpMethod + " " + c.entry.path + if seen[key] { + continue + } + seen[key] = true + resource, _ := deriveFromWalkedPath(c.entry.path, c.entry.httpMethod) + out = append(out, Suggestion{ + HTTPMethod: c.entry.httpMethod, + Path: c.entry.path, + Resource: resource, + }) + } + return out +} + +func splitPathSegments(p string) []string { + p = strings.Trim(p, "/") + if p == "" { + return nil + } + return strings.Split(p, "/") +} + +func isPlaceholderSegment(seg string) bool { + return strings.HasPrefix(seg, "{") && strings.HasSuffix(seg, "}") +} + +// segmentsMatch reports whether a path template's segments match a request's. +// A {placeholder} matches any single concrete segment, so both the template +// and resolved forms of a path match the same route. +func segmentsMatch(templateSegs, reqSegs []string) bool { + if len(templateSegs) != len(reqSegs) { + return false + } + for i, ts := range templateSegs { + if isPlaceholderSegment(ts) { + continue + } + if ts != reqSegs[i] { + return false + } + } + return true +} + +// sharedLeadingSegments counts how many leading segments two paths have in +// common, treating a {placeholder} as matching any concrete segment. +func sharedLeadingSegments(templateSegs, reqSegs []string) int { + n := 0 + for i := 0; i < len(templateSegs) && i < len(reqSegs); i++ { + if !isPlaceholderSegment(templateSegs[i]) && templateSegs[i] != reqSegs[i] { + break + } + n++ + } + return n +} + +func absDiff(a, b int) int { + if a > b { + return a - b + } + return b - a +} diff --git a/internal/routes/pathindex_test.go b/internal/routes/pathindex_test.go new file mode 100644 index 0000000..8911d49 --- /dev/null +++ b/internal/routes/pathindex_test.go @@ -0,0 +1,263 @@ +package routes + +import ( + "strings" + "testing" +) + +// indexFixture builds an index directly, bypassing the spec cache, so the +// matching rules can be tested without a server or a temp HOME. +func indexFixture(t *testing.T, pairs ...string) *PathIndex { + t.Helper() + idx := &PathIndex{} + for _, p := range pairs { + parts := strings.SplitN(p, " ", 2) + if len(parts) != 2 { + t.Fatalf("fixture entry %q must be \"METHOD /path\"", p) + } + idx.push(parts[0], parts[1]) + } + return idx +} + +func campaignsIndex(t *testing.T) *PathIndex { + t.Helper() + return indexFixture(t, + "GET /v1/environments/{environment_id}/campaigns", + "POST /v1/environments/{environment_id}/campaigns", + "GET /v1/environments/{environment_id}/campaigns/{campaign_id}", + "PUT /v1/environments/{environment_id}/campaigns/{campaign_id}", + "GET /v1/environments/{environment_id}/campaigns/{campaign_id}/action_metrics", + "POST /v1/environments/{environment_id}/newsletters/{newsletter_id}/translations", + "GET /v1/environments/{environment_id}/actions/{action_id}", + "GET /cdp/api/workspaces/{workspace_id}/sources", + ) +} + +// Nested sub-resources — /v1/a/{a_id}/b/{b_id}/c — are where the invented +// paths show up, so a real one must pass and its invented neighbours must not, +// at any depth. +func TestPathIndexNestedSubResources(t *testing.T) { + idx := campaignsIndex(t) + + if _, ok := idx.Lookup("GET", "/v1/environments/456/campaigns/48/action_metrics"); !ok { + t.Error("expected a real nested sub-resource to match") + } + + for _, path := range []string{ + "/v1/environments/456/campaigns/48/actions", + "/v1/environments/456/campaigns/48/actions/12", + "/v1/environments/456/campaigns/48/foo/bar", + "/v1/environments/456/newsletters/7/made_up", + } { + if _, ok := idx.Lookup("GET", path); ok { + t.Errorf("expected %s not to match", path) + } + if !idx.Covers(path) { + t.Errorf("expected %s to be judged (deep inside a documented scope)", path) + } + } + + // A nested route that exists only for another verb reports that verb + // rather than being called absent. + nested := "/v1/environments/456/newsletters/7/translations" + if _, ok := idx.Lookup("GET", nested); ok { + t.Error("expected GET not to match a POST-only nested route") + } + if methods := idx.MethodsFor(nested); strings.Join(methods, ",") != "POST" { + t.Errorf("expected POST to be reported for %s, got %v", nested, methods) + } +} + +func TestPathIndexLookupTemplateAndResolvedForms(t *testing.T) { + idx := campaignsIndex(t) + + for _, path := range []string{ + "/v1/environments/{environment_id}/campaigns", + "/v1/environments/456/campaigns", + } { + if _, ok := idx.Lookup("GET", path); !ok { + t.Errorf("expected GET %s to match a route", path) + } + } + + template, ok := idx.Lookup("GET", "/v1/environments/456/campaigns/48") + if !ok { + t.Fatal("expected a resolved two-id path to match") + } + if template != "/v1/environments/{environment_id}/campaigns/{campaign_id}" { + t.Errorf("unexpected template: %s", template) + } +} + +// The sub-path form of a resource read is the mistake this index exists to +// catch: campaign actions come back inside the campaign, and there is no +// /campaigns/:id/actions route to call. +func TestPathIndexRejectsUnknownSubPath(t *testing.T) { + idx := campaignsIndex(t) + + if _, ok := idx.Lookup("GET", "/v1/environments/456/campaigns/48/actions"); ok { + t.Error("expected an invented sub-path not to match") + } + if methods := idx.MethodsFor("/v1/environments/456/campaigns/48/actions"); len(methods) != 0 { + t.Errorf("expected no methods for an unknown shape, got %v", methods) + } +} + +func TestPathIndexMethodsForDistinguishesWrongVerb(t *testing.T) { + idx := campaignsIndex(t) + + if _, ok := idx.Lookup("DELETE", "/v1/environments/456/campaigns"); ok { + t.Error("expected DELETE not to match the campaigns collection") + } + + methods := idx.MethodsFor("/v1/environments/456/campaigns") + if strings.Join(methods, ",") != "GET,POST" { + t.Errorf("expected sorted GET,POST, got %v", methods) + } +} + +func TestPathIndexLookupIsMethodCaseInsensitive(t *testing.T) { + idx := campaignsIndex(t) + + if _, ok := idx.Lookup("get", "/v1/environments/456/campaigns"); !ok { + t.Error("expected a lower-case verb to match") + } +} + +func TestPathIndexCovers(t *testing.T) { + idx := campaignsIndex(t) + + // Inside a documented scope: absence from the index means something. + for _, path := range []string{ + "/v1/environments/456/campaigns", + "/v1/environments/456/campaigns/48/actions", + "/v1/environments/456/made_up_resource", + "/cdp/api/workspaces/1/sources", + } { + if !idx.Covers(path) { + t.Errorf("expected %s to be covered", path) + } + } + + // Too shallow to judge. "/v1/login" and friends are real endpoints the + // spec omits, so treating them as absent would block working calls; a + // tree the spec never described ("/v2/...") is equally not ours to judge. + for _, path := range []string{ + "/health", + "/v1/login", + "/v1/campaigns", + "/v2/environments/456/campaigns", + "/", + "", + } { + if idx.Covers(path) { + t.Errorf("expected %s not to be covered", path) + } + } +} + +func TestPathIndexSuggestRanksNearestFirst(t *testing.T) { + idx := campaignsIndex(t) + + got := idx.Suggest("/v1/environments/456/campaigns/48/actions", 3) + if len(got) == 0 { + t.Fatal("expected suggestions for a near-miss path") + } + + // Same shared prefix, same depth: a real sibling of the invented segment + // is a closer offer than the parent read one segment shorter. + if got[0].Path != "/v1/environments/{environment_id}/campaigns/{campaign_id}/action_metrics" { + t.Errorf("expected the same-depth sibling first, got %s", got[0].Path) + } + if got[0].String() != "GET /v1/environments/{environment_id}/campaigns/{campaign_id}/action_metrics" { + t.Errorf("unexpected rendering: %s", got[0].String()) + } + + // Whichever route ranks first, the hint it drives must name the resource + // the caller was reaching for. + if got[0].Resource != "campaigns" { + t.Errorf("expected a campaigns resource hint, got %q", got[0].Resource) + } + + // The parent read stays in the list, just behind. + var sawParent bool + for _, s := range got { + if s.Path == "/v1/environments/{environment_id}/campaigns/{campaign_id}" { + sawParent = true + } + } + if !sawParent { + t.Errorf("expected the campaign read among the suggestions, got %v", got) + } +} + +func TestPathIndexSuggestRespectsLimitAndDistance(t *testing.T) { + idx := campaignsIndex(t) + + if got := idx.Suggest("/v1/environments/456/campaigns", 2); len(got) > 2 { + t.Errorf("expected at most 2 suggestions, got %d", len(got)) + } + // Shares only "/cdp" with the indexed cdp route — below the two-segment + // floor, so suggesting it would be noise. + if got := idx.Suggest("/cdp/nonsense", 5); len(got) != 0 { + t.Errorf("expected no suggestions for a distant path, got %v", got) + } +} + +func TestPathIndexAddOpenAPISkipsNonOperationKeys(t *testing.T) { + spec := []byte(`{ + "openapi": "3.1.0", + "paths": { + "/v1/environments/{environment_id}/segments": { + "get": {"summary": "List segments"}, + "parameters": [{"name": "environment_id", "in": "path"}], + "summary": "Segments collection" + } + } + }`) + + idx := &PathIndex{} + if err := idx.add(spec); err != nil { + t.Fatalf("add: %v", err) + } + if idx.Len() != 1 { + t.Fatalf("expected only the get operation to be indexed, got %d entries", idx.Len()) + } + if _, ok := idx.Lookup("GET", "/v1/environments/9/segments"); !ok { + t.Error("expected the get operation to be indexed") + } + for _, verb := range []string{"PARAMETERS", "SUMMARY"} { + if _, ok := idx.Lookup(verb, "/v1/environments/9/segments"); ok { + t.Errorf("expected %s not to be indexed as a method", verb) + } + } +} + +func TestPathIndexAddWalkedRoutes(t *testing.T) { + walked := []byte(`[ + {"method": "GET", "path": "/v1/environments/:environment_id/campaigns"}, + {"method": "PUT", "path": "/v1/environments/:environment_id/campaigns/:campaign_id"} + ]`) + + idx := &PathIndex{} + if err := idx.add(walked); err != nil { + t.Fatalf("add: %v", err) + } + + // :param must normalize to {param} so both path forms still match. + template, ok := idx.Lookup("PUT", "/v1/environments/456/campaigns/48") + if !ok { + t.Fatal("expected a walked route to match a resolved path") + } + if template != "/v1/environments/{environment_id}/campaigns/{campaign_id}" { + t.Errorf("expected a normalized template, got %s", template) + } +} + +func TestPathIndexAddRejectsUnparseableSpec(t *testing.T) { + idx := &PathIndex{} + if err := idx.add([]byte(`{"not": "a spec"}`)); err == nil { + t.Error("expected an error for a document that is neither OpenAPI nor walked routes") + } +}