diff --git a/CLAUDE.md b/CLAUDE.md index af707b045..771e74d5b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -507,10 +507,10 @@ kbagent project create --url URL [--project ALIAS] [--name NAME] [--backend snow # delivered and may have succeeded (retryable:false, message says not to repeat -- check # `auth status`, whose stored session + claim link is the evidence). `--sync-backend-init` # raises that call's read timeout to 300s, which can outlast an agent's foreground shell. -kbagent project add --project NAME --url URL --token TOKEN +kbagent project add --project NAME --url URL [--token-stdin | --token-file PATH [--keep-token-file] | --token-env NAME | --token TOKEN] kbagent project list kbagent project remove --project NAME -kbagent project edit --project NAME [--url URL] [--token TOKEN] [--new-alias NEW] +kbagent project edit --project NAME [--url URL] [--token-stdin | --token-file PATH [--keep-token-file] | --token-env NAME | --token TOKEN] [--new-alias NEW] kbagent project status [--project NAME] kbagent project refresh --project ALIAS [--dry-run] [--force] [--yes] [--token-description DESC] [--token-expires-in N] kbagent project refresh --all [--dry-run] [--force] [--yes] [--token-description DESC] [--token-expires-in N] diff --git a/README.md b/README.md index 1c7370eff..f8cb1fff1 100644 --- a/README.md +++ b/README.md @@ -154,9 +154,9 @@ with Client(url=os.environ["KBC_URL"], token=os.environ["KBC_TOKEN"]) as kbc: ![30-second demo](docs/assets/demo-readme-main.gif) ```bash -# Connect a project (Storage API token from Keboola UI) +# Connect a project (Storage API token from Keboola UI, typed at a hidden prompt) kbagent project add --project prod \ - --url https://connection.keboola.com --token YOUR_TOKEN + --url https://connection.keboola.com --token-stdin # Find anything (table / config / flow / data app) across ALL projects in one call kbagent search "customer_id" @@ -205,9 +205,12 @@ Four ways to register projects, depending on what you have. If you are a human at a terminal with a browser, start with **browser login** (last one below); the token-based options are the ones to use for CI and anything unattended. -**Single project** — you have a Storage API token from the UI: +**Single project** — you have a Storage API token from the UI. `--token-stdin` shows a hidden prompt on a terminal and reads a pipe otherwise, so the token stays out of your shell history: ```bash -kbagent project add --project prod --url https://connection.keboola.com --token YOUR_TOKEN +kbagent project add --project prod --url https://connection.keboola.com --token-stdin + +# CI / non-interactive: pipe the token in +printf '%s' "$KBC_TOKEN" | kbagent project add --project prod --url https://connection.keboola.com --token-stdin ``` **Many projects by ID** — you have a Manage API or Personal Access Token + the project IDs: diff --git a/docs/TUTORIAL.md b/docs/TUTORIAL.md index 77e98c83f..6f8307dc2 100644 --- a/docs/TUTORIAL.md +++ b/docs/TUTORIAL.md @@ -41,14 +41,25 @@ Nothing else is needed. ```bash kbagent project add --project prod \ --url https://connection.keboola.com \ - --token YOUR_STORAGE_API_TOKEN + --token-stdin ``` +`--token-stdin` shows a hidden prompt on a terminal: paste the token and press +Enter. In a script, pipe the token in instead: + +```bash +printf '%s' "$KBC_TOKEN" | kbagent project add --project prod \ + --url https://connection.keboola.com --token-stdin +``` + +Do not pass the token as `--token VALUE`. It works, but it writes the token +into your shell history and into a process listing. + Aliases are arbitrary; pick names that make sense in your head (`prod`, `dev`, `client-a`, `my-project`). The CLI uses them on every subsequent command (`--project prod`). -Alternative ways to pass the token (same effect, safer for shell history): +Other ways to pass the token (also safe for shell history): ```bash # Env var @@ -824,10 +835,10 @@ The simplest path: a public git repo, no auth gate, three commands from zero to a running container. ```bash -# Register the project once (Storage API token from the UI). +# Register the project once (Storage API token from the UI, typed at a hidden prompt). kbagent project add --project prod \ --url https://connection.keboola.com \ - --token YOUR_STORAGE_TOKEN + --token-stdin # Create the deployment shell + Storage config in one shot. kbagent --json data-app create \ diff --git a/docs/auth.md b/docs/auth.md index 87e4ef92f..58bd36968 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -20,7 +20,7 @@ token (`kbc_rt_*`) that kbagent renews for you (since v0.80.0). > hand the plain command to the user's own terminal instead. > > For CI, containers, cron, or any other unattended context, you have two -> options: a **static Storage token** (`kbagent project add --token ...`, or +> options: a **static Storage token** (`kbagent project add --token-env NAME`, or > the token-only `KBAGENT_PROJECT_FROM_ENV=1` + `KBC_TOKEN` + > `KBC_STORAGE_API_URL` path -- unaffected by anything on this page), or > **`kbagent auth login-password`** (since v0.84.0) if you specifically need a @@ -433,9 +433,15 @@ somewhere sessions do not reach — a scheduled agent, a CI step, the importable SDK. Give that project a static token explicitly: ```bash -kbagent project edit --project my-project-9840 --token YOUR_STATIC_TOKEN +kbagent project edit --project my-project-9840 --token-stdin ``` +`--token-stdin` shows a hidden prompt on a terminal, and reads a pipe +otherwise, so the token never reaches your shell history. The other ways in +are `--token-file PATH` (kbagent deletes the file once the edit succeeds) and +`--token-env NAME` for CI. `--token VALUE` still works, but it puts the token +in your shell history and in a process listing. + This is allowed and it warns while doing it: the project becomes a static-token project, so `kbagent auth logout --remove-projects` will no longer clean it up — remove it with `kbagent project remove` when you are done. diff --git a/docs/demos/demo-add-project.tape b/docs/demos/demo-add-project.tape index cddebee1b..2e70cd8f8 100644 --- a/docs/demos/demo-add-project.tape +++ b/docs/demos/demo-add-project.tape @@ -41,7 +41,7 @@ Enter Sleep 200ms Type "# kbagent project add --project prod \\" Enter -Type "# --url https://connection.keboola.com --token " +Type "# --url https://connection.keboola.com --token-stdin" Enter Sleep 200ms Type "# After it completes, the project appears in the list:" diff --git a/docs/use-cases.md b/docs/use-cases.md index 5d42eb548..77aad62b7 100644 --- a/docs/use-cases.md +++ b/docs/use-cases.md @@ -352,7 +352,7 @@ uv pip install keboola-cli # Add your first project kbagent project add --project analytics \ --url https://connection.keboola.com \ - --token YOUR_STORAGE_TOKEN + --token-stdin # The name you give here is used in all other commands: --project analytics # If you omit --project, read commands query ALL connected projects. diff --git a/docs/web-server.md b/docs/web-server.md index 9c3e83bf7..bff5bd497 100644 --- a/docs/web-server.md +++ b/docs/web-server.md @@ -463,7 +463,7 @@ a session-backed project from the web UI at all: schedule; it cannot be revoked from this CLI afterwards. For a project you would rather not expose this way, register it with a static -Storage token (`kbagent project add --token`) — that path has neither property. +Storage token (`kbagent project add --token-stdin`) — that path has neither property. ### `/auth/*` — three read/audit endpoints, three deliberate gaps diff --git a/install.sh b/install.sh index 74113c5a7..71d0ff306 100755 --- a/install.sh +++ b/install.sh @@ -263,7 +263,7 @@ printf ' %s(needs the plugin:%s %skbagent doctor%s %sprints the exact /plugin info "" printf ' %sOr do it by hand%s\n' "$BOLD" "$RESET" >&2 printf ' %skbagent project add%s --project myproject \\\n' "$BOLD" "$RESET" >&2 -printf ' --url https://connection.keboola.com --token YOUR_TOKEN %s# connect a project%s\n' "$DIM" "$RESET" >&2 +printf ' --url https://connection.keboola.com --token-stdin %s# connect a project%s\n' "$DIM" "$RESET" >&2 printf ' %skbagent --help%s %s# see everything you can do%s\n' "$BOLD" "$RESET" "$DIM" "$RESET" >&2 printf ' %skbagent doctor%s %s# verify your setup%s\n' "$BOLD" "$RESET" "$DIM" "$RESET" >&2 info "" diff --git a/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index 58984aea1..9c7853c35 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -75,7 +75,7 @@ a critical failure. `kbagent --json version` carries `kbagent.install_channel`, quote its `upgrade_command` (or `upgrade_hint` when that is empty) instead. `auth` needs **0.80.0+**; `login-password` needs **0.84.0+** -- else - refuse and point at a static Storage token (`project add --token`). + refuse and point at a static Storage token (`project add --token-stdin`). 7. **ALWAYS USE `--json`**. Every `kbagent` invocation MUST have `--json` as the first flag after `kbagent`. This makes output @@ -146,6 +146,7 @@ been retired, so its absence is NOT a promise (see §1 Rule 6). | Pre-flight a data-app repo, or inspect an existing app's repo | `kbagent data-app validate-repo --git-repo URL --type python-js` BEFORE `create` (BLOCKING/WARN/OK, <=5 GitHub API calls); `data-app git-repo` afterwards for clone-URL introspection | `config detail --component-id keboola.data-apps` then read `parameters.dataApp.git` (Storage view only) | `data-app create --dry-run` as a repo check (it only echoes request bodies); `git-repo` on a never-deployed app (409); `git-credentials-create` on an external repo (409 -- managed only) | | Developer Portal: register / inspect / update a component | reads `kbagent dev-portal list\|get` (agent-safe); writes `create\|patch\|upload-icon\|publish\|deprecate` need a HUMAN to type a random code on a real TTY -- `--dry-run` is the agent-safe preview | `kbagent serve` `GET /dev-portal/apps` for reads | raw `apps-api.keboola.com`; ANY write from a non-TTY/agent shell (no bypass; exits 6) | | Manage project members & invitations | `kbagent project invite --email E --role admin\|guest\|readOnly\|share`, `invite --from-csv FILE [--workers N]` for bulk, `member-list [--include-pending]`, `invitation-list`, `invitation-cancel`, `member-remove --yes` (**destructive**), `member-set-role`. See [member-workflow.md](../skills/kbagent/references/member-workflow.md) | `--invitation-id ID` when an email lookup is ambiguous | a per-row shell loop over `invite` (defeats the service's parallelism + idempotency); `member-remove` without `--yes` non-interactively (prompts and hangs); reading `config.json` to infer membership | +| Give a project a Storage token (register it, or replace the one it has) | `kbagent project add|edit --project P --token-stdin` -- hidden prompt on a TTY, reads a pipe otherwise. If YOU must run it, have the person write the token to a file and pass `--token-file PATH`: kbagent reads it and deletes it once the command succeeds. In CI, `--token-env NAME` | `--token VALUE`, which lands in the shell history, the REPL history file and a process listing -- kbagent warns when you do it | telling a person to type `--token `; `project edit` does NOT read `KBC_TOKEN` (it would repoint the alias at another project on a plain `--new-alias`) | | Rename a project alias | `kbagent project edit --project OLD --new-alias NEW [--dry-run]` -- cascades through `config.json` (`projects` + `default_project`) and the nested sync directory. **Any lineage cache is NOT updated**: re-run `kbagent lineage build` after | `project remove` + `project add` (re-enters the token; loses the nested sync workspace) | hand-editing `config.json` (no validation, easy to miss the `default_project` cascade) | | Run kbagent headless from a daemon / container / CI with only a token in env | export `KBAGENT_PROJECT_FROM_ENV=1` + `KBC_TOKEN` + `KBC_STORAGE_API_URL`, then use `--project __env__` -- in-memory project, token NEVER persisted; same env also powers `kbagent serve` | a one-shot `project add` (works, but writes the token to `config.json`) | hand-writing a `config.json` with the token; passing `--token` per command (no such passthrough) | | Call the running `kbagent serve` from a scheduled-agent subprocess | `kbagent http get/post/patch/delete ` -- uses the `KBAGENT_SERVE_URL` + `KBAGENT_SERVE_TOKEN` the scheduler injects; `http get /openapi.json` discovers endpoints | forking `kbagent ` (also fine -- `KBAGENT_CONFIG_DIR` is propagated) | `curl $KBAGENT_SERVE_URL/...` by hand (loses the auth header, error mapping and JSON formatting `kbagent http` adds) | @@ -155,7 +156,7 @@ been retired, so its absence is NOT a promise (see §1 Rule 6). | ANY semantic-layer write (add / edit / remove / import / promote / build / scope add|remove|set|request-create) | `kbagent semantic-layer export` FIRST (the metastore has no soft-delete and no version history -- the snapshot is the only restore path), then the write, `--dry-run` where offered. Needs a token whose Storage Admin role is `admin` on the project (a master token qualifies, but so does any project-admin user token as of PSGO-282) -- a non-admin token still gets `MISSING_MASTER_TOKEN`/403 on writes, just not on reads. `Read` [semantic-layer-workflow.md](../skills/kbagent/references/semantic-layer-workflow.md) before starting: it carries the per-verb recipes, the rename cascade and the promote classification | `semantic-layer diff` (`--project-a/-b` or `--file-a/-b`) to confirm what a write would change | raw metastore REST (no rollback, no orphan scan, no modelUUID rewrite); a write with no export taken | | Share a semantic-layer object across projects, or make it org-wide | **ASK THE USER which project(s) before EVER passing `--scope organization\|targeted` (on `model create`/`add `) or running `scope set --scope organization`/`scope request-create` -- never guess or default to widening visibility.** Once the user names the target(s): `--scope targeted --target-project ALIAS\|ID` grants specific projects; `scope add\|remove --project P --type T --context-id ID --target-project ALIAS\|ID` adjusts an existing item's grants from the owning project (a merge), `scope set --target-project ...` / `--clear` replaces the list; org-wide needs `scope request-create` then an org-admin's `scope set --scope organization --yes` (preview with `--dry-run`) -- **irreversible, no downgrade endpoint, destructive-class**. `add ` without `--scope` inherits its model's scope. `scope get --type T --context-id ID` shows the current state first. See [metastore-scope-workflow.md](../skills/kbagent/references/metastore-scope-workflow.md) | `scope request-list --type T` to find items an org-admin should act on | passing `--scope organization\|targeted` or running `scope set --scope organization` without the user having named the target project(s) first; assuming `--scope organization` succeeds on a non-admin token (it 403s -- creating directly at organization scope needs the SAME organization-admin role as elevating); forgetting that `import`/`promote`/`build --model` into a shared model also share every new item (they take the model's scope unless `import`/`promote` get `--scope`); trying to elevate an item created before 0.97.0 (schema version 1.0.0: recreate it via export, delete, create, import) | | User asks to "log in" / authenticate via browser / register a session's projects | ATTENDED session + a background shell: run `kbagent auth login --device-code --stack URL --register-projects` in a **BACKGROUND** shell (capture stdout+stderr, human mode -- `--json` puts the panel on stderr), relay the verification URL + user code to the user, then poll `kbagent --json auth status` (exit 0 = signed in, exit 3 = not yet). The human's part is approving in the browser, not typing the command. No background shell -> hand the plain `auth login --stack URL --register-projects` to the user's terminal. To register projects from an EXISTING session, `kbagent auth register-projects --all` or `--project-id ID` is non-interactive and agent-safe | -- | running `auth login` in a FOREGROUND tool shell (~120 s timeout kills it mid-flight); running it unattended (nobody can approve); re-running it blind without checking `auth status` first (orphans a session); the flagless `register-projects` picker unattended; reading the token out of `auth.json`; using a numeric project id as an alias | -| User has NO Keboola account / project at all and asks to get started | `kbagent project create --url URL [--project ALIAS] [--name NAME] [--backend snowflake\|bigquery]` (0.95.0+) -- provisions a real project, stores its session and registers the alias in one call, agent-runnable. **Then relay the result's `confirm_url` to the human verbatim and say the project is owned by nobody until they open it**; after they confirm, the agent session is revoked by design -> `kbagent auth login --stack URL` | the user creating the project in the Keboola UI, then `auth login` / `project add --token` | calling it on a stack where a session already exists (exit 5 -- one session per stack); reporting success without the confirm link (an unclaimed project is a billable orphan); retrying it after a 5xx/429/503 without being asked (each success creates a new organization + project + credit grant) | +| User has NO Keboola account / project at all and asks to get started | `kbagent project create --url URL [--project ALIAS] [--name NAME] [--backend snowflake\|bigquery]` (0.95.0+) -- provisions a real project, stores its session and registers the alias in one call, agent-runnable. **Then relay the result's `confirm_url` to the human verbatim and say the project is owned by nobody until they open it**; after they confirm, the agent session is revoked by design -> `kbagent auth login --stack URL` | the user creating the project in the Keboola UI, then `auth login` / `project add --token-stdin` | calling it on a stack where a session already exists (exit 5 -- one session per stack); reporting success without the confirm link (an unclaimed project is a billable orphan); retrying it after a 5xx/429/503 without being asked (each success creates a new organization + project + credit grant) | | CI task has account credentials | `kbagent auth login-password --email E (--password-stdin \| --password P) [--totp-secret SEED]` (0.84.0+), agent-runnable | a static Storage token | `auth login` unattended | If the table does not cover the user's task, **ask clarifying diff --git a/plugins/kbagent/skills/kbagent-promotion-pipeline/references/env-injection.md b/plugins/kbagent/skills/kbagent-promotion-pipeline/references/env-injection.md index 1a986ef08..d0b5672d7 100644 --- a/plugins/kbagent/skills/kbagent-promotion-pipeline/references/env-injection.md +++ b/plugins/kbagent/skills/kbagent-promotion-pipeline/references/env-injection.md @@ -1,7 +1,7 @@ # Why every step uses `KBAGENT_PROJECT_FROM_ENV` / `__env__` kbagent's normal mode of operation is a **registered project**: `kbagent -project add --project ALIAS --url URL --token TOKEN` writes the token into +project add --project ALIAS --url URL --token-stdin` writes the token into `~/.config/keboola-agent-cli/config.json`, and every later command references that alias. That's the right model for a developer's own machine, but wrong for CI: it means a token would have to be written to disk (or the config diff --git a/plugins/kbagent/skills/kbagent/SKILL.md b/plugins/kbagent/skills/kbagent/SKILL.md index 8bef3c6aa..276895011 100644 --- a/plugins/kbagent/skills/kbagent/SKILL.md +++ b/plugins/kbagent/skills/kbagent/SKILL.md @@ -565,8 +565,9 @@ KBC_LOGIN_EMAIL=... KBC_LOGIN_PASSWORD=... KBC_LOGIN_TOTP_SECRET=... \ credentials: ```bash -# Single project -kbagent --json project add --project prod --url https://connection.keboola.com --token YOUR_TOKEN +# Single project. The token goes in on stdin, never as a --token value (that lands in the +# shell history). The person runs it and types the token at the hidden prompt; a script pipes it in: +printf '%s' "$KBC_TOKEN" | kbagent --json project add --project prod --url https://connection.keboola.com --token-stdin # Or bulk-onboard from organization (org admin) # Manage token: interactive prompt by default; for CI add --allow-env-manage-token diff --git a/plugins/kbagent/skills/kbagent/references/auth-workflow.md b/plugins/kbagent/skills/kbagent/references/auth-workflow.md index 0a3622dd0..440024e6e 100644 --- a/plugins/kbagent/skills/kbagent/references/auth-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/auth-workflow.md @@ -76,7 +76,7 @@ For CI, containers, or any other unattended context there are two options (since v0.84.0): if the task has account email + password (+ a TOTP seed for MFA), use `kbagent auth login-password` -- see "Unattended login" below, an agent MAY run it directly. Otherwise keep using a static Storage token -(`kbagent project add --token ...` or `KBAGENT_PROJECT_FROM_ENV`) -- that +(`kbagent project add --token-stdin` or `KBAGENT_PROJECT_FROM_ENV`) -- that path is unchanged by either feature. ## No Keboola account at all: `project create` @@ -362,7 +362,7 @@ If your workflow needs one of those, register the same project again under a different alias with a static Storage token: ``` -kbagent project add --project my-project-static --url --token +kbagent project add --project my-project-static --url --token-stdin ``` The two aliases can coexist; only the sentinel-token one is guarded. @@ -438,7 +438,7 @@ differently on purpose: access token rotates on its own -- so the project is reported under `skipped` with that reason rather than raising. `--force` does not convert it either. -- **`project edit --token` converts it, with a warning.** This is the +- **`project edit --token-stdin` converts it, with a warning.** This is the supported deliberate conversion. Once done the alias is a static-token project, so `auth logout --remove-projects` no longer cleans it up -- use `kbagent project remove`. The warning is identical under `--dry-run`, and diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 880753173..6eb9745f9 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -75,12 +75,12 @@ end-to-end walkthrough and troubleshooting. ## Project Management - `project create --url URL [--project ALIAS] [--name NAME] [--backend snowflake|bigquery] [--sync-backend-init]` -- create a **brand-new** Keboola project from a machine with no Keboola identity: no account, no token, no `auth login` first. The only kbagent command that works from nothing. In one call it provisions the project, stores the returned project-pinned session in `auth.json`, and registers the project in `config.json` under the `kbc-session://` sentinel -- becoming the default project when nothing else was registered, so the next command needs no `--project`. **The project it creates is owned by nobody**: the result's `confirm_url` is a single-use, days-limited link a human must open and sign in at to take ownership -- relay it verbatim, an unclaimed project is a billable orphan. `auth status` re-prints it as `agent_confirm_url` while it is pending. Confirming **revokes this session**, so the next step is always `kbagent auth login --stack URL`; the alias survives it, because the sentinel keys on project id + stack and not on the session. Requires the `agent-provisioning` stack feature (`STACK_FEATURES__AGENT_PROVISIONING`), which is off on most stacks -- the command is always registered, so seeing it in `--help` proves nothing; without the feature the stack 404s and the command exits 1 with `AUTH_NOT_SUPPORTED_ON_STACK`, naming the flag and how to connect an existing project instead. Refuses with exit 5 when a session for that stack already exists (one session per stack; whoever has one has an account and should use the Keboola UI). `--backend` omitted keeps the stack maintainer's default; `--sync-backend-init` waits for backend init instead of the async default, which warns that the first Storage command may fail until it lands (since 0.95.0) -- `project add --project NAME --url URL --token TOKEN` -- connect a project (token verified via API) +- `project add --project NAME --url URL [--token-stdin | --token-file PATH | --token-env NAME | --token TOKEN]` -- connect a project (token verified via API). Prefer `--token-stdin` (hidden prompt on a TTY, reads a pipe otherwise); `--token-file PATH` reads a file and deletes it once the command succeeds (`--keep-token-file` keeps it); `--token-env NAME` reads the variable you name, for CI. `--token VALUE` writes the token into the shell history, the REPL history file and a process listing, so it warns when used on a terminal. The sources are mutually exclusive (exit 2). `KBC_TOKEN` is still read implicitly here (since vNEXT) - `project list` -- list all connected projects (tokens masked). Carries `auth_mode` on every `--json` entry and an `Auth` column (before `Token`) in the human table: exactly `session` (browser login) or `static` (Storage token), **always present and never empty** -- including on a row synthesized for a project whose connection failed -- so branch on it instead of testing for absence. This is how you answer "which mode is this project in": `kbagent project list --json | jq '.data[].auth_mode'`. In the human table a session project's `Token` cell is a dash, because the stored sentinel is not a credential and masked (`kbc-...9840`) it reads like a truncated real token; the `--json` `token` value is unchanged (`mask_token(...)`, a stable contract) and the sentinel's body is the project id, which has its own column (since v0.80.0) - `project remove --project NAME` -- disconnect a project -- `project edit --project NAME [--url URL] [--token TOKEN] [--new-alias NEW] [--dry-run]` -- update connection details and/or rename the alias. `--new-alias` cascades through config.json (`projects` key + `default_project` if matched) and the nested sync directory `//` when present (-2 collision suffix, git-mv with shutil fallback). Lineage cache rebuild is manual (see gotchas, since v0.31.0). Combined with `--url` / `--token` in one call, those mutations target the new alias post-rename. `--dry-run` previews everything (collision check, planned disk-rename method, lineage-cache warning) without mutating state -- same exit codes as live for validation errors. `--token` on a browser-login (session) project is allowed and converts it to a static-token project, but warns that `auth logout --remove-projects` will no longer clean that alias up (`project remove` does); `--json` carries the text in an additive top-level `warnings` array, and `--dry-run` previews the identical warning (since v0.80.0) +- `project edit --project NAME [--url URL] [--token-stdin | --token-file PATH | --token-env NAME | --token TOKEN] [--new-alias NEW] [--dry-run]` -- update connection details and/or rename the alias. `--new-alias` cascades through config.json (`projects` key + `default_project` if matched) and the nested sync directory `//` when present (-2 collision suffix, git-mv with shutil fallback). Lineage cache rebuild is manual (see gotchas, since v0.31.0). Combined with `--url` / `--token` in one call, those mutations target the new alias post-rename. `--dry-run` previews everything (collision check, planned disk-rename method, lineage-cache warning) without mutating state -- same exit codes as live for validation errors. `--token` on a browser-login (session) project is allowed and converts it to a static-token project, but warns that `auth logout --remove-projects` will no longer clean that alias up (`project remove` does); `--json` carries the text in an additive top-level `warnings` array, and `--dry-run` previews the identical warning (since v0.80.0). The same four token sources as `project add`, with two deliberate differences: this command never prompts when no token is given (the token is optional here), and it does NOT read `KBC_TOKEN` -- a new token rewrites the alias's `project_id` and `project_name` from whatever it verifies as, so an exported variable could silently repoint the alias at another project during a plain `--new-alias`. `--token-file` is deleted only after the edit succeeds, never on `--dry-run` and never on a failure (since vNEXT) - `project status [--project NAME]` -- test connectivity and response time. Same guaranteed `auth_mode` field as `project list`, with the `Auth` column right after `Status` and present on ERROR rows too (since v0.80.0) -- `project refresh --project ALIAS | --all [--dry-run] [--force] [--yes] [--token-description DESC] [--token-expires-in N]` -- mint replacement Storage tokens via the Manage API for projects whose token is expired or invalid. `--force` also replaces still-valid non-expiring tokens. Browser-login (session) projects land in `skipped` with the reason that there is no static token to replace -- their credential lives in `auth.json` and rotates on its own -- and `--force` does not convert them either; use `project edit --token` for a deliberate single-project conversion (since v0.80.0). **Since 0.98.0 (#745)** a non-empty `projects_failed` exits **1** (was 0); `--dry-run` exits **1** on a failure too +- `project refresh --project ALIAS | --all [--dry-run] [--force] [--yes] [--token-description DESC] [--token-expires-in N]` -- mint replacement Storage tokens via the Manage API for projects whose token is expired or invalid. `--force` also replaces still-valid non-expiring tokens. Browser-login (session) projects land in `skipped` with the reason that there is no static token to replace -- their credential lives in `auth.json` and rotates on its own -- and `--force` does not convert them either; use `project edit --token-stdin` for a deliberate single-project conversion (since v0.80.0). **Since 0.98.0 (#745)** a non-empty `projects_failed` exits **1** (was 0); `--dry-run` exits **1** on a failure too - `project description-get --project NAME` -- read the dashboard project description (KBC.projectDescription on the default branch). Returns `{"description": ""}` if not set, not an error - `project description-set --project NAME [--text STR | --file PATH | --stdin]` -- set the dashboard project description (markdown). Pass exactly one of `--text`, `--file`, or `--stdin`. Writes to `KBC.projectDescription` on the default branch -- always the main branch, regardless of any active dev branch - `project use ALIAS` -- pin `ALIAS` as the persistent default project. Stored as `default_project` in config.json. A registered project ID pins that project's alias *(since 0.96.1)*. Overridden at runtime by `KBAGENT_PROJECT=ALIAS` (env, beats pin) and by `--project ALIAS` (CLI flag, beats both) @@ -223,7 +223,7 @@ Mint, revoke, and rotate scoped Storage API tokens (the Keboola single-bucket-wr - `token list --project NAME --with-last-used` -- adds `lastUsed`, `lastUsedEvent` and `lastUsedStatus` per token and sorts dormant-first, turning the listing into a revocation worklist. **Opt-in: one extra API call per token** (parallel fan-out), so do not reach for it when you only need an id. Storage tokens carry no `lastUsed` of their own, so it is derived from `GET /v2/storage/tokens/{id}/events` narrowed server-side to `q=token.id:{id}`. `lastUsedStatus` is `used` | `never` | `unknown` | `error` -- see gotchas before acting on it, `never` and `unknown` are NOT interchangeable and dev-branch activity is invisible. `--columns` (repeatable) selects and orders the human table (`id`, `description`, `created`, `refreshed`, `expires`, `master`, `created_by`, `last_used`, `last_used_event`); it does not affect `--json`. (since v0.88.0) - `token create --project NAME --description DESC [--bucket-write BUCKET ...] [--bucket-read BUCKET ...] [--component-access ID ...] [--can-read-all-file-uploads] [--expires-in N]` -- create a scoped token. `--bucket-write` / `--bucket-read` (both repeatable) grant per-bucket permissions; permissions are built read-first then write, so a bucket listed in both ends up writable. `--component-access` (repeatable) restricts the token to named components. `--expires-in N` sets a TTL in seconds. The secret is printed ONCE inside a Rich Panel. Requires a master token (pre-flight `MISSING_MASTER_TOKEN` otherwise, since v0.89.0). - `token delete --project NAME --token-id ID [--yes]` -- revoke a token by its numeric id (DELETE `/v2/storage/tokens/{id}` -> 204). Destructive; confirms via prompt unless `--yes` or `--json`. -- `token refresh --project NAME --token-id ID [--yes]` -- rotate a token's secret (POST `.../refresh`); the new secret is printed ONCE. Confirms unless `--yes` or `--json`. Needs `canManageTokens`, NOT a master token (unlike `create`). The new secret is not written back to config.json -- rotating the alias's own token leaves it holding a dead value until `project edit --project ALIAS --token `. +- `token refresh --project NAME --token-id ID [--yes]` -- rotate a token's secret (POST `.../refresh`); the new secret is printed ONCE. Confirms unless `--yes` or `--json`. Needs `canManageTokens`, NOT a master token (unlike `create`). The new secret is not written back to config.json -- rotating the alias's own token leaves it holding a dead value until `project edit --project ALIAS --token-stdin`. ## Data Lineage - `lineage build -d DIR -o FILE [--refresh] [--ai]` -- build column-level lineage graph from sync'd data diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index c9149ba70..902cb3031 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -192,7 +192,7 @@ Versioning convention: server-side). kbagent reclassifies exactly that 401 to `ErrorCode.MISSING_MASTER_TOKEN` with the remedy in the message. Fix: check `kbagent project info` -> `is_master_token`, and register a - project-admin token (`kbagent project edit --token ...`). Do NOT escalate + project-admin token (`kbagent project edit --token-stdin`). Do NOT escalate this one to support -- it is by design. - **(updated 0.97.0 -- PSGO-282, go-monorepo#596):** the "every call" part above is now history. The Metastore no longer requires a master @@ -390,7 +390,7 @@ Versioning convention: `flow new`/`update`/`validate`), and Storage bucket `sharing` (the Storage API enforces the master privilege). If a task needs one of the three static-only features, register the same project again with a static Storage token - (`project add --project --token ...`) instead of fighting the guard. + (`project add --project --token-stdin`) instead of fighting the guard. - **`dev-portal` is not on the list either.** It authenticates with its own Developer Portal identity (`dev-portal identity add`), never a project token, so a session project changes nothing there. And `flow` no longer splits: @@ -418,13 +418,32 @@ Versioning convention: credential lives in `auth.json` and its access token rotates on its own. The project appears under `skipped` with that reason instead of raising or being silently converted. A deliberate single-project conversion to a static token - is `project edit --token` (next bullet). -- **`project edit --token` on a session project is allowed, warns, and - converts.** The project becomes a static-token project, which means + is `project edit --token-stdin` (next bullet). +- **A new token on a session project is allowed, warns, and converts.** The project becomes a static-token project, which means `auth logout --remove-projects` will no longer clean that alias up -- use `project remove` when done with it. The warning text is identical in `--dry-run`; in `--json` it arrives in an additive top-level `warnings` array, so a consumer must not assume the key is absent. +- **`--token VALUE` puts a Storage token in the user's shell history; use + `--token-stdin` instead** (since vNEXT). `project add` and `project edit` + both take four mutually exclusive sources (a second one is exit 2): + `--token-stdin` (hidden prompt on a TTY, reads a pipe otherwise), + `--token-file PATH`, `--token-env NAME`, and `--token VALUE`. Never hand a + person a `--token VALUE` command. If they can run it themselves, give them + `--token-stdin`. If you must run it, ask them to write the token to a file + and pass `--token-file`. In CI use `--token-env`. `--token VALUE` warns on a + terminal that the token is now in the history. +- **`--token-file` deletes the file it read** (since vNEXT). Only after the + command succeeds -- never on `--dry-run`, never after a failure, so a bad + token does not cost the user the file. `--keep-token-file` turns the delete + off for a read-only mount. A delete that fails is a warning, not an error. + Note that this unlinks the file; it does not scrub the bytes off the disk. +- **`project edit` ignores `KBC_TOKEN`; `project add` reads it** (since + vNEXT). Not an oversight. A new token rewrites the alias's `project_id` and + `project_name` from whatever it verifies as, and nothing checks that the + token belongs to the same project, so an exported `KBC_TOKEN` could silently + repoint an alias at a different project during a plain `--new-alias`. Name + the variable with `--token-env KBC_TOKEN` if you do want it read there. - **`auth logout --remove-projects` needs the `admin` permission class, the bare `auth logout` only `write`** (since v0.80.0). The flag deletes `config.json` entries, the same observable effect as `project remove`, so a @@ -692,7 +711,7 @@ Versioning convention: #599) — exactly the shape `org setup` / `project refresh` mint. Since v0.89.0 `token create` pre-flights `isMasterToken` and fails fast with `MISSING_MASTER_TOKEN` (exit 3) naming the fix (`kbagent project edit - --project ALIAS --token `). + --project ALIAS --token-stdin`). - **That defect is CREATE-ONLY — `refresh` / `list` / `delete` are NOT master-guarded.** `RefreshTokenVoter` lets any token rotate **itself** and a `canManageTokens` token rotate another, so rotating a leaked device token @@ -704,7 +723,7 @@ Versioning convention: - **`token refresh` does not write the new secret back to `config.json`.** The value is printed once, exactly like `create`. Rotating the very token an alias uses therefore leaves that alias holding a dead value — follow it with - `kbagent project edit --project ALIAS --token ` or the next command + `kbagent project edit --project ALIAS --token-stdin` or the next command fails on auth. - **The secret is a ONE-TIME reveal.** `create` / `refresh` print the token value once (human mode: inside a Rich panel; `--json`: the `token` field). It is never @@ -2094,7 +2113,7 @@ config, the retry fires, and the retry destroys it for good. `lineage build`. - Combined invocations are atomic in the obvious order: `--new-alias` is applied first, then `--url` / `--token` mutations target the new alias key. - So `kbagent project edit --project foo --new-alias bar --token NEW` does + So `kbagent project edit --project foo --new-alias bar --token-stdin` does the rename, then writes the new token under `bar`. If `--new-alias` is identical to the current alias, it's a no-op (matches "rename to same name" idempotency). @@ -2987,7 +3006,7 @@ type inventory and examples. ## Token handling - Tokens are always masked in output (e.g. `901-...pt0k`) -- this is normal -- Token can be passed via `--token`, `KBC_TOKEN` env var, or interactive prompt +- Token can be passed via `--token-stdin` (preferred), `--token-file`, `--token-env` (all three since vNEXT), `--token`, `KBC_TOKEN` env var (`project add` only), or interactive prompt (`project add` only) - Manage API token (since v0.29.0): default-deny on env -- via interactive hidden prompt; pass top-level `--allow-env-manage-token` to opt in to `KBC_MANAGE_API_TOKEN`. Never as CLI argument. See the `(since v0.29.0)` entry at the top of this file. - Master token for sharing: `KBC_MASTER_TOKEN_{ALIAS}` (e.g. `KBC_MASTER_TOKEN_PROD`) or `KBC_MASTER_TOKEN` as global fallback. Alias is uppercased, hyphens become underscores. Required for `sharing share` and `sharing unshare`; `sharing list/link/unlink` use regular project tokens. @@ -3795,7 +3814,7 @@ requires a **master (admin) token** — `canManageTokens` alone is not enough guard the Storage API returns a vague 500 "Application error" that misleads operators into thinking the OAuth wizard is broken. - Fix path: re-add the project with a master token - (`kbagent project edit --project --token `) or open + (`kbagent project edit --project --token-stdin`) or open the OAuth flow via the Keboola UI instead. - AI agents creating the project token via `kbagent project add` / `kbagent project refresh` get a non-master token by default — they must diff --git a/src/keboola_agent_cli/auth/auth_client.py b/src/keboola_agent_cli/auth/auth_client.py index afdb6f42d..a5e813fc3 100644 --- a/src/keboola_agent_cli/auth/auth_client.py +++ b/src/keboola_agent_cli/auth/auth_client.py @@ -792,7 +792,7 @@ def _raise_provisioning_error(self, response: httpx.Response) -> NoReturn: f"project that already exists: create one at {self._base_url} in a " f"browser, then connect it with `kbagent auth login --stack " f"{self._base_url}` or `kbagent project add --project --url " - f"{self._base_url} --token `." + f"{self._base_url} --token-stdin`." ), status_code=404, error_code=ErrorCode.AUTH_NOT_SUPPORTED_ON_STACK, @@ -969,7 +969,7 @@ def _map_auth_error( message=( f"Browser login is not enabled on this Keboola stack yet " f"({self._base_url}). Use a static Storage token instead: " - "kbagent project add --project --url --token ." + "kbagent project add --project --url --token-stdin." ), status_code=404, error_code=ErrorCode.AUTH_NOT_SUPPORTED_ON_STACK, @@ -1042,7 +1042,7 @@ def _raise_password_login_error(self, response: httpx.Response, *, is_mfa: bool) message=( f"Programmatic auth is not enabled on this Keboola stack yet " f"({self._base_url}). Use a static Storage token instead: " - "kbagent project add --project --url --token ." + "kbagent project add --project --url --token-stdin." ), status_code=404, error_code=ErrorCode.AUTH_NOT_SUPPORTED_ON_STACK, diff --git a/src/keboola_agent_cli/commands/_helpers.py b/src/keboola_agent_cli/commands/_helpers.py index d8d79005d..511e6247c 100644 --- a/src/keboola_agent_cli/commands/_helpers.py +++ b/src/keboola_agent_cli/commands/_helpers.py @@ -21,6 +21,7 @@ from ..config_store import ConfigStore from ..constants import ( ENV_KBC_MANAGE_API_TOKEN, + ENV_KBC_TOKEN, EXIT_JOB_TIMEOUT_TERMINATED, EXIT_PERMISSION_DENIED, ) @@ -106,23 +107,288 @@ def parse_json_arg(raw: str, *, label: str) -> Any: raise ValueError(f"{label}: cannot read file: {exc}") from exc -def read_password_stdin() -> str: - """Read a password from stdin. +def read_password_stdin(label: str = "Password: ") -> str: + """Read a secret from stdin. TTY -> getpass (hidden, line-based, Enter to confirm). Pipe/redirected -> read to EOF, strip whitespace. Using `sys.stdin.read()` unconditionally would hang interactively until the user sent EOF (Ctrl-D); getpass on TTY fixes that. - Shared by `auth login-password --password-stdin` and `dev-portal - identity add/edit --password-stdin` -- the same input contract, so one - helper rather than a private copy per command module. + Shared by `auth login-password --password-stdin`, `dev-portal identity + add/edit --password-stdin` and `project add/edit --token-stdin` -- the + same input contract, so one helper rather than a private copy per + command module. ``label`` is the prompt shown on a TTY. """ if sys.stdin.isatty(): - return getpass.getpass("Password: ").strip() + return getpass.getpass(label).strip() return sys.stdin.read().strip() +def _read_token_file(path: Path) -> str: + """Read a Storage API token out of a file, and check the file's mode.""" + try: + token = path.read_text(encoding="utf-8").strip() + except OSError as exc: + raise typer.BadParameter( + f"Cannot read token file {path}: {exc}", + param_hint="--token-file", + ) from exc + if not token: + raise typer.BadParameter(f"Token file {path} is empty.", param_hint="--token-file") + # Windows reports fabricated POSIX bits (an ordinary file reads as ~0o666), + # so the mode check would warn on every use. Skip it there, as + # auth/state_store.py and services/doctor_service.py already do. + if os.name != "nt": + try: + mode = path.stat().st_mode & 0o777 + except OSError: # pragma: no cover - the read above already succeeded + mode = 0 + if mode & 0o077: + typer.echo( + f"Warning: {path} is accessible to other users (mode {mode:04o}). " + "A token file should be 0600.", + err=True, + ) + return token + + +def _require_single_line(token: str, param_hint: str) -> str: + """Refuse a token that holds a line break or another control character. + + A Storage token is one printable line. When the value is several lines -- + a whole file or ``.env`` piped in by mistake -- the HTTP layer fails with + an error that quotes the entire header value ("Illegal header value b'...'"), + and nothing catches it, so the traceback would print the token and + everything piped in with it. Refusing here keeps the value out of that + message: the error names the flag, never the value. An empty or blank + value is refused too, so an unset variable piped in does not reach the API. + """ + if not token.strip(): + raise typer.BadParameter("The token is empty.", param_hint=param_hint) + if not token.isprintable(): + raise typer.BadParameter( + "The token contains a line break or another control character. " + "Pass the token alone, on one line.", + param_hint=param_hint, + ) + return token + + +# Shared declarations for the token-input flags. `project add` and `project +# edit` offer the same set, and one definition keeps their help text from +# drifting apart -- `project.py` is already over its soft size ceiling. +TOKEN_STDIN_OPTION = typer.Option( + False, + "--token-stdin", + help="Read the token from stdin instead of --token. On a TTY this is a hidden " + "prompt (Enter to confirm); on a pipe it reads until EOF (e.g. " + "`printf '%s' \"$TOKEN\" | kbagent project edit --project P --token-stdin`). " + "The token stays out of your shell history and out of a process listing.", +) + +TOKEN_FILE_OPTION = typer.Option( + None, + "--token-file", + help="Read the token from this file. kbagent deletes the file after the command " + "succeeds -- pass --keep-token-file to keep it. Deleting a file removes the " + "directory entry, not the bytes on disk. Give the file mode 0600.", + exists=True, + readable=True, + dir_okay=False, +) + +TOKEN_ENV_OPTION = typer.Option( + None, + "--token-env", + help="Read the token from the environment variable with this name, e.g. " + "`--token-env CI_KBC_TOKEN`. kbagent reads no variable unless you name it. " + "Use this in CI, where the runner sets the variable from a secret.", +) + +KEEP_TOKEN_FILE_OPTION = typer.Option( + False, + "--keep-token-file", + help="Keep the --token-file file instead of deleting it. Use this for a " + "read-only mount, or for a file you read more than once.", +) + + +def token_came_from_command_line(ctx: typer.Context, param_name: str = "token") -> bool: + """Report whether a parameter's value was typed on the command line. + + `--token` and its `envvar=` reach the command in one parameter, and only + the typed one is in the shell history, so the warning has to tell them + apart. Click records the origin, but Typer bundles its own copy of Click + (`typer._click`), so the `ParameterSource` enum a running context returns + is a different class from `click.core.ParameterSource`. Comparing the two + with `==` is always False, which switches the warning off without a word. + Match on the member name, which is the same in both copies. + + An unknown origin counts as the command line: a missing warning defeats + the point of the flag, while a spare one costs the reader a line. + """ + source = ctx.get_parameter_source(param_name) + return source is None or source.name == "COMMANDLINE" + + +def resolve_storage_token_input( + *, + token: str | None, + token_stdin: bool = False, + token_file: Path | None = None, + token_env: str | None = None, + keep_token_file: bool = False, + required: bool, + token_from_cli: bool = True, +) -> str | None: + """Resolve a Storage API token from exactly one permitted source. + + The point of every source other than ``--token`` is that the value + never reaches the command line, where it lands in the shell history, + in the kbagent REPL history file, and in a process listing. + + Resolution order -- the four explicit sources are mutually exclusive: + + 1. ``--token-stdin`` -- hidden prompt on a TTY, read to EOF on a pipe. + 2. ``--token-file`` -- read the file. Call ``discard_token_file()`` + after the command succeeds to delete it. + 3. ``--token-env`` -- read the named environment variable. + 4. ``token`` -- the ``--token`` value. For ``project add`` this also + carries ``KBC_TOKEN``, because Typer merges the flag and the envvar + into one parameter. Such a value is implicit, so it does not count + as an explicit source and it gets no shell-history warning; the + caller reports the difference through ``token_from_cli``. + 5. Nothing given -- prompt on a TTY when ``required`` is True, and + exit 2 when there is no TTY. A caller whose token is optional + (``project edit``) passes ``required=False`` and gets ``None``. + + Args: + token: Value of ``--token``, or of the envvar the caller declares. + token_stdin: ``--token-stdin`` was passed. + token_file: Path from ``--token-file``. + token_env: Variable name from ``--token-env``. + keep_token_file: ``--keep-token-file`` was passed. Validated here + because it is meaningless without ``--token-file``; the flag + itself is acted on by ``discard_token_file()``. + required: The command cannot continue without a token. + token_from_cli: ``token`` came from the command line, not from the + environment. Keeps the shell-history warning truthful. + + Returns: + The token, or ``None`` when nothing was given and ``required`` is + False. + + Raises: + typer.BadParameter: More than one explicit source, an unreadable or + empty file, an unset or empty named variable, an empty or blank + value from any source (``--token ""`` included), a value with a + line break or another control character, or ``--keep-token-file`` + without ``--token-file``. + typer.Exit: ``required`` is True and there is nothing to read + (exit code 2). + """ + if keep_token_file and token_file is None: + raise typer.BadParameter( + "--keep-token-file applies only together with --token-file.", + param_hint="--keep-token-file", + ) + + given = [ + flag + for flag, present in ( + ("--token", token is not None and token_from_cli), + ("--token-stdin", token_stdin), + ("--token-file", token_file is not None), + ("--token-env", token_env is not None), + ) + if present + ] + if len(given) > 1: + raise typer.BadParameter( + f"Specify exactly one of {' / '.join(given)}.", + param_hint=given[0], + ) + + if token_stdin: + return _require_single_line( + read_password_stdin(label="Storage API token: "), "--token-stdin" + ) + + if token_file is not None: + return _require_single_line(_read_token_file(token_file), "--token-file") + + if token_env is not None: + value = os.environ.get(token_env, "") + if not value: + raise typer.BadParameter( + f"Environment variable {token_env} is unset or empty.", + param_hint="--token-env", + ) + return _require_single_line(value, "--token-env") + + if token is not None: + _require_single_line(token, "--token") + if token_from_cli and hasattr(sys.stdin, "isatty") and sys.stdin.isatty(): + typer.echo( + "Warning: the token you passed with --token is now in your shell " + "history. Delete that history entry, or invalidate the token. " + "Use --token-stdin to keep the next one out of the history.", + err=True, + ) + return token + + if not required: + return None + + if hasattr(sys.stdin, "isatty") and sys.stdin.isatty(): + return typer.prompt("Storage API token", hide_input=True) + + typer.echo( + "Error: No token available. Pass --token-stdin, --token-file, " + f"--token-env, or --token; set {ENV_KBC_TOKEN}; or run interactively.", + err=True, + ) + raise typer.Exit(code=2) + + +def discard_token_file(path: Path | None, *, keep: bool = False, dry_run: bool = False) -> None: + """Delete the ``--token-file`` after the command that read it succeeded. + + The file carries the token to kbagent; it is not a store. Deleting it + at read time would cost the user the file every time the token failed + verification, and deleting it during ``--dry-run`` would make a preview + destructive. So the caller invokes this only on the success path. + + A failed unlink is a warning, not an error. A read-only mount (a + Kubernetes secret, a CI volume) is a legitimate setup, and the command + has already done its work by this point. + + ``unlink`` removes the directory entry; it does not scrub the bytes. On + a copy-on-write filesystem or an SSD the content can survive. + """ + if path is None: + return + if dry_run: + typer.echo(f"Note: {path} was kept (--dry-run). It still holds the token.", err=True) + return + if keep: + typer.echo( + f"Warning: {path} still holds the token (--keep-token-file). Delete it yourself.", + err=True, + ) + return + try: + path.unlink() + except OSError as exc: + typer.echo( + f"Warning: could not delete {path} ({exc}). It still holds the token.", + err=True, + ) + return + typer.echo(f"Deleted the token file {path}.", err=True) + + def get_formatter(ctx: typer.Context) -> OutputFormatter: """Retrieve the OutputFormatter from the Typer context.""" return ctx.obj["formatter"] diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 17fbca623..1a8725106 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -40,8 +40,9 @@ ## Quick Start - # Add a single project - kbagent --json project add --project my-project --url https://connection.keboola.com --token YOUR_TOKEN + # Add a single project -- --token-stdin prompts (hidden) on a TTY and reads + # a pipe otherwise, so the token stays out of the shell history + kbagent --json project add --project my-project --url https://connection.keboola.com --token-stdin # Or bulk-onboard all projects from an organization KBC_MANAGE_API_TOKEN=xxx kbagent --allow-env-manage-token --json org setup --org-id 123 --url https://connection.keboola.com --yes @@ -86,7 +87,7 @@ through the CLI. `auth login-password` (below) is the unattended path: it is safe to run headlessly given the credentials it needs, and is what CI/automation should use to get a full session (as opposed to a - single-project static Storage token via `project add --token` / + single-project static Storage token via `project add --token-stdin` / `KBAGENT_PROJECT_FROM_ENV`). kbagent auth login [--stack URL|alias] [--device-code] [--register-projects] @@ -288,8 +289,32 @@ the stack finish it in the background (the async default warns that the first Storage command may fail until it lands). - kbagent project add --project NAME --url URL --token TOKEN + kbagent project add --project NAME --url URL [--token-stdin | --token-file PATH | --token-env NAME | --token TOKEN] Add a new project connection. Token verified against API. + See "Getting a Storage token in" below before you pick a flag. + + HOW TO GET A STORAGE TOKEN IN (applies to `project add` and `project edit`) + + Never put a Storage API token on the command line. The `--token ` + form writes the token into the user's shell history, into the kbagent + REPL history file, and into a process listing. + + Use this order: + + 1. If the person can run the command, give them the command with + `--token-stdin` and let them run it. The prompt hides the input. + 2. If you must run the command yourself, ask the person to write the + token into a file. Then pass `--token-file `. kbagent reads the + file and deletes it after the command succeeds. Pass + `--keep-token-file` only for a read-only mount. + 3. In CI, where the runner sets the variable, pass `--token-env `. + `project add` also still reads KBC_TOKEN on its own; `project edit` + does not. + 4. Use `--token ` only when nothing else works. Then tell the + person that the token is in their shell history, and that they must + delete that entry or invalidate the token. + + The four sources are mutually exclusive. Passing two is exit 2. kbagent project list List all connected projects (tokens always masked). An `Auth` column @@ -309,12 +334,18 @@ kbagent project remove --project NAME Remove a project connection. - kbagent project edit --project NAME [--url URL] [--token TOKEN] [--new-alias NEW] - Edit project connection. Re-verifies token if changed. --new-alias renames + kbagent project edit --project NAME [--url URL] [--token-stdin | --token-file PATH | --token-env NAME | --token TOKEN] [--new-alias NEW] + Edit project connection. Re-verifies token if changed. The token is + optional here, so with no token flag the command changes no credential + and never prompts. Unlike `project add`, this command ignores KBC_TOKEN: + a new token rewrites the alias's project_id and project_name from + whatever it verifies as, so an exported variable could silently repoint + the alias at another project during an unrelated edit such as + --new-alias. --new-alias renames the alias and cascades the rename through config.json and the nested sync directory at //. Lineage cache embeds the alias in FQNs and is NOT auto-updated; rebuild via `kbagent lineage build` after rename. - Passing --token for a browser-login (session) project is allowed and + Passing a new token for a browser-login (session) project is allowed and converts it to a static-token project, but it emits a warning: once converted, `auth logout --remove-projects` no longer cleans that alias up (use `project remove`). In --json mode the warning is carried in an @@ -331,7 +362,7 @@ Browser-login (session) projects are reported under "skipped" with the reason that there is no static token to replace -- their access token rotates on its own from auth.json. --force does NOT convert them either; - use `project edit --token` for a deliberate one-project conversion. + use `project edit --token-stdin` for a deliberate one-project conversion. kbagent project description-get --project NAME Read the Keboola dashboard project description (markdown). Backed by @@ -989,7 +1020,8 @@ Rotate a token's secret (new secret printed ONCE; confirms unless --yes / --json). Needs canManageTokens only -- NOT master-guarded (that API defect is create-only). The new secret is NOT written back to config.json: rotating the alias's own token leaves it dead - until `project edit --project ALIAS --token `. + until `project edit --project ALIAS --token-stdin` (paste the new secret at the hidden + prompt; see "HOW TO GET A STORAGE TOKEN IN"). Notes: uses the per-project Storage token (no manage token). `token create` requires a MASTER (admin) token; list/delete/refresh only need the canManageTokens privilege. The importable SDK (Client(url,token)) has NO pre-flight guard and mirrors these as @@ -2408,7 +2440,9 @@ KBAGENT_CONVERSATION_ID Conversation/session ID (REQUIRED -- sent as X-Conversation-ID header). Prefer the --conversation-id global flag from an agent harness: a standalone `export` does not survive between tool calls. - KBC_TOKEN Storage API token (fallback for --token) + KBC_TOKEN Storage API token, read by `project add` only. + `project edit` ignores it -- name it with + --token-env KBC_TOKEN if you want it there. KBC_STORAGE_API_URL Default stack URL (fallback for --url) KBC_MANAGE_API_TOKEN Manage API token (org setup, project refresh). Default-DENY since 0.29.0: pass --allow-env-manage-token diff --git a/src/keboola_agent_cli/commands/project.py b/src/keboola_agent_cli/commands/project.py index fd42a74aa..24aa89713 100644 --- a/src/keboola_agent_cli/commands/project.py +++ b/src/keboola_agent_cli/commands/project.py @@ -4,7 +4,6 @@ No business logic belongs here. """ -import sys from enum import StrEnum from pathlib import Path from typing import Any @@ -25,12 +24,19 @@ from ..errors import ConfigError, ErrorCode, KeboolaApiError from ..services.project_service import AUTH_MODE_SESSION, AUTH_MODE_STATIC from ._helpers import ( + KEEP_TOKEN_FILE_OPTION, + TOKEN_ENV_OPTION, + TOKEN_FILE_OPTION, + TOKEN_STDIN_OPTION, check_cli_permission, + discard_token_file, get_formatter, get_service, item_failure_exit_code, map_error_to_exit_code, resolve_manage_token, + resolve_storage_token_input, + token_came_from_command_line, ) from ._metadata_input import resolve_text_input from ._project_create import format_provision_result @@ -161,39 +167,6 @@ def _format_status_table(console: Console, statuses: list[dict[str, Any]]) -> No console.print(table) -def _resolve_token(token: str | None) -> str: - """Resolve the Storage API token, falling back to interactive prompt. - - Token resolution order (Typer handles steps 1-2 automatically via envvar): - 1. --token CLI argument - 2. KBC_TOKEN env var (handled by Typer's envvar parameter) - 3. Interactive prompt with hidden input (if TTY) - 4. Error if none available - - Args: - token: Token from --token or KBC_TOKEN env var (resolved by Typer), or None. - - Returns: - The Storage API token. - - Raises: - typer.Exit: If no token can be resolved. - """ - if token: - return token - - is_tty = hasattr(sys.stdin, "isatty") and sys.stdin.isatty() - if is_tty: - return typer.prompt("Storage API token", hide_input=True) - - typer.echo( - f"Error: No token available. Pass --token, set {ENV_KBC_TOKEN} env var, " - "or run interactively.", - err=True, - ) - raise typer.Exit(code=2) - - @project_app.command("add") def project_add( ctx: typer.Context, @@ -205,17 +178,34 @@ def project_add( ), token: str | None = typer.Option( None, - help="Storage API token (also via KBC_TOKEN env var)", + help="Storage API token. Also settable via KBC_TOKEN. Prefer --token-stdin: a " + "token passed here lands in your shell history and in a process listing.", envvar=ENV_KBC_TOKEN, ), + token_stdin: bool = TOKEN_STDIN_OPTION, + token_file: Path | None = TOKEN_FILE_OPTION, + token_env: str | None = TOKEN_ENV_OPTION, + keep_token_file: bool = KEEP_TOKEN_FILE_OPTION, ) -> None: """Add a new Keboola project connection. - Token is read from --token, KBC_TOKEN env var, or prompted interactively. + The token comes from --token-stdin, --token-file, --token-env, --token, + the KBC_TOKEN env var, or an interactive prompt. Every source other than + --token keeps the value out of your shell history. """ formatter = get_formatter(ctx) service = get_service(ctx, "project_service") - resolved_token = _resolve_token(token) + resolved_token = resolve_storage_token_input( + token=token, + token_stdin=token_stdin, + token_file=token_file, + token_env=token_env, + keep_token_file=keep_token_file, + required=True, + # `--token` and KBC_TOKEN arrive in one parameter; only a value typed + # on the command line is in the shell history. + token_from_cli=token_came_from_command_line(ctx), + ) try: result = service.add_project(alias=alias, stack_url=url, token=resolved_token) @@ -226,6 +216,7 @@ def project_add( f"added (project: {escape(d['project_name'])}, id: {d['project_id']})" ), ) + discard_token_file(token_file, keep=keep_token_file) except KeboolaApiError as exc: exit_code = map_error_to_exit_code(exc) formatter.error( @@ -345,9 +336,14 @@ def project_edit( "New Storage API token. On a project registered by 'kbagent auth " "login' this deliberately converts it to a static-token project, " "which 'kbagent auth logout --remove-projects' no longer cleans up; " - "a warning says so." + "a warning says so. Prefer --token-stdin: a token passed here lands in " + "your shell history and in a process listing." ), ), + token_stdin: bool = TOKEN_STDIN_OPTION, + token_file: Path | None = TOKEN_FILE_OPTION, + token_env: str | None = TOKEN_ENV_OPTION, + keep_token_file: bool = KEEP_TOKEN_FILE_OPTION, new_alias: str | None = typer.Option( None, "--new-alias", @@ -374,19 +370,37 @@ def project_edit( ) -> None: """Edit an existing Keboola project connection. - If --token is provided, the token is re-verified against the API. - Combined with --new-alias, the rename is applied first and any - --url / --token mutation lands on the new alias key. Pass --dry-run - to preview without mutating state. + A new token can come from --token-stdin, --token-file, --token-env or + --token; every source other than --token keeps the value out of your + shell history. The token is optional here, so with none of them the + command changes no credential and never prompts. + + Unlike `project add`, this command ignores KBC_TOKEN. A new token + rewrites the alias's project_id and project_name from whatever the + token verifies as, so an exported variable could silently repoint the + alias at another project during an unrelated edit such as --new-alias. + + If a token is provided, it is re-verified against the API. Combined + with --new-alias, the rename is applied first and any --url / --token + mutation lands on the new alias key. Pass --dry-run to preview without + mutating state. """ formatter = get_formatter(ctx) service = get_service(ctx, "project_service") + resolved_token = resolve_storage_token_input( + token=token, + token_stdin=token_stdin, + token_file=token_file, + token_env=token_env, + keep_token_file=keep_token_file, + required=False, + ) try: result = service.edit_project( alias=alias, stack_url=url, - token=token, + token=resolved_token, new_alias=new_alias, dry_run=dry_run, ) @@ -426,6 +440,7 @@ def _human(c: Console, d: dict) -> None: formatter.warning(warning) formatter.output(result, _human) + discard_token_file(token_file, keep=keep_token_file, dry_run=dry_run) except KeboolaApiError as exc: exit_code = map_error_to_exit_code(exc) formatter.error( diff --git a/src/keboola_agent_cli/commands/token.py b/src/keboola_agent_cli/commands/token.py index 95e90e08b..e2095be09 100644 --- a/src/keboola_agent_cli/commands/token.py +++ b/src/keboola_agent_cli/commands/token.py @@ -367,7 +367,7 @@ def token_refresh( Needs canManageTokens, NOT a master token -- unlike `token create` (the API defect behind that guard is create-only, issue #599). Note the new secret is printed but not stored: rotating the token this project alias itself uses - leaves the alias holding a dead value until you run `project edit --token`. + leaves the alias holding a dead value until you run `project edit --token-stdin`. """ formatter = get_formatter(ctx) if ( diff --git a/src/keboola_agent_cli/config_store.py b/src/keboola_agent_cli/config_store.py index 3b1707b5c..dc83795d8 100644 --- a/src/keboola_agent_cli/config_store.py +++ b/src/keboola_agent_cli/config_store.py @@ -708,7 +708,7 @@ def _reject_session_credential_swap(alias: str, stored_token: str, new_token: st remedy=( "A browser-login session keeps its credential in auth.json and " f"rotates it automatically. Run `kbagent project edit --project {alias} " - "--token ` to convert the project to a static Storage token " + "--token-stdin` to convert the project to a static Storage token " "deliberately, or `kbagent auth logout --remove-projects` to drop the " "session projects first." ), diff --git a/src/keboola_agent_cli/errors.py b/src/keboola_agent_cli/errors.py index d82589921..c3aed5479 100644 --- a/src/keboola_agent_cli/errors.py +++ b/src/keboola_agent_cli/errors.py @@ -320,9 +320,9 @@ def __init__(self, feature: str, *, remedy: str = "") -> None: else: message = ( f"{message} Point the project at a static Storage token instead: " - "`kbagent project edit --project --token ` converts the " + "`kbagent project edit --project --token-stdin` converts the " "alias you are already using, replacing its session credential. Use " - "`kbagent project add --project --url --token ` " + "`kbagent project add --project --url --token-stdin` " "only for a genuinely new alias -- `project add` rejects one that already exists." ) super().__init__(message) diff --git a/src/keboola_agent_cli/metastore_client.py b/src/keboola_agent_cli/metastore_client.py index ec2744989..03a8941af 100644 --- a/src/keboola_agent_cli/metastore_client.py +++ b/src/keboola_agent_cli/metastore_client.py @@ -211,7 +211,7 @@ def _do_request(self, method: str, path: str, **kwargs: Any) -> httpx.Response: remedy = ( f"The token ({self._masked_token}) is not a project admin. Check " f"`kbagent project info` -> is_master_token, and register a " - f"project-admin token (`kbagent project edit --token ...`) to " + f"project-admin token (`kbagent project edit --token-stdin`) to " f"use semantic-layer write commands." ) raise KeboolaApiError( diff --git a/src/keboola_agent_cli/services/config_service.py b/src/keboola_agent_cli/services/config_service.py index 5daba8c54..e75e8db07 100644 --- a/src/keboola_agent_cli/services/config_service.py +++ b/src/keboola_agent_cli/services/config_service.py @@ -2407,7 +2407,7 @@ def get_oauth_url( f"master token, so it cannot mint the short-lived " f"component-scoped child token the OAuth wizard expects. " f"Either re-add the project with a master token " - f"(`kbagent project edit --project {alias} --token `) " + f"(`kbagent project edit --project {alias} --token-stdin`) " f"or open the OAuth flow via the Keboola UI." ), ) diff --git a/src/keboola_agent_cli/services/token_service.py b/src/keboola_agent_cli/services/token_service.py index 82b83901b..fe9c090e6 100644 --- a/src/keboola_agent_cli/services/token_service.py +++ b/src/keboola_agent_cli/services/token_service.py @@ -316,7 +316,7 @@ def _require_master_token(self, client: KeboolaClient, *, alias: str) -> None: f"the Storage API rejects the request with a generic 500 " f"'Application error.' (CreateTokenVoter LogicException, " f"issue #599). Point the alias at a master token " - f"(`kbagent project edit --project {alias} --token `) " + f"(`kbagent project edit --project {alias} --token-stdin`) " f"-- master = the token from your own user account in the " f"Keboola UI, `isMasterToken: true` in `kbagent token list`." ), diff --git a/tests/test_auth_sentinel_guards.py b/tests/test_auth_sentinel_guards.py index 421988b25..6f207a51b 100644 --- a/tests/test_auth_sentinel_guards.py +++ b/tests/test_auth_sentinel_guards.py @@ -99,7 +99,7 @@ class TestRemedyText: def test_default_remedy_leads_with_project_edit(self) -> None: exc = SessionAuthUnsupportedError("The Keboola AI Service") - assert "project edit --project --token " in exc.message + assert "project edit --project --token-stdin" in exc.message # `project add` is still mentioned, but scoped to the case it works for. assert "project add --project " in exc.message assert "rejects one that already exists" in exc.message diff --git a/tests/test_cli.py b/tests/test_cli.py index 34158218c..39b50156a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,6 +3,8 @@ import json import os import re +from collections.abc import Iterator +from contextlib import contextmanager from pathlib import Path from unittest.mock import MagicMock, patch @@ -5688,8 +5690,9 @@ def test_project_add_token_from_env(self, tmp_path: Path) -> None: def test_project_add_token_interactive(self, tmp_path: Path) -> None: """Interactive hidden prompt works for project add when no env var. - We mock _resolve_token to simulate the interactive prompt returning a token, - since CliRunner does not have a real TTY and sys.stdin.isatty() is False. + We mock resolve_storage_token_input to simulate the interactive prompt + returning a token, since CliRunner does not have a real TTY and + sys.stdin.isatty() is False. """ mock_client = make_mock_client(project_name="PromptProject", project_id=888) config_dir = tmp_path / "config" @@ -5699,7 +5702,7 @@ def test_project_add_token_interactive(self, tmp_path: Path) -> None: patch("keboola_agent_cli.cli.ConfigStore") as MockStore, patch("keboola_agent_cli.cli.ProjectService") as MockService, patch( - "keboola_agent_cli.commands.project._resolve_token", + "keboola_agent_cli.commands.project.resolve_storage_token_input", return_value=TEST_TOKEN, ), ): @@ -5730,6 +5733,129 @@ def test_project_add_token_interactive(self, tmp_path: Path) -> None: assert output["status"] == "ok" assert output["data"]["alias"] == "prompttest" + @staticmethod + @contextmanager + def _patched_add(tmp_path: Path, mock_client: MagicMock) -> Iterator[None]: + """The `project add` wiring the token-source tests all need.""" + config_dir = tmp_path / "config" + config_dir.mkdir(exist_ok=True) + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockService, + ): + store_instance = ConfigStore(config_dir=config_dir) + MockStore.return_value = store_instance + MockService.return_value = ProjectService( + config_store=store_instance, + client_factory=lambda url, token: mock_client, + ) + yield + + def test_project_add_token_from_file_deletes_the_file(self, tmp_path: Path) -> None: + """The file carries the token in; it must not stay behind holding it.""" + token_file = tmp_path / "token.txt" + token_file.write_text(f"{TEST_TOKEN}\n", encoding="utf-8") + token_file.chmod(0o600) + + with self._patched_add(tmp_path, make_mock_client(project_name="FileProject")): + result = runner.invoke( + app, + [ + "--json", + "project", + "add", + "--project", + "filetest", + "--token-file", + str(token_file), + ], + ) + + assert result.exit_code == 0, f"Exit code {result.exit_code}: {result.output}" + assert json.loads(result.stdout)["data"]["alias"] == "filetest" + assert not token_file.exists() + assert TEST_TOKEN not in result.output + + def test_project_add_keep_token_file_leaves_the_file(self, tmp_path: Path) -> None: + token_file = tmp_path / "token.txt" + token_file.write_text(TEST_TOKEN, encoding="utf-8") + token_file.chmod(0o600) + + with self._patched_add(tmp_path, make_mock_client()): + result = runner.invoke( + app, + [ + "--json", + "project", + "add", + "--project", + "keeptest", + "--token-file", + str(token_file), + "--keep-token-file", + ], + ) + + assert result.exit_code == 0, f"Exit code {result.exit_code}: {result.output}" + assert token_file.exists() + assert "still holds the token" in result.stderr + + def test_project_add_token_from_stdin(self, tmp_path: Path) -> None: + with self._patched_add(tmp_path, make_mock_client(project_name="StdinProject")): + result = runner.invoke( + app, + ["--json", "project", "add", "--project", "stdintest", "--token-stdin"], + input=f"{TEST_TOKEN}\n", + ) + + assert result.exit_code == 0, f"Exit code {result.exit_code}: {result.output}" + assert json.loads(result.stdout)["data"]["project_name"] == "StdinProject" + assert TEST_TOKEN not in result.output + + def test_project_add_token_from_named_env_var(self, tmp_path: Path) -> None: + """--token-env names the variable, so nothing is read implicitly.""" + with ( + self._patched_add(tmp_path, make_mock_client(project_name="NamedEnvProject")), + patch.dict(os.environ, {"CI_KBC_TOKEN": TEST_TOKEN}), + ): + result = runner.invoke( + app, + [ + "--json", + "project", + "add", + "--project", + "namedenvtest", + "--token-env", + "CI_KBC_TOKEN", + ], + ) + + assert result.exit_code == 0, f"Exit code {result.exit_code}: {result.output}" + assert json.loads(result.stdout)["data"]["project_name"] == "NamedEnvProject" + + def test_project_add_rejects_two_token_sources(self, tmp_path: Path) -> None: + token_file = tmp_path / "token.txt" + token_file.write_text(TEST_TOKEN, encoding="utf-8") + + with self._patched_add(tmp_path, make_mock_client()): + result = runner.invoke( + app, + [ + "project", + "add", + "--project", + "conflicttest", + "--token", + TEST_TOKEN, + "--token-file", + str(token_file), + ], + ) + + assert result.exit_code == 2 + assert token_file.exists(), "a rejected command must not delete the file" + def test_project_add_rejects_http_url(self, tmp_path: Path) -> None: """http:// URL rejected with error at project add time.""" mock_client = make_mock_client() diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 3529e47b4..d49d2a751 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,9 +1,21 @@ """Tests for commands._helpers shared command-layer utilities.""" +import os +from collections.abc import Callable +from pathlib import Path + import pytest import typer - -from keboola_agent_cli.commands._helpers import map_error_to_exit_code, read_password_stdin +from typer.testing import CliRunner + +from keboola_agent_cli.commands import _helpers +from keboola_agent_cli.commands._helpers import ( + discard_token_file, + map_error_to_exit_code, + read_password_stdin, + resolve_storage_token_input, + token_came_from_command_line, +) from keboola_agent_cli.errors import ErrorCode, KeboolaApiError, map_error_code_to_type @@ -519,3 +531,313 @@ def raise_eof(): require_random_code_confirmation("patch app") assert exc.value.exit_code == 6 + + +class TestResolveStorageTokenInput: + """`project add` / `project edit` accept a Storage token from four sources. + + Everything except `--token` keeps the value off the command line, and so + out of the shell history, the kbagent REPL history file, and a process + listing. The sources are mutually exclusive, and only a command-line + `--token` earns the shell-history warning -- a value that Typer merged in + from KBC_TOKEN was never typed, so warning about it would be a lie. + """ + + _SENTINEL = "9999-kbagent-test-sentinel-storage-token" + + def _write_token_file(self, tmp_path: Path, mode: int = 0o600) -> Path: + path = tmp_path / "token.txt" + path.write_text(f"{self._SENTINEL}\n", encoding="utf-8") + path.chmod(mode) + return path + + # ----- one source at a time ----- + + def test_stdin_source(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + _helpers, "read_password_stdin", lambda label="Password: ": self._SENTINEL + ) + result = resolve_storage_token_input(token=None, token_stdin=True, required=True) + assert result == self._SENTINEL + + def test_file_source_strips_trailing_newline(self, tmp_path: Path) -> None: + path = self._write_token_file(tmp_path) + assert resolve_storage_token_input(token=None, token_file=path, required=True) == ( + self._SENTINEL + ) + + def test_env_source(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("CI_KBC_TOKEN", self._SENTINEL) + result = resolve_storage_token_input(token=None, token_env="CI_KBC_TOKEN", required=True) + assert result == self._SENTINEL + + def test_token_from_cli_returns_value(self) -> None: + result = resolve_storage_token_input(token=self._SENTINEL, required=True) + assert result == self._SENTINEL + + # ----- the shell-history warning ----- + + def test_cli_token_on_a_tty_warns_about_shell_history( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + monkeypatch.setattr(_helpers.sys.stdin, "isatty", lambda: True, raising=False) + resolve_storage_token_input(token=self._SENTINEL, required=True, token_from_cli=True) + assert "shell history" in capsys.readouterr().err + + def test_env_token_never_warns( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + """A KBC_TOKEN value reaches the resolver in the same parameter as + `--token`, but it was never typed, so it is not in the history.""" + monkeypatch.setattr(_helpers.sys.stdin, "isatty", lambda: True, raising=False) + resolve_storage_token_input(token=self._SENTINEL, required=True, token_from_cli=False) + assert "shell history" not in capsys.readouterr().err + + def test_no_tty_does_not_warn(self, capsys: pytest.CaptureFixture[str]) -> None: + resolve_storage_token_input(token=self._SENTINEL, required=True, token_from_cli=True) + assert "shell history" not in capsys.readouterr().err + + # ----- mutual exclusion ----- + + def test_two_explicit_sources_are_rejected(self, tmp_path: Path) -> None: + path = self._write_token_file(tmp_path) + with pytest.raises(typer.BadParameter, match="Specify exactly one of"): + resolve_storage_token_input( + token=None, token_stdin=True, token_file=path, required=True + ) + + def test_cli_token_conflicts_with_an_explicit_source(self, tmp_path: Path) -> None: + path = self._write_token_file(tmp_path) + with pytest.raises(typer.BadParameter, match="Specify exactly one of"): + resolve_storage_token_input(token=self._SENTINEL, token_file=path, required=True) + + def test_env_token_loses_to_an_explicit_source(self, tmp_path: Path) -> None: + """KBC_TOKEN is implicit, so it does not collide -- explicit wins.""" + path = self._write_token_file(tmp_path) + result = resolve_storage_token_input( + token="9999-value-from-the-environment", + token_file=path, + required=True, + token_from_cli=False, + ) + assert result == self._SENTINEL + + def test_keep_token_file_without_token_file_is_rejected(self) -> None: + with pytest.raises(typer.BadParameter, match="only together with --token-file"): + resolve_storage_token_input(token=None, keep_token_file=True, required=False) + + # ----- bad input ----- + + def test_empty_file_is_rejected(self, tmp_path: Path) -> None: + path = tmp_path / "empty.txt" + path.write_text(" \n", encoding="utf-8") + with pytest.raises(typer.BadParameter, match="is empty"): + resolve_storage_token_input(token=None, token_file=path, required=True) + + def test_unreadable_file_is_rejected(self, tmp_path: Path) -> None: + # A directory makes read_text() raise OSError on every platform, which is + # exactly what the resolver must turn into a BadParameter. chmod(0o000) + # would not achieve this on Windows (nor as root on POSIX), so it is not + # a portable way to produce an unreadable path. + not_a_file = tmp_path / "a_directory" + not_a_file.mkdir() + with pytest.raises(typer.BadParameter, match="Cannot read token file"): + resolve_storage_token_input(token=None, token_file=not_a_file, required=True) + + @pytest.mark.parametrize("value", ["", None]) + def test_unset_or_empty_named_variable_is_rejected( + self, monkeypatch: pytest.MonkeyPatch, value: str | None + ) -> None: + if value is None: + monkeypatch.delenv("CI_KBC_TOKEN", raising=False) + else: + monkeypatch.setenv("CI_KBC_TOKEN", value) + with pytest.raises(typer.BadParameter, match="unset or empty"): + resolve_storage_token_input(token=None, token_env="CI_KBC_TOKEN", required=True) + + # ----- file mode ----- + + @pytest.mark.skipif( + os.name == "nt", + reason="POSIX mode bits are not the access-control mechanism on Windows: " + "chmod cannot narrow an ACL, stat reports 0o666, and the check " + "short-circuits before grading them (same reason as " + "test_auth_state_store.py / test_doctor_service.py)", + ) + def test_world_readable_file_warns( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + path = self._write_token_file(tmp_path, mode=0o644) + resolve_storage_token_input(token=None, token_file=path, required=True) + assert "accessible to other users" in capsys.readouterr().err + + @pytest.mark.skipif( + os.name == "nt", + reason="POSIX mode bits are not the access-control mechanism on Windows: " + "chmod cannot narrow an ACL, stat reports 0o666, and the check " + "short-circuits before grading them", + ) + def test_private_file_does_not_warn( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + path = self._write_token_file(tmp_path, mode=0o600) + resolve_storage_token_input(token=None, token_file=path, required=True) + assert "accessible to other users" not in capsys.readouterr().err + + @pytest.mark.skipif( + os.name != "nt", + reason="verifies the real Windows behaviour: the mode check is skipped, " + "so a path that stat reports as group/other-accessible raises no warning", + ) + def test_permission_check_is_skipped_on_windows( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + path = self._write_token_file(tmp_path, mode=0o644) + assert resolve_storage_token_input(token=None, token_file=path, required=True) == ( + self._SENTINEL + ) + assert "accessible to other users" not in capsys.readouterr().err + + # ----- nothing given ----- + + def test_optional_token_returns_none_without_prompting( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`project edit --new-alias` passes no token and must not be asked for one.""" + monkeypatch.setattr(_helpers.sys.stdin, "isatty", lambda: True, raising=False) + monkeypatch.setattr( + _helpers.typer, "prompt", lambda *a, **k: pytest.fail("must not prompt") + ) + assert resolve_storage_token_input(token=None, required=False) is None + + def test_required_token_prompts_on_a_tty(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(_helpers.sys.stdin, "isatty", lambda: True, raising=False) + monkeypatch.setattr(_helpers.typer, "prompt", lambda *a, **k: "prompted-token") + assert resolve_storage_token_input(token=None, required=True) == "prompted-token" + + def test_required_token_without_a_tty_exits_2(self, capsys: pytest.CaptureFixture[str]) -> None: + with pytest.raises(typer.Exit) as excinfo: + resolve_storage_token_input(token=None, required=True) + assert excinfo.value.exit_code == 2 + err = capsys.readouterr().err + for flag in ("--token-stdin", "--token-file", "--token-env", "KBC_TOKEN"): + assert flag in err + + # ----- leak regression ----- + + def test_token_value_never_appears_in_captured_output( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + """Regression pin: no source may echo the token to stdout or stderr.""" + monkeypatch.setenv("CI_KBC_TOKEN", self._SENTINEL) + monkeypatch.setattr( + _helpers, "read_password_stdin", lambda label="Password: ": self._SENTINEL + ) + monkeypatch.setattr(_helpers.sys.stdin, "isatty", lambda: True, raising=False) + token_file = self._write_token_file(tmp_path, mode=0o644) + sources: list[Callable[[], str | None]] = [ + lambda: resolve_storage_token_input(token=self._SENTINEL, required=True), + lambda: resolve_storage_token_input(token=None, token_stdin=True, required=True), + lambda: resolve_storage_token_input(token=None, token_file=token_file, required=True), + lambda: resolve_storage_token_input( + token=None, token_env="CI_KBC_TOKEN", required=True + ), + ] + for resolve in sources: + assert resolve() == self._SENTINEL + captured = capsys.readouterr() + assert self._SENTINEL not in captured.out + assert self._SENTINEL not in captured.err + + +class TestDiscardTokenFile: + """`--token-file` is a carrier, not a store, so the file goes once the + command has succeeded. Never on a preview, never on a failure, and never + loudly enough to fail a command that already did its work.""" + + def _token_file(self, tmp_path: Path) -> Path: + path = tmp_path / "token.txt" + path.write_text("9999-carrier\n", encoding="utf-8") + return path + + def test_deletes_the_file(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + path = self._token_file(tmp_path) + discard_token_file(path) + assert not path.exists() + assert "Deleted the token file" in capsys.readouterr().err + + def test_keep_leaves_the_file_and_warns( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + path = self._token_file(tmp_path) + discard_token_file(path, keep=True) + assert path.exists() + assert "still holds the token" in capsys.readouterr().err + + def test_dry_run_leaves_the_file( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + path = self._token_file(tmp_path) + discard_token_file(path, dry_run=True) + assert path.exists() + assert "--dry-run" in capsys.readouterr().err + + def test_no_file_is_a_no_op(self, capsys: pytest.CaptureFixture[str]) -> None: + discard_token_file(None) + assert capsys.readouterr().err == "" + + def test_unlink_failure_warns_but_does_not_raise( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """discard must warn, not raise, when the unlink fails -- a read-only + mount (a Kubernetes secret, a CI volume) is a legitimate setup, and the + command has already succeeded by this point. + + A directory makes unlink() raise OSError on every platform, so it stands + in for the read-only-mount failure without depending on POSIX directory + permissions -- chmod(0o500) on a directory does not block deletion of a + file inside it on Windows.""" + not_a_file = tmp_path / "a_directory" + not_a_file.mkdir() + discard_token_file(not_a_file) + assert not_a_file.exists() + assert "could not delete" in capsys.readouterr().err + + +class TestTokenCameFromCommandLine: + """Regression pin for the vendored-Click trap. + + Typer ships its own copy of Click, so `click.core.ParameterSource` and the + enum a running context returns are different classes. The first version of + this check compared them with `==`, which is always False -- the + shell-history warning went silent and `--token` stopped conflicting with + the other sources. Matching on the member name survives both copies. + """ + + def _probe_app(self) -> tuple[typer.Typer, list[bool]]: + seen: list[bool] = [] + app = typer.Typer() + + @app.command() + def probe( + ctx: typer.Context, + token: str | None = typer.Option(None, envvar="PROBE_TOKEN"), + ) -> None: + seen.append(token_came_from_command_line(ctx)) + + return app, seen + + def test_command_line_value_is_reported_as_typed(self) -> None: + app, seen = self._probe_app() + result = CliRunner().invoke(app, ["--token", "typed-value"]) + assert result.exit_code == 0, result.output + assert seen == [True] + + def test_environment_value_is_not_reported_as_typed( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("PROBE_TOKEN", "env-value") + app, seen = self._probe_app() + result = CliRunner().invoke(app, []) + assert result.exit_code == 0, result.output + assert seen == [False] diff --git a/tests/test_project_edit_cli.py b/tests/test_project_edit_cli.py index 2c92541ff..a9b23b4cb 100644 --- a/tests/test_project_edit_cli.py +++ b/tests/test_project_edit_cli.py @@ -20,6 +20,7 @@ from keboola_agent_cli.auth.sentinel import make_session_token from keboola_agent_cli.cli import app from keboola_agent_cli.config_store import ConfigStore +from keboola_agent_cli.errors import KeboolaApiError from keboola_agent_cli.models import ProjectConfig from keboola_agent_cli.services.project_service import ProjectService @@ -267,7 +268,7 @@ def _seed_session_project(self, config_dir: Path) -> ConfigStore: ) return store - def _invoke(self, tmp_path: Path, argv: list[str]): + def _invoke(self, tmp_path: Path, argv: list[str], *, stdin: str | None = None): config_dir = tmp_path / "config" config_dir.mkdir() store = self._seed_session_project(config_dir) @@ -283,7 +284,7 @@ def _invoke(self, tmp_path: Path, argv: list[str]): ): MockStore.return_value = store MockService.return_value = service - result = runner.invoke(app, argv) + result = runner.invoke(app, argv, input=stdin) return store, result def test_human_mode_warns_on_stderr_and_converts(self, tmp_path: Path) -> None: @@ -303,6 +304,20 @@ def test_human_mode_warns_on_stderr_and_converts(self, tmp_path: Path) -> None: assert project is not None assert project.token == self.NEW_TOKEN + def test_token_stdin_still_warns_about_the_conversion(self, tmp_path: Path) -> None: + """The new sources change where the token comes from, nothing else.""" + store, result = self._invoke( + tmp_path, + ["project", "edit", "--project", self.SESSION_ALIAS, "--token-stdin"], + stdin=f"{self.NEW_TOKEN}\n", + ) + + assert result.exit_code == 0, result.output + assert "auth logout --remove-projects" in " ".join(result.stderr.split()) + project = store.get_project(self.SESSION_ALIAS) + assert project is not None + assert project.token == self.NEW_TOKEN + def test_json_mode_carries_the_warning_in_the_payload(self, tmp_path: Path) -> None: _store, result = self._invoke( tmp_path, @@ -322,3 +337,195 @@ def test_json_mode_carries_the_warning_in_the_payload(self, tmp_path: Path) -> N assert payload["status"] == "ok" assert len(payload["data"]["warnings"]) == 1 assert "auth logout --remove-projects" in payload["data"]["warnings"][0] + + +class TestProjectEditTokenSources: + """A new token can reach ``project edit`` without the command line. + + ``--token`` puts the value in the shell history, in the kbagent REPL + history file, and in a process listing. ``--token-stdin``, + ``--token-file`` and ``--token-env`` all keep it out of those places. + ``--token-file`` also deletes the file it read, but only once the edit + has actually succeeded. + """ + + ALIAS = "tokentest" + NEW_TOKEN = "901-22222-replacementTokenValue123" + + def _seeded_store(self, config_dir: Path) -> ConfigStore: + store = ConfigStore(config_dir=config_dir) + store.add_project( + self.ALIAS, + ProjectConfig( + stack_url="https://connection.keboola.com", + token=TEST_TOKEN, + project_name="Test Project", + project_id=1234, + ), + ) + return store + + def _invoke(self, tmp_path: Path, argv: list[str], *, client=None, stdin: str | None = None): + config_dir = tmp_path / "config" + config_dir.mkdir() + store = self._seeded_store(config_dir) + service = ProjectService( + config_store=store, + client_factory=lambda url, token: client if client is not None else make_mock_client(), + ) + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockService, + ): + MockStore.return_value = store + MockService.return_value = service + result = runner.invoke(app, argv, input=stdin) + return store, result + + def _token_file(self, tmp_path: Path) -> Path: + path = tmp_path / "token.txt" + path.write_text(f"{self.NEW_TOKEN}\n", encoding="utf-8") + path.chmod(0o600) + return path + + def test_token_file_is_read_and_then_deleted(self, tmp_path: Path) -> None: + path = self._token_file(tmp_path) + store, result = self._invoke( + tmp_path, + ["project", "edit", "--project", self.ALIAS, "--token-file", str(path)], + ) + + assert result.exit_code == 0, result.output + project = store.get_project(self.ALIAS) + assert project is not None + assert project.token == self.NEW_TOKEN + assert not path.exists() + assert self.NEW_TOKEN not in result.output + + def test_dry_run_keeps_the_token_file(self, tmp_path: Path) -> None: + """A preview must not destroy the caller's input.""" + path = self._token_file(tmp_path) + store, result = self._invoke( + tmp_path, + [ + "project", + "edit", + "--project", + self.ALIAS, + "--token-file", + str(path), + "--dry-run", + ], + ) + + assert result.exit_code == 0, result.output + assert path.exists() + assert "--dry-run" in result.stderr + project = store.get_project(self.ALIAS) + assert project is not None + assert project.token == TEST_TOKEN, "dry run must not change the stored token" + + def test_keep_token_file_keeps_the_file_and_warns(self, tmp_path: Path) -> None: + path = self._token_file(tmp_path) + _store, result = self._invoke( + tmp_path, + [ + "project", + "edit", + "--project", + self.ALIAS, + "--token-file", + str(path), + "--keep-token-file", + ], + ) + + assert result.exit_code == 0, result.output + assert path.exists() + assert "still holds the token" in result.stderr + + def test_a_failed_edit_keeps_the_token_file(self, tmp_path: Path) -> None: + """The token was never stored, so the file is still the only copy.""" + path = self._token_file(tmp_path) + failing = make_mock_client() + failing.verify_token.side_effect = KeboolaApiError( + "Invalid access token", status_code=401, error_code="INVALID_TOKEN" + ) + + _store, result = self._invoke( + tmp_path, + ["project", "edit", "--project", self.ALIAS, "--token-file", str(path)], + client=failing, + ) + + assert result.exit_code != 0 + assert path.exists() + + def test_token_env_names_the_variable(self, tmp_path: Path) -> None: + with patch.dict(os.environ, {"CI_KBC_TOKEN": self.NEW_TOKEN}): + store, result = self._invoke( + tmp_path, + [ + "project", + "edit", + "--project", + self.ALIAS, + "--token-env", + "CI_KBC_TOKEN", + ], + ) + + assert result.exit_code == 0, result.output + project = store.get_project(self.ALIAS) + assert project is not None + assert project.token == self.NEW_TOKEN + + def test_kbc_token_in_the_environment_is_ignored(self, tmp_path: Path) -> None: + """Unlike `project add`, an edit never picks the token up implicitly. + + A new token rewrites the alias's project_id and project_name from + whatever it verifies as, so an exported variable could silently + repoint the alias at a different project during an unrelated edit. + """ + with patch.dict(os.environ, {"KBC_TOKEN": self.NEW_TOKEN}): + store, result = self._invoke( + tmp_path, + ["project", "edit", "--project", self.ALIAS, "--new-alias", "renamed"], + ) + + assert result.exit_code == 0, result.output + project = store.get_project("renamed") + assert project is not None + assert project.token == TEST_TOKEN, "the rename must not touch the credential" + + def test_token_stdin_is_read_from_a_pipe(self, tmp_path: Path) -> None: + store, result = self._invoke( + tmp_path, + ["project", "edit", "--project", self.ALIAS, "--token-stdin"], + stdin=f"{self.NEW_TOKEN}\n", + ) + + assert result.exit_code == 0, result.output + project = store.get_project(self.ALIAS) + assert project is not None + assert project.token == self.NEW_TOKEN + assert self.NEW_TOKEN not in result.output + + def test_two_token_sources_are_rejected(self, tmp_path: Path) -> None: + path = self._token_file(tmp_path) + _store, result = self._invoke( + tmp_path, + [ + "project", + "edit", + "--project", + self.ALIAS, + "--token", + self.NEW_TOKEN, + "--token-file", + str(path), + ], + ) + + assert result.exit_code == 2 + assert path.exists(), "a rejected command must not delete the file" diff --git a/tests/test_project_token_stdin_no_leak.py b/tests/test_project_token_stdin_no_leak.py new file mode 100644 index 000000000..9cc71fa39 --- /dev/null +++ b/tests/test_project_token_stdin_no_leak.py @@ -0,0 +1,289 @@ +"""``--token-stdin`` must never print the token it read. + +The point of the flag is a token that never reaches the shell history. That +only holds if kbagent does not print it back either: not in human output, not +in ``--json``, not in ``--dry-run``, not in the DEBUG log. + +These tests run the real CLI with the real ``ProjectService`` and the real +``KeboolaClient``. Only the network is replaced, so every string the code path +can produce is checked -- the success line, the JSON payload, the dry-run +preview, the error message, the log records. +""" + +from __future__ import annotations + +import logging +import re +from collections.abc import Iterator +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler, HTTPServer +from pathlib import Path +from threading import Thread + +import pytest +from typer.testing import CliRunner + +from keboola_agent_cli.cli import app +from keboola_agent_cli.client import KeboolaClient +from keboola_agent_cli.config_store import ConfigStore +from keboola_agent_cli.models import ProjectConfig + +STACK = "https://connection.keboola.com" +VERIFY_URL = f"{STACK}/v2/storage/tokens/verify" +ALIAS = "prod" + +# Fake values. The middle part is checked on its own as well: `mask_token` +# keeps the prefix and the last four characters, so a broken mask would show +# up as the middle part being visible. +NEW_TOKEN = "901-77777-fakeStdinTokenDoNotUseXXXX" +OLD_TOKEN = "901-55555-fakeStoredTokenDoNotUseYYYY" +SECRETS = (NEW_TOKEN, NEW_TOKEN[4:-4], OLD_TOKEN, OLD_TOKEN[4:-4]) + +VERIFY_OK = {"id": "12345", "description": "d", "owner": {"id": 1234, "name": "Test Project"}} + +runner = CliRunner() + + +def _seed_project(config_dir: Path) -> None: + """Register ``ALIAS`` with ``OLD_TOKEN``, as a `project edit` target.""" + ConfigStore(config_dir=config_dir).add_project( + ALIAS, + ProjectConfig( + stack_url=STACK, token=OLD_TOKEN, project_name="Old Project", project_id=1234 + ), + ) + + +def _stored_token(config_dir: Path) -> str: + project = ConfigStore(config_dir=config_dir).get_project(ALIAS) + assert project is not None + return project.token + + +# scenario id -> (command args, seed the project first, verify-token status or +# None when the command makes no API call) +SCENARIOS = { + "add-ok": (["project", "add", "--project", ALIAS, "--url", STACK], False, 200), + "add-rejected-token": (["project", "add", "--project", ALIAS, "--url", STACK], False, 401), + "edit-ok": (["project", "edit", "--project", ALIAS], True, 200), + "edit-rejected-token": (["project", "edit", "--project", ALIAS], True, 401), + "edit-dry-run": (["project", "edit", "--project", ALIAS, "--dry-run"], True, None), +} + + +@pytest.mark.parametrize("verbose", [False, True], ids=["quiet", "verbose"]) +@pytest.mark.parametrize("json_mode", [False, True], ids=["human", "json"]) +@pytest.mark.parametrize("scenario", list(SCENARIOS)) +def test_token_read_from_stdin_is_never_printed( + scenario: str, + json_mode: bool, + verbose: bool, + tmp_path: Path, + httpx_mock, + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, +) -> None: + command, seed, verify_status = SCENARIOS[scenario] + config_dir = tmp_path / "config" + config_dir.mkdir() + if seed: + _seed_project(config_dir) + for name in ("KBC_TOKEN", "KBC_STORAGE_API_URL", "KBAGENT_PROJECT_FROM_ENV"): + monkeypatch.delenv(name, raising=False) + if verify_status is not None: + httpx_mock.add_response( + url=VERIFY_URL, + status_code=verify_status, + json=VERIFY_OK if verify_status == 200 else {"error": "Invalid access token"}, + ) + # The CLI sets the root logger to DEBUG under --verbose. pytest's own handler + # is already on the root logger, which makes that call a no-op here, so set + # the level the same way to see every record the run would print. + caplog.set_level(logging.DEBUG) + + argv = [ + "--config-dir", + str(config_dir), + *(["--json"] if json_mode else []), + *(["--verbose"] if verbose else []), + *command, + "--token-stdin", + ] + result = runner.invoke(app, argv, input=f"{NEW_TOKEN}\n") + + everything = "\n".join( + [result.stdout, result.stderr, result.output, caplog.text] + + [record.getMessage() for record in caplog.records] + ) + for secret in SECRETS: + assert secret not in everything + + # The run must not be vacuous: the token was read and used, and the DEBUG + # log did capture the HTTP call it was used in. + expected_exit = {200: 0, 401: 3, None: 0}[verify_status] + assert result.exit_code == expected_exit, result.output + if verify_status is None: + assert not httpx_mock.get_requests() + else: + (request,) = httpx_mock.get_requests() + assert request.headers["X-StorageApi-Token"] == NEW_TOKEN + assert "HTTP Request" in caplog.text + if verify_status == 200: + assert _stored_token(config_dir) == NEW_TOKEN + elif seed: + assert _stored_token(config_dir) == OLD_TOKEN, ( + "a dry run or a rejected token stores nothing" + ) + + +class _VerifyHandler(BaseHTTPRequestHandler): + """Answers the verify-token call and remembers the token header it got.""" + + received_token: str | None = None + + def do_GET(self) -> None: + type(self).received_token = self.headers.get("X-StorageApi-Token") + body = b'{"id": "1", "description": "d", "owner": {"id": 1, "name": "P"}}' + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, format: str, *args: object) -> None: + """Keep the test output quiet.""" + + +@contextmanager +def _verify_server() -> Iterator[str]: + server = HTTPServer(("127.0.0.1", 0), _VerifyHandler) + thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_port}" + finally: + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +def test_debug_log_of_the_real_transport_omits_the_token_header( + caplog: pytest.LogCaptureFixture, +) -> None: + """httpx and httpcore log at DEBUG; the header they send must not appear. + + The mocked transport above never reaches httpcore, which logs the most + detail. This sends a real request over a loopback socket. + """ + _VerifyHandler.received_token = None + caplog.set_level(logging.DEBUG) + + with _verify_server() as base_url, KeboolaClient(stack_url=base_url, token=NEW_TOKEN) as client: + client.verify_token() + + assert _VerifyHandler.received_token == NEW_TOKEN, "the token was sent on the wire" + assert any(record.name.startswith("httpcore") for record in caplog.records) + everything = caplog.text + "\n".join(record.getMessage() for record in caplog.records) + for secret in (NEW_TOKEN, NEW_TOKEN[4:-4]): + assert secret not in everything + + +SECOND_LINE = "second-line-of-something-piped-in-by-mistake" + + +def _error_text(output: str) -> str: + """Join the lines of the Rich error box, so a wrapped message reads as one line. + + CI renders the box with colour codes, so drop them too. + """ + plain = re.sub(r"\x1b\[[0-9;]*m", "", output) + return " ".join(plain.replace("\u2502", " ").split()) + + +@pytest.mark.parametrize("separator", ["\n", "\x1b"], ids=["line-break", "control-char"]) +@pytest.mark.parametrize("source", ["stdin", "file", "env", "token"]) +def test_multi_line_token_is_refused_without_printing_it( + source: str, separator: str, tmp_path: Path, httpx_mock, monkeypatch: pytest.MonkeyPatch +) -> None: + """A token with a line break must not reach the HTTP layer. + + The layer fails with "Illegal header value b''" and nothing + catches that, so an unhandled traceback would print the token and whatever + else was piped in with it (a whole `.env` file, for example). + """ + config_dir = tmp_path / "config" + config_dir.mkdir() + for name in ("KBC_TOKEN", "KBC_STORAGE_API_URL", "KBAGENT_PROJECT_FROM_ENV"): + monkeypatch.delenv(name, raising=False) + multi_line = f"{NEW_TOKEN}{separator}{SECOND_LINE}" + token_file = tmp_path / "token.txt" + token_file.write_text(multi_line, encoding="utf-8") + token_file.chmod(0o600) + monkeypatch.setenv("CI_KBC_TOKEN", multi_line) + flags, piped, flag_name = { + "stdin": (["--token-stdin"], multi_line, "--token-stdin"), + "file": (["--token-file", str(token_file)], None, "--token-file"), + "env": (["--token-env", "CI_KBC_TOKEN"], None, "--token-env"), + "token": (["--token", multi_line], None, "--token"), + }[source] + + argv = ["--config-dir", str(config_dir), "project", "add", "--project", ALIAS, "--url", STACK] + result = runner.invoke(app, [*argv, *flags], input=piped) + + assert result.exit_code == 2, result.output + message = _error_text(result.output) + assert f"Invalid value for {flag_name}: The token contains a line break" in message + assert not httpx_mock.get_requests() + assert token_file.exists(), "a refused token must not cost the user the file" + everything = result.output + result.stdout + result.stderr + for secret in (NEW_TOKEN, NEW_TOKEN[4:-4], SECOND_LINE): + assert secret not in everything + + +@pytest.mark.parametrize("piped", ["", "\n", " \n"]) +def test_empty_token_stdin_is_refused_before_any_request( + piped: str, tmp_path: Path, httpx_mock, monkeypatch: pytest.MonkeyPatch +) -> None: + """`printf '%s' "$KBC_TOKEN" | ... --token-stdin` with the variable unset pipes nothing.""" + config_dir = tmp_path / "config" + config_dir.mkdir() + for name in ("KBC_TOKEN", "KBC_STORAGE_API_URL", "KBAGENT_PROJECT_FROM_ENV"): + monkeypatch.delenv(name, raising=False) + + argv = ["--config-dir", str(config_dir), "project", "add", "--project", ALIAS, "--url", STACK] + result = runner.invoke(app, [*argv, "--token-stdin"], input=piped) + + assert result.exit_code == 2, result.output + assert "Invalid value for --token-stdin: The token is empty." in _error_text(result.output) + assert not httpx_mock.get_requests() + + +@pytest.mark.parametrize("command", ["add", "edit"]) +def test_explicit_empty_token_flag_is_refused( + command: str, tmp_path: Path, httpx_mock, monkeypatch: pytest.MonkeyPatch +) -> None: + """`--token "$NEW_TOKEN"` with the variable empty must fail, not pass silently. + + For `project edit` an empty value read as "no token given" would repoint + the alias to the new URL and keep the old stack's token, with exit 0. + """ + config_dir = tmp_path / "config" + config_dir.mkdir() + for name in ("KBC_TOKEN", "KBC_STORAGE_API_URL", "KBAGENT_PROJECT_FROM_ENV"): + monkeypatch.delenv(name, raising=False) + new_url = "https://connection.north-europe.azure.keboola.com" + if command == "edit": + _seed_project(config_dir) + + argv = ["--config-dir", str(config_dir), "project", command, "--project", ALIAS] + result = runner.invoke(app, [*argv, "--url", new_url, "--token", ""]) + + assert result.exit_code == 2, result.output + assert "Invalid value for --token: The token is empty." in _error_text(result.output) + assert not httpx_mock.get_requests() + stored = ConfigStore(config_dir=config_dir).get_project(ALIAS) + if command == "edit": + assert stored is not None + assert (stored.stack_url, stored.token) == (STACK, OLD_TOKEN) + else: + assert stored is None