From 9d8ab3044ef6637b28127248ad4477870cd160d7 Mon Sep 17 00:00:00 2001 From: soustruh Date: Wed, 2 Sep 2026 00:17:33 +0200 Subject: [PATCH 1/6] feat(project): accept a Storage token without the command line `project add` and `project edit` now take --token-stdin, --token-file and --token-env. None of them writes the token to the shell history, to the kbagent REPL history file, or to a process listing. --token still works. On a terminal it now warns that the token is in the history. --token-file deletes the file after the command succeeds. It keeps the file on --dry-run, after a failure, and with --keep-token-file. `project edit` does not read KBC_TOKEN. A new token overwrites the alias's project_id and project_name with the values the token verifies as. Nothing checks that the token belongs to the same project, so an exported variable could repoint an alias during a plain --new-alias. `project add` reads KBC_TOKEN as before. --- CLAUDE.md | 4 +- docs/auth.md | 10 +- plugins/kbagent/agents/keboola-expert.md | 3 +- .../kbagent/references/commands-reference.md | 8 +- .../skills/kbagent/references/gotchas.md | 29 +- src/keboola_agent_cli/commands/_helpers.py | 252 +++++++++++++- src/keboola_agent_cli/commands/context.py | 50 ++- src/keboola_agent_cli/commands/project.py | 101 +++--- tests/test_cli.py | 132 +++++++- tests/test_helpers.py | 308 +++++++++++++++++- tests/test_project_edit_cli.py | 211 +++++++++++- 11 files changed, 1030 insertions(+), 78 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 376610b79..ccd0e3a82 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -447,10 +447,10 @@ kbagent auth register-projects [--stack URL|alias] [--all] [--project-id ID ...] # starts the server but no `/auth/*` operation is destructive, so it affects nothing here. # See docs/web-server.md. -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/docs/auth.md b/docs/auth.md index 30f834351..1510757ca 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 @@ -370,9 +370,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/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index b302c00fd..bdb81fe2d 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 @@ -145,6 +145,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) | diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 83281d11d..d9ae5075d 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -66,12 +66,12 @@ and its message is truncated. See `auth-workflow.md` for the end-to-end walkthrough and troubleshooting. ## Project Management -- `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) +- `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) - `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. Overridden at runtime by `KBAGENT_PROJECT=ALIAS` (env, beats pin) and by `--project ALIAS` (CLI flag, beats both) @@ -213,7 +213,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 fd33996b9..66f3ad126 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -31,7 +31,7 @@ Versioning convention: `MasterTokenRequiredError` is swallowed 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 master token (`kbagent project edit --token ...`). Do NOT + register a master token (`kbagent project edit --token-stdin`). Do NOT escalate this one to support -- it is by design. - **Exit code is unchanged: 3, same as `INVALID_TOKEN`.** All three are authentication-class failures; only the diagnosis differs. A script @@ -195,7 +195,7 @@ Versioning convention: the Scheduler Service paths (`flow schedule`, `flow schedule-remove`); and the importable SDK (`lib.Client`). If a task needs one of those, register the same project again - with a static Storage token (`project add --project --token ...`) + with a static Storage token (`project add --project --token-stdin`) instead of fighting the guard. - **Two surfaces people wrongly expect on that list.** `dev-portal` is unaffected -- it authenticates with its own Developer Portal identity @@ -230,13 +230,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 diff --git a/src/keboola_agent_cli/commands/_helpers.py b/src/keboola_agent_cli/commands/_helpers.py index 08c620cfc..aa878788d 100644 --- a/src/keboola_agent_cli/commands/_helpers.py +++ b/src/keboola_agent_cli/commands/_helpers.py @@ -11,6 +11,7 @@ import os import secrets import sys +from pathlib import Path from typing import Any import typer @@ -18,6 +19,7 @@ from ..config_store import ConfigStore from ..constants import ( ENV_KBC_MANAGE_API_TOKEN, + ENV_KBC_TOKEN, EXIT_JOB_TIMEOUT_TERMINATED, EXIT_PERMISSION_DENIED, ) @@ -73,23 +75,261 @@ def resolve_manage_token(*, allow_env: bool = False) -> str: raise typer.Exit(code=2) -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 + + +# 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, 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 read_password_stdin(label="Storage API token: ") + + if token_file is not None: + return _read_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 value + + if 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 012f64668..7e0e07f67 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] @@ -252,8 +253,32 @@ ### Project Management - 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 @@ -273,8 +298,14 @@ 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. @@ -917,7 +948,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 @@ -2062,7 +2094,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, data-app password). 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 a0a0f1a72..fe599b27f 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,11 +24,18 @@ 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, map_error_to_exit_code, resolve_manage_token, + resolve_storage_token_input, + token_came_from_command_line, ) from ._metadata_input import resolve_text_input @@ -151,39 +157,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, @@ -195,17 +168,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) @@ -216,6 +206,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( @@ -273,9 +264,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", @@ -302,19 +298,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, ) @@ -354,6 +368,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/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 fb62dd88a..de90a399e 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,9 +1,20 @@ """Tests for commands._helpers shared command-layer utilities.""" +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 @@ -744,3 +755,296 @@ 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: + path = self._write_token_file(tmp_path, mode=0o000) + with pytest.raises(typer.BadParameter, match="Cannot read token file"): + resolve_storage_token_input(token=None, token_file=path, 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 ----- + + def test_world_readable_file_warns( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + monkeypatch.setattr(_helpers.os, "name", "posix") + 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 + + def test_private_file_does_not_warn( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + monkeypatch.setattr(_helpers.os, "name", "posix") + 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 + + def test_permission_check_is_skipped_on_windows( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + """Windows reports fabricated POSIX bits, so the mode check would warn + on every use. It must be skipped there, as the two sibling checks are.""" + monkeypatch.setattr(_helpers.os, "name", "nt") + 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: + """A Kubernetes secret or a CI volume is read-only. The command has + already succeeded by this point, so failing it here would be worse + than the leftover file.""" + locked = tmp_path / "locked" + locked.mkdir() + path = locked / "token.txt" + path.write_text("9999-carrier\n", encoding="utf-8") + locked.chmod(0o500) + try: + discard_token_file(path) + assert path.exists() + assert "could not delete" in capsys.readouterr().err + finally: + locked.chmod(0o700) + + +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" From b2e1697c5ad29f1179731bfb28989fc054add7a1 Mon Sep 17 00:00:00 2001 From: soustruh Date: Wed, 2 Sep 2026 15:50:33 +0200 Subject: [PATCH 2/6] test(project): make the token-input tests portable to Windows The Windows CI job runs the suite, and three of the new token-input tests failed there. Each assumed POSIX filesystem behavior that Windows does not share. Production code is unchanged; the nt guard added in the previous commit is correct. - test_unreadable_file_is_rejected used chmod(0o000), which does not make a file unreadable on Windows. It now passes a directory, so read_text raises OSError on every platform. - test_world_readable_file_warns and test_private_file_does_not_warn forced os.name to "posix", which defeated the nt guard and read Windows' fabricated 0o666 bits. They now skip on Windows, as test_auth_state_store.py and test_doctor_service.py already do for POSIX-permission tests. - test_permission_check_is_skipped_on_windows monkeypatched os.name to "nt". It now runs only on a real Windows host. - test_unlink_failure_warns_but_does_not_raise used a read-only directory, which does not block deletion on Windows. It now passes a directory, so unlink raises OSError on every platform. --- tests/test_helpers.py | 66 +++++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index de90a399e..70864db07 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,5 +1,6 @@ """Tests for commands._helpers shared command-layer utilities.""" +import os from collections.abc import Callable from pathlib import Path @@ -859,9 +860,14 @@ def test_empty_file_is_rejected(self, tmp_path: Path) -> None: resolve_storage_token_input(token=None, token_file=path, required=True) def test_unreadable_file_is_rejected(self, tmp_path: Path) -> None: - path = self._write_token_file(tmp_path, mode=0o000) + # 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=path, required=True) + 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( @@ -876,28 +882,41 @@ def test_unset_or_empty_named_variable_is_rejected( # ----- 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, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: - monkeypatch.setattr(_helpers.os, "name", "posix") 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, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: - monkeypatch.setattr(_helpers.os, "name", "posix") 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, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: - """Windows reports fabricated POSIX bits, so the mode check would warn - on every use. It must be skipped there, as the two sibling checks are.""" - monkeypatch.setattr(_helpers.os, "name", "nt") path = self._write_token_file(tmp_path, mode=0o644) assert resolve_storage_token_input(token=None, token_file=path, required=True) == ( self._SENTINEL @@ -995,20 +1014,19 @@ def test_no_file_is_a_no_op(self, capsys: pytest.CaptureFixture[str]) -> None: def test_unlink_failure_warns_but_does_not_raise( self, tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: - """A Kubernetes secret or a CI volume is read-only. The command has - already succeeded by this point, so failing it here would be worse - than the leftover file.""" - locked = tmp_path / "locked" - locked.mkdir() - path = locked / "token.txt" - path.write_text("9999-carrier\n", encoding="utf-8") - locked.chmod(0o500) - try: - discard_token_file(path) - assert path.exists() - assert "could not delete" in capsys.readouterr().err - finally: - locked.chmod(0o700) + """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: From a367de58d8a6d2002af342832f4de55771af8886 Mon Sep 17 00:00:00 2001 From: soustruh Date: Sat, 10 Oct 2026 03:31:45 +0200 Subject: [PATCH 3/6] fix(project): reject a multi-line token, use --token-stdin in the docs examples --- README.md | 11 ++++--- docs/TUTORIAL.md | 19 ++++++++--- docs/demos/demo-add-project.tape | 2 +- docs/use-cases.md | 2 +- docs/web-server.md | 2 +- install.sh | 2 +- plugins/kbagent/agents/keboola-expert.md | 2 +- .../references/env-injection.md | 2 +- plugins/kbagent/skills/kbagent/SKILL.md | 5 +-- .../kbagent/references/auth-workflow.md | 6 ++-- .../skills/kbagent/references/gotchas.md | 10 +++--- src/keboola_agent_cli/auth/auth_client.py | 6 ++-- src/keboola_agent_cli/commands/_helpers.py | 33 ++++++++++++++++--- src/keboola_agent_cli/commands/context.py | 4 +-- src/keboola_agent_cli/commands/token.py | 2 +- src/keboola_agent_cli/config_store.py | 2 +- src/keboola_agent_cli/errors.py | 4 +-- src/keboola_agent_cli/metastore_client.py | 2 +- .../services/config_service.py | 2 +- .../services/token_service.py | 2 +- tests/test_auth_sentinel_guards.py | 2 +- 21 files changed, 80 insertions(+), 42 deletions(-) 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/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 9f3c41a63..9c7853c35 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -156,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/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 88090a98d..902cb3031 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -711,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 @@ -723,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 @@ -2113,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). @@ -3006,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. @@ -3814,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 000783994..c57269f0b 100644 --- a/src/keboola_agent_cli/commands/_helpers.py +++ b/src/keboola_agent_cli/commands/_helpers.py @@ -153,6 +153,25 @@ def _read_token_file(path: Path) -> str: 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. + """ + 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. @@ -259,8 +278,9 @@ def resolve_storage_token_input( Raises: typer.BadParameter: More than one explicit source, an unreadable or - empty file, an unset or empty named variable, or - ``--keep-token-file`` without ``--token-file``. + empty file, an unset or empty named variable, 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). """ @@ -287,10 +307,12 @@ def resolve_storage_token_input( ) if token_stdin: - return read_password_stdin(label="Storage API token: ") + return _require_single_line( + read_password_stdin(label="Storage API token: "), "--token-stdin" + ) if token_file is not None: - return _read_token_file(token_file) + return _require_single_line(_read_token_file(token_file), "--token-file") if token_env is not None: value = os.environ.get(token_env, "") @@ -299,9 +321,10 @@ def resolve_storage_token_input( f"Environment variable {token_env} is unset or empty.", param_hint="--token-env", ) - return value + return _require_single_line(value, "--token-env") if token: + _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 " diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 55a7ecb25..1a8725106 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -345,7 +345,7 @@ 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 @@ -362,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 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 From 6aac4a9040bc8b681ffbc69e964ff050cd4cc021 Mon Sep 17 00:00:00 2001 From: soustruh Date: Sat, 10 Oct 2026 03:43:00 +0200 Subject: [PATCH 4/6] fix(project): refuse an empty token from --token-stdin --- src/keboola_agent_cli/commands/_helpers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/keboola_agent_cli/commands/_helpers.py b/src/keboola_agent_cli/commands/_helpers.py index c57269f0b..19e876b32 100644 --- a/src/keboola_agent_cli/commands/_helpers.py +++ b/src/keboola_agent_cli/commands/_helpers.py @@ -161,8 +161,11 @@ def _require_single_line(token: str, param_hint: str) -> str: 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. + 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. " From 119d3c9e3e5d43b66c4f7e4068c6c018b208d66f Mon Sep 17 00:00:00 2001 From: soustruh Date: Sat, 10 Oct 2026 08:56:28 +0200 Subject: [PATCH 5/6] fix(project): refuse an explicit empty --token, add the token leak tests --- src/keboola_agent_cli/commands/_helpers.py | 7 +- tests/test_project_token_stdin_no_leak.py | 284 +++++++++++++++++++++ 2 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 tests/test_project_token_stdin_no_leak.py diff --git a/src/keboola_agent_cli/commands/_helpers.py b/src/keboola_agent_cli/commands/_helpers.py index 19e876b32..511e6247c 100644 --- a/src/keboola_agent_cli/commands/_helpers.py +++ b/src/keboola_agent_cli/commands/_helpers.py @@ -281,8 +281,9 @@ def resolve_storage_token_input( Raises: typer.BadParameter: More than one explicit source, an unreadable or - empty file, an unset or empty named variable, a value with a line - break or another control character, or ``--keep-token-file`` + 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). @@ -326,7 +327,7 @@ def resolve_storage_token_input( ) return _require_single_line(value, "--token-env") - if token: + 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( 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..cc0fe1d34 --- /dev/null +++ b/tests/test_project_token_stdin_no_leak.py @@ -0,0 +1,284 @@ +"""``--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 +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.""" + return " ".join(output.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 From 92ab97c0f087c85679f9bba4324c24c3e715f222 Mon Sep 17 00:00:00 2001 From: soustruh Date: Sat, 10 Oct 2026 09:08:54 +0200 Subject: [PATCH 6/6] test(project): strip colour codes before matching the token error --- tests/test_project_token_stdin_no_leak.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/test_project_token_stdin_no_leak.py b/tests/test_project_token_stdin_no_leak.py index cc0fe1d34..9cc71fa39 100644 --- a/tests/test_project_token_stdin_no_leak.py +++ b/tests/test_project_token_stdin_no_leak.py @@ -13,6 +13,7 @@ from __future__ import annotations import logging +import re from collections.abc import Iterator from contextlib import contextmanager from http.server import BaseHTTPRequestHandler, HTTPServer @@ -191,8 +192,12 @@ def test_debug_log_of_the_real_transport_omits_the_token_header( def _error_text(output: str) -> str: - """Join the lines of the Rich error box, so a wrapped message reads as one line.""" - return " ".join(output.replace("\u2502", " ").split()) + """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"])