diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e219e3..1d56edf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,19 +6,60 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] -### Fixed +## [0.9.1] - 2026-10-09 + +### Security + +- **v0.9.0 binaries were built with Go 1.27.1, which has 9 published + vulnerabilities in code c1i calls. Upgrade.** This release builds with Go + 1.27.2 and `golang.org/x/net` 0.60.0, fixing 13 vulnerabilities in Go + 1.27.1's standard library and `x/net` 0.59.0's `http2`, 9 of them in code + c1i calls: GO-2026-6617, -6613, -6612, -6611, -6610, -6609, -6608, -6607, + -6605, -6604, -6603, -6600 and -6599. `google.golang.org/grpc` moves from + 1.83.2 to 1.84.0, and `grpc-gateway` and `timestamp-authority` to their + latest releases. + +### Upgrading from 0.9.0 + +No flags change. The OpenAPI spec cache moves to +`~/.c1i/cache/api-openapi.yaml`, so the first offline `docs openapi`, +`docs endpoints` or `docs endpoint` after upgrading fails until one fetch +succeeds. A spec refresh deletes any other file in `~/.c1i/cache/`, and a +write to the file token cache deletes the expired and unusable token files in +its tokens dir. The `docs endpoints --filter` no-match message no longer +names `c1i mcp` or `access-reviews`, since the spec now covers both. + +### Changed - **API discovery uses the current public API contract**, including MCP server, tool, and toolset operations. `docs openapi`, `docs endpoints`, and `docs endpoint` fetch `https://www.c1.ai/api/openapi.yaml` and use a source-specific cache at `~/.c1i/cache/api-openapi.yaml`. +### Fixed + +- **`docs endpoint` output changed from run to run.** A `$ref` shared by + several fields was expanded under whichever one a random map order reached + first, and left bare under the rest. The order is now fixed, so the same + endpoint prints the same schema every time. + +- **A non-spec response could poison the OpenAPI cache for 24 hours.** A 200 + whose body isn't an OpenAPI document, such as a captive portal's sign-in + page, was cached and served as the spec. Now only a body that parses as one + is cached; otherwise the existing cache is used, or the command fails. A + download that fails partway falls back the same way, and the cache file is + replaced atomically. Any fallback to an expired cache now prints a one-line + warning on stderr with the cache's age and the failure. + +- **With no home dir, the `docs` OpenAPI commands cached the spec in the + working directory**, under `./.c1i/cache/`. They now fetch the spec and use + it without a cache. + - **Old cache files stayed on disk forever.** A cache file renamed or dropped - by a release, such as the `openapi.yaml` the entry above replaces, was never - removed. Refreshing the OpenAPI spec now deletes any file in `~/.c1i/cache/` - that the running version doesn't use. It leaves subdirectories alone, and - skips the sweep when the cache dir is a symlink or, with no home dir, a path - relative to the working directory. + by a release, such as the `openapi.yaml` that `api-openapi.yaml` replaces, + was never removed. Refreshing the OpenAPI spec now deletes any file in + `~/.c1i/cache/` that the running version doesn't use. It leaves + subdirectories alone, and skips the sweep when the cache dir is a symlink. - **Stale access-token files stayed on disk forever.** Without a usable OS keyring, c1i caches one token file per set of credentials under @@ -33,12 +74,16 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). it takes a protobuf duration in seconds, such as `86400s`, matching `tasks update-grant-duration` and `entitlements create --duration-grant`. -- **Builds now use Go 1.27.2 and `golang.org/x/net` 0.60.0**, fixing 13 - published vulnerabilities in Go 1.27.1's standard library and `x/net` - 0.59.0's `http2`, 9 of them in code c1i calls: GO-2026-6617, -6613, -6612, - -6611, -6610, -6609, -6608, -6607, -6605, -6604, -6603, -6600 and -6599. - `google.golang.org/grpc` moves from 1.83.2 to 1.84.0, and `grpc-gateway` - and `timestamp-authority` to their latest releases. +- **The docs didn't say service principal delete and credential revoke are + soft.** `service-principals get` on a deleted principal and + `service-principals credentials get` on a revoked credential still return + the record with exit 0 and no field marking it. Their help and the README + now say so: the lists omit them, and `users get` shows a deleted principal + as `DELETED`. + +- The README's bash completion example wrote to `/etc/bash_completion.d`, + which needs root; it now installs per user. `docs endpoints --filter` help + now says it also matches operation descriptions. ## [0.9.0] - 2026-10-06 @@ -1880,7 +1925,8 @@ First changelog entry; releases through v0.1.5 predate this file (see the - CI enforces `gofmt` via golangci-lint; module-wide formatting normalized. -[Unreleased]: https://github.com/ConductorOne/c1i/compare/v0.9.0...HEAD +[Unreleased]: https://github.com/ConductorOne/c1i/compare/v0.9.1...HEAD +[0.9.1]: https://github.com/ConductorOne/c1i/releases/tag/v0.9.1 [0.9.0]: https://github.com/ConductorOne/c1i/releases/tag/v0.9.0 [0.8.0]: https://github.com/ConductorOne/c1i/releases/tag/v0.8.0 [0.7.0]: https://github.com/ConductorOne/c1i/releases/tag/v0.7.0 diff --git a/README.md b/README.md index 5d8d656..51e74b2 100644 --- a/README.md +++ b/README.md @@ -755,6 +755,11 @@ c1i service-principals credentials update --service-principal-id c1i service-principals credentials revoke --service-principal-id ``` +Delete and revoke are soft: `get` on a deleted principal and `credentials get` +on a revoked credential still return the record, exit 0, with no field marking +it (the principal's embedded `user.status` stays `ENABLED`). Only the lists +omit them; `users get ` shows `"status": "DELETED"` and `deletedAt`. + `--expires` takes a positive Go duration (e.g. `720h`). The server accepts the range `(0s, 4320h]` — up to 180 days — rejecting more with `value must be inside range (0s, 4320h0m0s]`. Fractional seconds are preserved. @@ -869,7 +874,7 @@ c1i docs guide c1i docs guide register-mcp-server ``` -`docs openapi`, `docs endpoints`, and `docs endpoint` use the current public API contract at `https://www.c1.ai/api/openapi.yaml`, including MCP server, tool, and toolset operations. The spec is cached for 24 hours at `~/.c1i/cache/api-openapi.yaml`; fetch failures fall back to that cache. +`docs openapi`, `docs endpoints`, and `docs endpoint` use the current public API contract at `https://www.c1.ai/api/openapi.yaml`, including MCP server, tool, and toolset operations. The spec is cached for 24 hours at `~/.c1i/cache/api-openapi.yaml`; fetch failures fall back to that cache, however old, with a one-line warning on stderr naming its age. `docs search` is a semantic search with no relevance threshold: every query returns up to 10 nearest matches, so even a nonsense query comes back with plausible-looking hits. A returned hit is not proof a concept exists, and an unexpected hit is not proof the thing you searched for is absent — read the snippet, or fetch the page with `docs page`, to judge. To check whether an API endpoint exists, use `docs endpoints --filter`, which has a real no-match. @@ -1324,7 +1329,8 @@ invocation. ```sh # bash -c1i completion bash > /etc/bash_completion.d/c1i +mkdir -p ~/.local/share/bash-completion/completions +c1i completion bash > ~/.local/share/bash-completion/completions/c1i # zsh c1i completion zsh > "${fpath[1]}/_c1i" diff --git a/cmd/agents.md b/cmd/agents.md index 54e10c9..014d4a1 100644 --- a/cmd/agents.md +++ b/cmd/agents.md @@ -115,11 +115,13 @@ Nor do those five call the same place, which matters for egress rules and for why one can fail while another works: `docs openapi`, `docs endpoints` and `docs endpoint` fetch `https://www.c1.ai/api/openapi.yaml` (cached 24h at `~/.c1i/cache/api-openapi.yaml`, so a run can return rows without sending a -request), while `docs search` and `docs page` call a third party — `api.mintlify.com` — -with a public client-side key. `docs search` is semantic with no relevance -threshold, so it always returns up to 10 plausible hits even for a nonsense -query: never read a result as proof a concept exists, or an unexpected result -as proof it is absent. `docs endpoints --filter` DOES have a true no-match. +request), while `docs search` and `docs page` call a third party — +`api.mintlify.com` — with a public client-side key. `docs search` is semantic +with no relevance threshold, so it always returns up to 10 plausible hits even +for a nonsense query: never read a result as proof a concept exists, or an +unexpected result as proof it is absent. `docs endpoints --filter` DOES have a +true no-match. If a fetch fails, an expired cache is served with a one-line +`Warning:` on stderr naming its age. Don't store files in `~/.c1i/cache/`: a spec refresh deletes any file there that the running version doesn't use. @@ -452,6 +454,11 @@ resource with `--resource-id` likewise means you drop exit `0` with `deletedAt` set. So a `deleted_at` in an `access-profiles list` row is null in practice — don't read the null as "not deleted", check with a get. +- `service-principals delete` and `service-principals credentials revoke` are + soft: `get` and `credentials get` still return the record at exit `0` with no + field marking it (the principal's `user.status` stays `ENABLED`). Only the + lists omit them; `users get ` shows `"status": "DELETED"` and + `deletedAt`. ## Upgrading c1i diff --git a/cmd/cache.go b/cmd/cache.go index 1af3f3b..35fe3e1 100644 --- a/cmd/cache.go +++ b/cmd/cache.go @@ -13,8 +13,8 @@ var cacheFiles = map[string]bool{cacheFileName: true} // pruneCacheDir is best-effort: cleanup must never fail the command. It skips // subdirectories, a symlinked dir (Lstat sees a link, not a dir) whose target -// may hold files c1i doesn't own, and a relative dir, which is what an unset -// HOME yields and would sweep the working directory instead. +// may hold files c1i doesn't own, and a relative dir, which would sweep the +// working directory instead. func pruneCacheDir(dir string) { if !filepath.IsAbs(dir) { return diff --git a/cmd/cache_test.go b/cmd/cache_test.go index 21a0b72..8da1245 100644 --- a/cmd/cache_test.go +++ b/cmd/cache_test.go @@ -3,11 +3,12 @@ package cmd import ( "bytes" "context" + "errors" "io" + "io/fs" "net/http" "os" "path/filepath" - "runtime" "strings" "testing" "time" @@ -17,37 +18,70 @@ type stubRoundTripper func(*http.Request) (*http.Response, error) func (f stubRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } -// refreshOpenAPICache runs `docs openapi` against an expired cache so it -// fetches a stub spec and rewrites the cache. -func refreshOpenAPICache(t *testing.T, cacheDir string) { +// runDocsOpenapi runs `docs openapi` with rt serving every request and returns +// its stdout and stderr. +func runDocsOpenapi(t *testing.T, rt stubRoundTripper) (string, string, error) { t.Helper() - cachePath := filepath.Join(cacheDir, cacheFileName) - if err := os.WriteFile(cachePath, []byte(stubOpenAPISpec), 0o600); err != nil { - t.Fatal(err) - } - expired := time.Now().Add(-2 * cacheMaxAge) - if err := os.Chtimes(cachePath, expired, expired); err != nil { - t.Fatal(err) - } - orig := http.DefaultClient.Transport - http.DefaultClient.Transport = stubRoundTripper(func(r *http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(strings.NewReader(stubOpenAPISpec)), - Request: r, - }, nil - }) + http.DefaultClient.Transport = rt t.Cleanup(func() { http.DefaultClient.Transport = orig }) + out, errOut := &bytes.Buffer{}, &bytes.Buffer{} prevCtx := docsOpenapiCmd.Context() - docsOpenapiCmd.SetOut(&bytes.Buffer{}) + docsOpenapiCmd.SetOut(out) + docsOpenapiCmd.SetErr(errOut) docsOpenapiCmd.SetContext(context.Background()) t.Cleanup(func() { docsOpenapiCmd.SetOut(nil) + docsOpenapiCmd.SetErr(nil) docsOpenapiCmd.SetContext(prevCtx) }) - if err := docsOpenapiCmd.RunE(docsOpenapiCmd, nil); err != nil { + err := docsOpenapiCmd.RunE(docsOpenapiCmd, nil) + return out.String(), errOut.String(), err +} + +func serveSpec(body string) stubRoundTripper { + return func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(body)), + Request: r, + }, nil + } +} + +// writeExpiredCache writes body as a cache file old enough to force a fetch. +func writeExpiredCache(t *testing.T, cachePath, body string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(cachePath), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(cachePath, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + expired := time.Now().Add(-2 * cacheMaxAge) + if err := os.Chtimes(cachePath, expired, expired); err != nil { + t.Fatal(err) + } +} + +// useTempHome points the home dir at a temp dir and returns the cache path. +// USERPROFILE is what os.UserHomeDir reads on Windows. +func useTempHome(t *testing.T) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + return filepath.Join(home, cacheDirName, "cache", cacheFileName) +} + +// refreshOpenAPICache runs `docs openapi` against an expired cache so it +// fetches a stub spec and rewrites the cache. +func refreshOpenAPICache(t *testing.T, cacheDir string) { + t.Helper() + cachePath := filepath.Join(cacheDir, cacheFileName) + writeExpiredCache(t, cachePath, stubOpenAPISpec) + if _, _, err := runDocsOpenapi(t, serveSpec(stubOpenAPISpec)); err != nil { t.Fatalf("docs openapi: %v", err) } info, err := os.Stat(cachePath) @@ -57,11 +91,12 @@ func refreshOpenAPICache(t *testing.T, cacheDir string) { } func TestCacheWritePrunesLeftoverFiles(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - cacheDir := filepath.Join(home, cacheDirName, "cache") - if err := os.MkdirAll(filepath.Join(cacheDir, "subdir"), 0o700); err != nil { - t.Fatal(err) + cacheDir := filepath.Dir(useTempHome(t)) + home := filepath.Dir(filepath.Dir(cacheDir)) + for _, d := range []string{"subdir", "empty-subdir"} { + if err := os.MkdirAll(filepath.Join(cacheDir, d), 0o700); err != nil { + t.Fatal(err) + } } for _, name := range []string{"openapi.yaml", ".tmp-leftover"} { if err := os.WriteFile(filepath.Join(cacheDir, name), []byte("x"), 0o600); err != nil { @@ -92,7 +127,7 @@ func TestCacheWritePrunesLeftoverFiles(t *testing.T) { for _, e := range entries { got = append(got, e.Name()) } - want := []string{cacheFileName, "subdir"} + want := []string{cacheFileName, "empty-subdir", "subdir"} if strings.Join(got, ",") != strings.Join(want, ",") { t.Errorf("cache dir holds %v, want %v", got, want) } @@ -105,8 +140,7 @@ func TestCacheWritePrunesLeftoverFiles(t *testing.T) { // A cache dir the user redirected elsewhere may hold files c1i doesn't own. func TestCachePruneSkipsSymlinkedCacheDir(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) + home := filepath.Dir(filepath.Dir(filepath.Dir(useTempHome(t)))) target := t.TempDir() stray := filepath.Join(target, "not-ours.txt") if err := os.WriteFile(stray, []byte("x"), 0o600); err != nil { @@ -127,25 +161,180 @@ func TestCachePruneSkipsSymlinkedCacheDir(t *testing.T) { } } -// With no home dir the cache path is relative, so it lands in the working dir. -func TestCachePruneSkipsRelativeCacheDir(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("an empty HOME doesn't unset the home dir on Windows") +// With no usable home dir the spec is fetched and used, and the working dir, +// where a relative cache path would land, is neither read, written nor pruned. +func TestOpenAPISpecWithoutHomeDirSkipsCache(t *testing.T) { + for _, home := range []string{"", "."} { + t.Run("HOME="+home, func(t *testing.T) { + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + if got, err := os.UserHomeDir(); err == nil && filepath.IsAbs(got) { + t.Skipf("home dir still resolves to %q", got) + } + t.Chdir(t.TempDir()) + relCache := filepath.Join(cacheDirName, "cache", cacheFileName) + writeExpiredCache(t, relCache, "paths: {}\n") + stray := filepath.Join(cacheDirName, "cache", "not-ours.txt") + if err := os.WriteFile(stray, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + before := snapshotTree(t, ".") + + got, _, err := runDocsOpenapi(t, serveSpec(stubOpenAPISpec)) + if err != nil { + t.Fatalf("docs openapi: %v", err) + } + if got != stubOpenAPISpec { + t.Errorf("printed %q, want the fetched spec", got) + } + if after := snapshotTree(t, "."); after != before { + t.Errorf("working dir changed:\nbefore: %s\nafter: %s", before, after) + } + }) } - t.Setenv("HOME", "") - t.Chdir(t.TempDir()) - cacheDir := filepath.Join(cacheDirName, "cache") - if err := os.MkdirAll(cacheDir, 0o700); err != nil { +} + +// snapshotTree lists every file under root with its contents. +func snapshotTree(t *testing.T, root string) string { + t.Helper() + var b strings.Builder + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() { + return err + } + data, err := os.ReadFile(p) + b.WriteString(p + "=" + string(data) + ";") + return err + }) + if err != nil { t.Fatal(err) } - stray := filepath.Join(cacheDir, "not-ours.txt") - if err := os.WriteFile(stray, []byte("x"), 0o600); err != nil { - t.Fatal(err) + return b.String() +} + +// Bodies a 200 can carry that aren't a spec: a captive portal page, which +// isn't a YAML map, and a JSON object with no paths. +var notASpec = []string{ + "Sign in to Wi-Fi", + `{"message": "sign in required"}`, +} + +func TestOpenAPIInvalidBodyKeepsCache(t *testing.T) { + for _, body := range notASpec { + cachePath := useTempHome(t) + writeExpiredCache(t, cachePath, stubOpenAPISpec) + + got, _, err := runDocsOpenapi(t, serveSpec(body)) + if err != nil { + t.Fatalf("docs openapi: %v", err) + } + if got != stubOpenAPISpec { + t.Errorf("printed %q, want the cached spec", got) + } + if b, err := os.ReadFile(cachePath); err != nil || string(b) != stubOpenAPISpec { + t.Errorf("cache was overwritten: %q, %v", b, err) + } } +} - refreshOpenAPICache(t, cacheDir) +func TestOpenAPIInvalidBodyWithoutCacheErrors(t *testing.T) { + for _, body := range notASpec { + cachePath := useTempHome(t) + + if _, _, err := runDocsOpenapi(t, serveSpec(body)); err == nil { + t.Errorf("expected an error for %q", body) + } + if _, err := os.Stat(filepath.Dir(cachePath)); !errors.Is(err, fs.ErrNotExist) { + t.Errorf("cache dir was created for %q: %v", body, err) + } + } +} + +type failingReader struct{} +func (failingReader) Read([]byte) (int, error) { return 0, errors.New("connection reset") } + +func TestOpenAPIReadErrorFallsBackToCache(t *testing.T) { + cachePath := useTempHome(t) + writeExpiredCache(t, cachePath, stubOpenAPISpec) + + got, _, err := runDocsOpenapi(t, func(r *http.Request) (*http.Response, error) { + body := io.MultiReader(strings.NewReader("paths:\n"), failingReader{}) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(body), Request: r}, nil + }) + if err != nil { + t.Fatalf("docs openapi: %v", err) + } + if got != stubOpenAPISpec { + t.Errorf("printed %q, want the cached spec", got) + } +} + +// Serving an expired cache because the fetch failed warns on stderr with its +// age and the reason; stdout stays the bare spec. +func TestOpenAPIStaleCacheWarns(t *testing.T) { + for name, rt := range map[string]stubRoundTripper{ + "network": func(*http.Request) (*http.Response, error) { return nil, errors.New("no route to host") }, + "status": func(r *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusServiceUnavailable, Body: http.NoBody, Request: r}, nil + }, + "not a spec": serveSpec(notASpec[0]), + } { + t.Run(name, func(t *testing.T) { + writeExpiredCache(t, useTempHome(t), stubOpenAPISpec) + + got, stderr, err := runDocsOpenapi(t, rt) + if err != nil { + t.Fatalf("docs openapi: %v", err) + } + if got != stubOpenAPISpec { + t.Errorf("printed %q, want the cached spec", got) + } + if want := "Warning: using cached OpenAPI spec from 2d ago ("; !strings.HasPrefix(stderr, want) || strings.Count(stderr, "\n") != 1 { + t.Errorf("stderr = %q, want one line starting %q", stderr, want) + } + }) + } +} + +func TestOpenAPIFreshCacheIsSilent(t *testing.T) { + primeOpenAPICache(t) + _, stderr, err := runDocsOpenapi(t, func(*http.Request) (*http.Response, error) { + t.Error("a fresh cache hit sent a request") + return nil, errors.New("unexpected request") + }) + if err != nil || stderr != "" { + t.Errorf("err = %v, stderr = %q; want neither", err, stderr) + } +} + +// The only caller never passes a relative dir today; this pins the guard so a +// future caller can't turn the prune loose on the working directory. +func TestPruneCacheDirSkipsRelativeDir(t *testing.T) { + t.Chdir(t.TempDir()) + dir := filepath.Join(cacheDirName, "cache") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + stray := filepath.Join(dir, "not-ours.txt") + if err := os.WriteFile(stray, []byte("keep"), 0o600); err != nil { + t.Fatal(err) + } + pruneCacheDir(dir) if _, err := os.Stat(stray); err != nil { - t.Errorf("file in a working-dir-relative cache dir was removed: %v", err) + t.Errorf("prune of a relative dir removed %s: %v", stray, err) + } +} + +func TestFormatAgeClampsFutureMtime(t *testing.T) { + for d, want := range map[time.Duration]string{ + -5 * time.Minute: "0m", + 90 * time.Second: "1m", + 3 * time.Hour: "3h", + 50 * time.Hour: "2d", + } { + if got := formatAge(d); got != want { + t.Errorf("formatAge(%v) = %q, want %q", d, got, want) + } } } diff --git a/cmd/docs_openapi.go b/cmd/docs_openapi.go index f18c41d..5e1378a 100644 --- a/cmd/docs_openapi.go +++ b/cmd/docs_openapi.go @@ -160,7 +160,7 @@ Examples: } func init() { - docsEndpointsCmd.Flags().String("filter", "", "Filter endpoints by pattern (matches path, summary, operation ID)") + docsEndpointsCmd.Flags().String("filter", "", "Filter endpoints by pattern (matches path, summary, operation ID, description)") docsCmd.AddCommand(docsOpenapiCmd) docsCmd.AddCommand(docsEndpointsCmd) docsCmd.AddCommand(docsEndpointCmd) @@ -169,8 +169,8 @@ func init() { func fetchOpenAPISpec(cmd *cobra.Command) ([]byte, error) { cachePath := openAPICachePath() - if info, err := os.Stat(cachePath); err == nil { - if time.Since(info.ModTime()) < cacheMaxAge { + if cachePath != "" { + if info, err := os.Stat(cachePath); err == nil && time.Since(info.ModTime()) < cacheMaxAge { return os.ReadFile(cachePath) // #nosec G304 -- cachePath is a fixed internal path (openAPICachePath), not caller input } } @@ -182,36 +182,92 @@ func fetchOpenAPISpec(cmd *cobra.Command) ([]byte, error) { resp, err := http.DefaultClient.Do(req) if err != nil { - // Fall back to cache on network error. - if data, readErr := os.ReadFile(cachePath); readErr == nil { // #nosec G304 -- cachePath is a fixed internal path (openAPICachePath), not caller input - return data, nil - } - return nil, fmt.Errorf("fetching OpenAPI spec: %w", err) + return staleOpenAPISpec(cmd, cachePath, fmt.Errorf("fetching OpenAPI spec: %w", err)) } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - if data, readErr := os.ReadFile(cachePath); readErr == nil { // #nosec G304 -- cachePath is a fixed internal path (openAPICachePath), not caller input - return data, nil - } - return nil, fmt.Errorf("fetching OpenAPI spec: HTTP %d", resp.StatusCode) + return staleOpenAPISpec(cmd, cachePath, fmt.Errorf("fetching OpenAPI spec: HTTP %d", resp.StatusCode)) } data, err := io.ReadAll(resp.Body) if err != nil { - return nil, err + return staleOpenAPISpec(cmd, cachePath, fmt.Errorf("reading OpenAPI spec: %w", err)) + } + // A captive portal answers 200 with HTML, and caching that would serve it + // until cacheMaxAge. + var doc map[string]any + err = yaml.Unmarshal(data, &doc) + if _, ok := doc["paths"]; err != nil || !ok { + return staleOpenAPISpec(cmd, cachePath, fmt.Errorf("fetching OpenAPI spec: %s did not return an OpenAPI document", openapiURL)) } - _ = os.MkdirAll(filepath.Dir(cachePath), 0o700) - if os.WriteFile(cachePath, data, 0o644) == nil { // #nosec G306 -- cached OpenAPI spec is public C1 API documentation, not sensitive - pruneCacheDir(filepath.Dir(cachePath)) + if cachePath != "" { + writeOpenAPICache(cachePath, data) } + return data, nil +} +// staleOpenAPISpec returns the cached spec, however old, or err if there is +// none. It warns on stderr, leaving stdout clean for pipes. +func staleOpenAPISpec(cmd *cobra.Command, cachePath string, err error) ([]byte, error) { + if cachePath == "" { + return nil, err + } + info, statErr := os.Stat(cachePath) + data, readErr := os.ReadFile(cachePath) // #nosec G304 -- cachePath is a fixed internal path (openAPICachePath), not caller input + if statErr != nil || readErr != nil { + return nil, err + } + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: using cached OpenAPI spec from %s ago (%v)\n", + formatAge(time.Since(info.ModTime())), err) return data, nil } +func formatAge(d time.Duration) string { + d = max(d, 0) // a future mtime (clock skew) reads as 0m, not negative + switch { + case d >= 24*time.Hour: + return fmt.Sprintf("%dd", int(d/(24*time.Hour))) + case d >= time.Hour: + return fmt.Sprintf("%dh", int(d/time.Hour)) + default: + return fmt.Sprintf("%dm", int(d/time.Minute)) + } +} + +// writeOpenAPICache is best-effort. The rename keeps a reader from seeing a +// partial file. The temp name isn't in cacheFiles, so the prune after the +// rename sweeps one a crash left behind; a concurrent refresh's prune can also +// remove ours, which only skips this write. +func writeOpenAPICache(cachePath string, data []byte) { + dir := filepath.Dir(cachePath) + if os.MkdirAll(dir, 0o700) != nil { + return + } + tmp, err := os.CreateTemp(dir, "."+cacheFileName+".tmp-*") + if err != nil { + return + } + tmpName := tmp.Name() + defer func() { _ = os.Remove(tmpName) }() + _, writeErr := tmp.Write(data) + chmodErr := tmp.Chmod(0o644) // #nosec G302 -- cached OpenAPI spec is public C1 API documentation, not sensitive + if closeErr := tmp.Close(); writeErr != nil || chmodErr != nil || closeErr != nil { + return + } + if os.Rename(tmpName, cachePath) == nil { + pruneCacheDir(dir) + } +} + +// openAPICachePath returns "" when there is no usable home dir, which disables +// the cache: a relative path would put it in the working directory. func openAPICachePath() string { - home, _ := os.UserHomeDir() + home, err := os.UserHomeDir() + if err != nil || !filepath.IsAbs(home) { + return "" + } return filepath.Join(home, cacheDirName, "cache", cacheFileName) } @@ -237,9 +293,16 @@ func resolveRefs(node any, root map[string]any, depth int, seen map[string]bool) return resolveRefs(resolved, root, depth+1, seen) } } + // seen is shared across siblings, so only the first to reach a ref + // expands it; sorted keys make that the same sibling on every run. + keys := make([]string, 0, len(v)) + for key := range v { + keys = append(keys, key) + } + sort.Strings(keys) out := make(map[string]any, len(v)) - for key, val := range v { - out[key] = resolveRefs(val, root, depth, seen) + for _, key := range keys { + out[key] = resolveRefs(v[key], root, depth, seen) } return out case []any: diff --git a/cmd/docs_openapi_test.go b/cmd/docs_openapi_test.go index 21cf2d9..46eb8be 100644 --- a/cmd/docs_openapi_test.go +++ b/cmd/docs_openapi_test.go @@ -2,10 +2,13 @@ package cmd import ( "bytes" + "encoding/json" "os" "path/filepath" "strings" "testing" + + "go.yaml.in/yaml/v3" ) // stubOpenAPISpec is a minimal spec with one path, just enough for @@ -19,15 +22,13 @@ paths: operationId: c1.api.user.v1.Users.Get ` -// primeOpenAPICache points HOME at a temp dir and pre-populates the OpenAPI -// cache file so fetchOpenAPISpec reads the stub spec above without hitting -// the network. The cache file's mtime is "now", which is inside the 24h +// primeOpenAPICache points the home dir at a temp dir and pre-populates the +// OpenAPI cache file so fetchOpenAPISpec reads the stub spec above without +// hitting the network. The cache file's mtime is "now", which is inside the 24h // cacheMaxAge window fetchOpenAPISpec checks. func primeOpenAPICache(t *testing.T) { t.Helper() - dir := t.TempDir() - t.Setenv("HOME", dir) - cacheDir := filepath.Join(dir, cacheDirName, "cache") + cacheDir := filepath.Dir(useTempHome(t)) if err := os.MkdirAll(cacheDir, 0o700); err != nil { t.Fatalf("failed to create cache dir: %v", err) } @@ -103,3 +104,44 @@ func TestDocsEndpointsNoFilterHasNoMissMessage(t *testing.T) { t.Errorf("expected no stderr output with no filter, got: %q", stderr) } } + +// Siblings sharing a ref must resolve the same way on every run. +func TestResolveRefsDeterministicAcrossSiblings(t *testing.T) { + const spec = ` +paths: + /things: + get: + responses: + "200": {$ref: "#/components/responses/Thing"} + post: + requestBody: {$ref: "#/components/schemas/Thing"} + responses: + "200": {$ref: "#/components/responses/Thing"} + put: + requestBody: {$ref: "#/components/schemas/Thing"} +components: + responses: + Thing: + content: {schema: {$ref: "#/components/schemas/Thing"}} + schemas: + Thing: + properties: {id: {type: string}} +` + var root map[string]any + if err := yaml.Unmarshal([]byte(spec), &root); err != nil { + t.Fatal(err) + } + pathObj := root["paths"].(map[string]any)["/things"] + var first []byte + for i := 0; i < 20; i++ { + out, err := json.Marshal(resolveRefs(pathObj, root, 0, nil)) + if err != nil { + t.Fatal(err) + } + if i == 0 { + first = out + } else if !bytes.Equal(out, first) { + t.Fatalf("run %d differs:\n%s\nfirst:\n%s", i, out, first) + } + } +} diff --git a/cmd/service_principals_credentials.go b/cmd/service_principals_credentials.go index 1c69d63..7a2eaee 100644 --- a/cmd/service_principals_credentials.go +++ b/cmd/service_principals_credentials.go @@ -111,7 +111,11 @@ var spCredentialsListCmd = &cobra.Command{ var spCredentialsGetCmd = &cobra.Command{ Use: "get ", Short: "Get a single client credential (pretty JSON)", - Args: cobra.ExactArgs(1), + Long: `Get a single client credential (pretty JSON). + +A revoked credential is still returned, unchanged and with exit 0: no field +marks it revoked. "service-principals credentials list" omits it.`, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if err := requireNonEmpty(cmd, "service-principal-id"); err != nil { return err diff --git a/cmd/service_principals_get.go b/cmd/service_principals_get.go index 1421e11..af0d584 100644 --- a/cmd/service_principals_get.go +++ b/cmd/service_principals_get.go @@ -9,7 +9,12 @@ import ( var servicePrincipalsGetCmd = &cobra.Command{ Use: "get ", Short: "Get a single service principal by ID (pretty JSON)", - Args: cobra.ExactArgs(1), + Long: `Get a single service principal by ID (pretty JSON). + +A deleted principal is still returned, with exit 0 and no field marking it +deleted: its embedded user.status stays "ENABLED". "service-principals list" +omits it, and "users get " shows "status": "DELETED" and "deletedAt".`, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { baseURL, err := GetBaseURL() if err != nil { diff --git a/internal/tokensource/cache_test.go b/internal/tokensource/cache_test.go index 7ac9054..689d78c 100644 --- a/internal/tokensource/cache_test.go +++ b/internal/tokensource/cache_test.go @@ -561,15 +561,31 @@ func TestStoreSweepLeavesCredentialsDir(t *testing.T) { } cred := filepath.Join(creds, cacheKey("h", "c", "cred-secret")+".json") writeTokenFile(t, cred, time.Now().Add(-time.Hour)) - + // Expired token-shaped files above the tokens dir, so only the sweep's + // scope keeps them. A 0700 root, as ~/.config often is, passes the trust check. key := testCacheKey("host", "client") + p, _ := cachePath(key) + c1iDir := filepath.Dir(filepath.Dir(p)) + if err := os.Chmod(filepath.Dir(c1iDir), 0o700); err != nil { + t.Fatal(err) + } + decoys := []string{ + filepath.Join(c1iDir, cacheKey("h", "c", "c1i-secret")+".json"), + filepath.Join(filepath.Dir(c1iDir), cacheKey("h", "c", "root-secret")+".json"), + } + for _, d := range decoys { + writeTokenFile(t, d, time.Now().Add(-time.Hour)) + } + storeCachedToken(key, freshToken(30*time.Minute)) - if p, _ := cachePath(key); loadFileCachedToken(key) == nil { + if loadFileCachedToken(key) == nil { t.Fatalf("store did not write %s, so the sweep never ran", p) } - if _, err := os.Lstat(cred); err != nil { - t.Errorf("credentials file was removed: %v", err) + for _, f := range append([]string{cred}, decoys...) { + if _, err := os.Lstat(f); err != nil { + t.Errorf("%s was removed: %v", f, err) + } } }