Skip to content
Open
13 changes: 13 additions & 0 deletions .cursor/bugbot-cmd.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,16 @@ Prefer `outputformat.PrintJSONEnvelope` for structured output so the payload is
## Pagination Safety

Validate that pagination never crosses host boundaries.

## Repeated Flag Shapes Register Through a Shared pflag.Value

A flag shape that recurs across commands, or carries the same validation requirement wherever
it appears (a count, a duration, an ID format), must register through one shared `pflag.Value`
(see `internal/countflags`, `cmd/internal/pollflags`) — not a fresh ad hoc check hand-rolled in
each command's `RunE`.

## Sentinel Defaults Are a Documented Exception

A flag where a value like `0` means "unset, use a default" (a port, a replica count) must not
be forced through a shared validator built for a different flag's semantics — but the exception
must be called out.
5 changes: 5 additions & 0 deletions .cursor/bugbot-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,8 @@ Extract utility functions only after the same code appears in 2+ independent pla
## Scope Discipline: Foundational PRs

Foundational PRs should do architectural work without bundling unrelated refactorings.

## Document Intentional Asymmetry Between Look-Alike Siblings

When two structurally similar functions/phases behave differently on failure by design, say so
inline — otherwise a later refactor "fixes" the asymmetry and breaks the reason it existed.
11 changes: 11 additions & 0 deletions .cursor/bugbot-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,14 @@ one shared sentinel — not two phrasings that differ by which path reached the
An error whose only consumer is a boolean — file-type predicates, "is this ours" checks — must
not be phrased as user guidance. Someone will later "align" it with a real message and put
a remedy into a string nothing prints.

## Terminal Errors Must Name the Exact Recovery Command

A refusal in a terminal/blocked state must name the exact command that unblocks it, not "check
the logs" — and that command must not itself dead-end into another unexplained error.

## Retry Safety Must Be Classified by Response Shape, Not Status Code Alone

A status code that's sometimes-safe to retry (502/504) must not be retried using the same rule
as one that's always safe (503) — check whether the response proves the request was actually
received before assuming a retry can't double-submit.
16 changes: 16 additions & 0 deletions .cursor/bugbot-package-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,19 @@ Document how functions fail, especially for streaming, long-running, and resourc

Intentional limitations must be tracked with JIRA issues. However in package documentation or in the codebase,
do NOT list issues by number. Just describe the limitation and the intended future work to address it.

## A New API Wrapper Extends drapi, Never Re-Implements It

A package adding typed request/response shapes or a caller-specific error envelope over a
DataRobot API surface (see `internal/drapi/filesapi`, `internal/workload/apiclient`) must build
entirely on `drapi`'s verb functions, `EndpointURL`, and `HTTPError`. If the new package
constructs its own `http.Client`, re-derives a timeout default, or defines a second error type
shaped like `HTTPError`, that's a sign it's duplicating `drapi` rather than extending it.

## Non-DataRobot-API Calls Do Not Belong in drapi

A request that isn't to the authenticated DataRobot platform API — a local OAuth-redirect
handshake, a plugin download, a third-party registry fetch — must not be routed through
`drapi`'s verb functions or forced through `AuthorizeRequest`/`URLMatchesConfiguredBase`. Those
exist to protect a bearer token that has no business being attached to an unrelated origin; a
plain, purpose-built client for that call is correct, not a gap.
5 changes: 5 additions & 0 deletions .cursor/bugbot-resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,8 @@ Rollback and restore operations must be idempotent.
## File Permissions & Attributes Preserved

File modes, symlinks, and extended attributes must be preserved through backup and restore cycles.

## Duplicated Constants Need a Canonical-Source Comment

When an import cycle forces a second copy of a shared constant (timeout, retry count), comment
it as a duplicate pointing at the canonical definition — so the two don't drift silently.
16 changes: 16 additions & 0 deletions .cursor/bugbot-security.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,22 @@ Write tests that attempt to override security constraints and verify they fail.

Validate input at every package boundary — don't assume upstream packages provide safe data.

Worked example: `docs/development/drapi-client.md` — before attaching a bearer token to
a server-supplied URL (pagination cursor, live endpoint), verify it with
`drapi.URLMatchesConfiguredBase()` first.
Comment on lines +15 to +17

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I need or want worked examples in the bugbot cursor rules.


## Local Callback Listeners Must Validate Request Provenance

A listener bound to localhost for an OAuth-style handoff is reachable by any open web page —
localhost has no same-origin restriction. It must check something a page can't forge
(`Sec-Fetch-Dest`, a nonce) before treating a request as authentic, not just accept whatever
hits the port.

## Security Mitigation Claims Must Name the Residual Bypass

A doc or comment describing a security fix must scope its claim to the exact vectors it blocks
and name what still gets through — not claim a blanket guarantee the fix doesn't provide.

## Validate Integration Points Explicitly

Verify inter-package contracts with integration tests, not just assumptions.
Expand Down
5 changes: 5 additions & 0 deletions .cursor/bugbot-validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,8 @@ Validation tests must cover both success and failure paths.
## Validation Error Messages

Validation errors must name the field that failed and explain why.

## Heuristic Detection Must Degrade, Never Refuse

A check built on heuristics (missing project markers, ambiguous environment) must warn or
prompt, never hard-block — heuristics have false positives a hard refusal can't recover from.
8 changes: 8 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,14 @@ For full details, see [docs/development/configuration.md](docs/development/confi
- **To make a key persistable**, add it to `config.PersistableKeys` and have the
write site call `config.UpdateConfigFile("my-key")`.

## Auth & API Client Conventions

For OAuth/browser login internals, see [docs/development/authentication.md](docs/development/authentication.md).
For wiring in a new authenticated DataRobot API call—timeout clamping,
the `HTTPError`/`errors.As` contract, and the origin-safety check before
attaching credentials to a server-supplied URL—see
[docs/development/drapi-client.md](docs/development/drapi-client.md).

## Code Review Guidelines

All PRs are reviewed against **bugbot rules** in [.cursor/BUGBOT.md](.cursor/BUGBOT.md). Rules are organized by risk level:
Expand Down
1 change: 1 addition & 0 deletions docs/development/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ If you're new to developing the CLI, start here:
- **[Tool registry](tool-registry.md)**—how install-hint strategies and manager detection work; how to add a new tool or package manager.
- **[Plugins](plugins.md)**—develop and test CLI plugins, understand the plugin system architecture.
- **[Remote plugins](remote-plugins.md)**—create and distribute remote plugins, plugin registry management.
- **[drapi client conventions](drapi-client.md)**—how to wire in a new authenticated API call: timeout clamping, the `HTTPError`/`errors.As` contract, and origin-safety checks before attaching credentials to a server-supplied URL.
- **[Workload state directory validation](workload-wapi-validation.md)**—reference for `validator/v10` tags and cross-field rules for workload local state.
- **[Editing user-owned files](editing-user-files.md)**—rules for in-place edits to `.datarobot.yaml` and `.env`: comment preservation, round-trip symmetry, and when a refusal beats a rewrite.
- **[Releasing](releasing.md)**—release process, versioning strategy, and GoReleaser configuration.
Expand Down
4 changes: 4 additions & 0 deletions docs/development/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,7 @@ at `0644` by an older CLI would otherwise stay world-readable forever.
When attaching credentials to an absolute URL returned by a server response,
first check it with `drapi.URLMatchesConfiguredBase()`. URLs built locally with
`config.GetEndpointURL()` already stay on the configured DataRobot host.

For how the bearer token is attached to outgoing requests once you have
it—and the timeout/error-handling conventions for any new authenticated
API call—see [drapi client conventions](drapi-client.md).
74 changes: 74 additions & 0 deletions docs/development/drapi-client.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# drapi client conventions

`internal/drapi` is the one shared HTTP client every authenticated call to
the DataRobot API is expected to build on. This page is the dev-doc mirror
of `internal/drapi/doc.go` (`go doc ./internal/drapi` for the canonical
source) — read this first when wiring in a new API call; read the package
doc when you need the full detail.

## Always build on the verb functions

Use `drapi.Get`, `drapi.Post`, `drapi.Patch`, or `drapi.Delete` (or their
`*JSON` variants) with a URL from `drapi.EndpointURL()` /
`config.GetEndpointURL()`. Never construct your own `http.Client` or set
the `Authorization` header by hand — `AuthorizeRequest` (`internal/drapi/auth.go`)
is called internally by every verb function and sets the bearer token,
`User-Agent`, and the optional API-consumer trace header consistently.

If you must build your own `*http.Request` (multipart uploads, custom error
decoding), use `drapi.Do(req, timeout)` rather than duplicating
`NewHTTPClient(t).Do(req)` — it applies the same timeout clamp described
below.

## Timeouts

Every verb function takes an optional trailing `time.Duration`. Omit it, or
pass `<=0`, and `NewHTTPClient` clamps it to `DefaultClientTimeout` (30s)
internally — the clamp cannot be bypassed by a caller. This was tightened
specifically so a stray `0` or negative duration could never produce
an unbounded request.

## The `HTTPError` / `errors.As` contract

A non-2xx response becomes a `*drapi.HTTPError{StatusCode, URL, Detail, Body}`,
unpackable via `errors.As`. Roughly 32 call sites across the codebase depend
on this contract holding — treat it as load-bearing when touching
`internal/drapi`.

`Body` is the raw, uninterpreted response body (different DataRobot APIs
disagree on their error envelope shape); `Detail` is best-effort and
populated by the caller — see `internal/workload/apiclient.LiftDetail` for
the pattern of parsing a FastAPI `{"detail": ...}` envelope into `Detail`.

**Known gap, don't copy this as the pattern**: `Get()` currently builds a
bare `&HTTPError{StatusCode, URL}` on failure instead of calling
`ErrFromResp()` the way `Post`, `Patch`, and `Delete` do — so a `GET`
failure carries no `Body`/`Detail` today, unlike every other verb. This is
a known asymmetry, not an intentional design choice; a fix is a reasonable
follow-up, but a new caller should not assume `Get()` failures carry the
same diagnostic detail as the others until it's closed.

## Origin safety

Before attaching the bearer token to any URL your code did not build
locally — a server-returned pagination cursor, a workload's live `endpoint`
URL — call `drapi.URLMatchesConfiguredBase(rawURL)` first and skip
authorization when it doesn't match. Attaching a CLI credential to an
arbitrary server-supplied URL would leak it to whatever origin that URL
points at.

Two worked examples:

- `AssertNextOnSameHost` (`internal/drapi/pagination.go`) guards a paginated
list response's `Next` cursor before the client follows it.
- `endpointCheckAuthForURL` (`internal/workload/up/endpoint_check.go`)
authenticates a post-deploy endpoint check only when the URL is the
DataRobot-hosted workload gateway path *and* matches the configured base;
a direct/customer-container URL always stays anonymous, so the CLI token
never lands in a customer container's access log (RAPTOR-19741).

## See also

- [Authentication](authentication.md) — the OAuth-style login flow that
produces the token these conventions consume.
- `go doc ./internal/drapi` — the full package doc.
53 changes: 41 additions & 12 deletions docs/development/structure.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ cli/
│ ├── self/ # Self-management commands
│ ├── start/ # Application startup
│ ├── task/ # Task commands
│ └── templates/ # Template management
│ ├── templates/ # Template management
│ └── workload/ # Workload lifecycle commands (feature-gated)
├── internal/ # Private application code
│ ├── assets/ # Embedded assets
│ ├── auth/ # Authentication logic (OAuth-style browser flow)
│ ├── cli/ # CLI infrastructure (GatedCommand, etc.)
│ ├── config/ # Configuration management
│ ├── copier/ # Template copying utilities
Expand All @@ -30,7 +32,8 @@ cli/
│ ├── task/ # Task discovery and execution
│ ├── telemetry/ # Anonymous usage analytics (Amplitude)
│ ├── tools/ # Tool prerequisites
│ └── version/ # Version information
│ ├── version/ # Version information
│ └── workload/ # Workload manifest, wizard, deploy, and sync engine
├── tui/ # Terminal UI components
│ ├── banner.go # Banner display
│ ├── interrupt.go # Interrupt handling
Expand Down Expand Up @@ -63,8 +66,9 @@ Contains all CLI command implementations using the Cobra framework. Each subdire
#### Example

- `cmd/auth/cmd.go`: The auth command group
- `cmd/auth/login.go`: The login subcommand
- `cmd/auth/logout.go`: The logout subcommand
- `cmd/auth/login/cmd.go`: The login subcommand
- `cmd/auth/logout/cmd.go`: The logout subcommand
- ...

### internal/

Expand All @@ -88,7 +92,24 @@ DataRobot API client implementation for:

See the package doc (`go doc ./internal/drapi`) for the timeout-override
convention and the `HTTPError`/`errors.As` contract new commands should
follow when wiring in a call.
follow when wiring in a call, or [drapi client conventions](drapi-client.md)
for the same content as a dev doc.

#### auth/

OAuth-style browser login flow (`auth.EnsureAuthenticated`, used as a
`PreRunE` gate on any command that needs credentials) plus the
credential-verification and interactive URL-picker logic behind
`dr auth login`/`set-url`. See
[Authentication](authentication.md) for the full flow.

#### workload/

Manifest parsing/compilation, the `dr workload config` setup wizard, the
`dr workload up` deploy orchestrator, and the code-sync engine behind
`dr artifact code sync`. A dedicated architecture guide is in review with
the Workload API team; until it lands, `internal/workload/*/doc.go` (e.g.
`go doc ./internal/workload/up`) is the best available reference.

#### envbuilder/

Expand Down Expand Up @@ -230,20 +251,28 @@ Configuration is managed through Viper and stored in:

- `~/.config/datarobot/drconfig.yaml`: Global configuration and authentication tokens

Access configuration through the `internal/config` package:
Access configuration through `internal/config` and its `viperx` wrapper
(the only supported way to read/write viper state — `spf13/viper` cannot be
imported directly outside `internal/config`):

```go
import "github.com/datarobot/cli/internal/config"
import (
"github.com/datarobot/cli/internal/config"
"github.com/datarobot/cli/internal/config/viperx"
)

// Get configuration values
apiKey := config.GetAPIKey()
endpoint := config.GetEndpoint()
apiKey := viperx.GetString(config.DataRobotAPIKey)
endpoint := config.GetBaseURL()

// Set configuration values
config.SetAPIKey("new-key")
config.SaveConfig()
// Persist a value — only keys listed in config.PersistableKeys are written
viperx.Set(config.DataRobotURL, "https://app.datarobot.com")
config.UpdateConfigFile(config.DataRobotURL)
```

See [Configuration](configuration.md) for the full contract (persistable
keys, env-var binding, redaction rules).

## Testing structure

Tests are colocated with the code they test:
Expand Down
Loading