From 0ab5ac90acbfefe171076c66113e142f9fd22ee6 Mon Sep 17 00:00:00 2001 From: Zeke Mostov Date: Thu, 15 Oct 2026 15:13:11 -0400 Subject: [PATCH] Drop voter user records from activity output, point the root help at the embedded skills, document uploading the GPG key with gh, and make failed login name the next setup step. --- AGENTS.md | 6 +- docs/activities.md | 3 + docs/commands.md | 2 + docs/gpg-signing.md | 24 ++- skills/bootstrapping-organization/SKILL.md | 8 +- skills/managing-identities/SKILL.md | 7 + skills/monitoring-activities/SKILL.md | 3 +- skills/provisioning-agent-identity/SKILL.md | 2 +- skills/references/cli-convention.md | 4 +- skills/signing-git-commits/SKILL.md | 5 +- src/auth.rs | 219 ++++++++++++-------- src/cli.rs | 42 +++- src/errors.rs | 100 ++++++++- src/operations.rs | 35 +++- tests/auth_command.rs | 66 +++++- tests/e2e/activities.rs | 30 +++ tests/e2e/identity.rs | 66 +++++- tests/e2e/sessions.rs | 2 +- 18 files changed, 500 insertions(+), 124 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a150a2a..63130bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -80,6 +80,7 @@ Default guidance for coding-agent runs in this repository. - Use Clap field types, defaults, value parsers, argument groups, and conflicts to enforce CLI invariants during parsing instead of recreating the same validation in command execution. +- Each flag has one spelling; do not add aliases. - Keep fields on Clap `Args` structs private unless another construction path is intentional. Downstream functions should accept validated domain inputs, not a publicly constructible bag of CLI options. @@ -295,6 +296,7 @@ Default guidance for coding-agent runs in this repository. describes, not again on the command. - Statuses, error codes, record fields, and other commands appear in backticks; literal examples stay literal. Value names are `UPPER_SNAKE`. -- The root help states prompting, JSON output, and exit codes, and points at - the `cli-convention` reference for record shapes and error codes. A test +- The root help states prompting, JSON output, and exit codes, points at + the embedded skills for multi-step tasks, and points at the + `cli-convention` reference for record shapes and error codes. A test binds the error-code table in that reference to the `ErrorCode` enum. diff --git a/docs/activities.md b/docs/activities.md index 40deda4..940117a 100644 --- a/docs/activities.md +++ b/docs/activities.md @@ -16,6 +16,9 @@ tk activity list --since 24h --limit 100 tk activity get --id ACTIVITY_ID ``` +Activity records list votes under `data.activity.votes` with each voter's +`userId` and `selection`. + Consensus: ```bash diff --git a/docs/commands.md b/docs/commands.md index 5c84ac8..951e96a 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -23,6 +23,8 @@ TK_NON_INTERACTIVE=true to fail fast instead. --message-format json prints one JSON record per line, each with a `reason` field; error records also carry a `code`. JSON output never prompts. +Before a multi-step task, read its workflow: `tk skills list`, then +`tk skills show --name NAME`. Record shapes and error codes: `tk skills show --name references/cli-convention`. Exit codes: 0 success, 1 runtime error, 2 usage error. ``` diff --git a/docs/gpg-signing.md b/docs/gpg-signing.md index b61d29a..4e0b22c 100644 --- a/docs/gpg-signing.md +++ b/docs/gpg-signing.md @@ -24,8 +24,28 @@ echo hello | tk gpg sign ``` To sign commits with this key, see [Git signing](./git-signing.md#gpg-signing). -To add the key to GitHub, paste the output of `tk gpg keys export` into -Settings, SSH and GPG keys, New GPG key. + +## GitHub + +```bash +# Let the GitHub CLI add GPG keys and read your account's emails. +gh auth refresh -s write:gpg_key,user:email + +# Show each key's user ID. +tk gpg keys list +# List the emails GitHub has verified on your account. +gh api user/emails --jq '.[] | select(.verified) | .email' + +# Upload the public key. +tk gpg keys export --key FINGERPRINT > ./signing-key.asc +gh gpg-key add ./signing-key.asc --title tk-signing +``` + +GitHub marks a commit Verified only when its committer email appears in the +key's user ID and is a verified email on the GitHub account. Add a missing +email under GitHub Settings, Emails, and open the confirmation link GitHub +sends; a key whose user ID lacks the email needs a new key created with that +email in its `--user-id`. ## Key management diff --git a/skills/bootstrapping-organization/SKILL.md b/skills/bootstrapping-organization/SKILL.md index bdc0304..38bb484 100644 --- a/skills/bootstrapping-organization/SKILL.md +++ b/skills/bootstrapping-organization/SKILL.md @@ -135,12 +135,14 @@ which human approves. The organization id comes from step 1. ## Troubleshooting -- `login` fails with `unauthorized` (401 or 403): the profile's public key is not - registered on a user in that organization. Add it to the root user at - https://app.turnkey.com, then rerun the same `login`. +- `login` fails with `unauthorized` (401 or 403): the profile's public key, + `details.publicKey`, is not registered on a user in that organization. Add + it to the root user at https://app.turnkey.com, then rerun the same `login`. - `login` fails with `invalid_input` naming `profile set`: the profile was saved with a different organization or endpoint. Run the `tk profile set` command the message names, then `login` again. +- `login` fails with any other error: see + [managing-identities](../managing-identities/SKILL.md). - `user tag create` fails with `api_error` and a 400: a tag with that name already exists. Read the id from `tk --profile admin --message-format json user tag list`. - `user update` returns `status: "pending"`: an existing policy gates user diff --git a/skills/managing-identities/SKILL.md b/skills/managing-identities/SKILL.md index 8771805..821f56d 100644 --- a/skills/managing-identities/SKILL.md +++ b/skills/managing-identities/SKILL.md @@ -137,6 +137,13 @@ intended user and the user is not a root quorum member. Deletion is final. ## Troubleshooting +- `login` fails: follow `details.nextStep` when present, checking its + condition first. A missing profile is `not_found`. A key Turnkey does not + accept is `unauthorized`; register `details.publicKey` on a user, then log + in again. + A missing credential file is `not_found` and a malformed one + `invalid_input`; neither carries `nextStep`. See + [cli-convention](../references/cli-convention.md) for the full `details`. - `user create` fails with `not_found` for `--tag-name`: the tag does not exist in this organization. Create it with `tk --profile admin --message-format json user tag create --name agent` or diff --git a/skills/monitoring-activities/SKILL.md b/skills/monitoring-activities/SKILL.md index 70c465c..0cd3b99 100644 --- a/skills/monitoring-activities/SKILL.md +++ b/skills/monitoring-activities/SKILL.md @@ -41,8 +41,7 @@ Inputs: the activity id from the pending record, and the approver's profile `status` is the record status; `data.activity.status` the raw value (`ACTIVITY_STATUS_CONSENSUS_NEEDED` while approvals are outstanding). - `data.activity.type` and `data.activity.intent` describe the request; - `data.activity.votes[]` lists votes with `userId` and `selection`. + `data.activity.type` and `data.activity.intent` describe the request. Without an id, page through recent activities: diff --git a/skills/provisioning-agent-identity/SKILL.md b/skills/provisioning-agent-identity/SKILL.md index 3c70eb0..7155243 100644 --- a/skills/provisioning-agent-identity/SKILL.md +++ b/skills/provisioning-agent-identity/SKILL.md @@ -137,7 +137,7 @@ no minting row here: the one key never expires. - `login` fails with `unauthorized` 401 or 403: the create activity is pending or rejected, or `PUBLIC_KEY` was mistyped. Compare `tk --profile admin --message-format json api-key list --user-id AGENT_USER_ID` - with `data.publicKey` from step 2. + with the error's `details.publicKey`. - The self-registration in step 6 returns `completed` or `pending`: the DENY is missing or names the wrong tag, such as the `''` an unset `AGENT_TAG` leaves. First create `agents-no-credentials` and diff --git a/skills/references/cli-convention.md b/skills/references/cli-convention.md index 63923e4..6a10a21 100644 --- a/skills/references/cli-convention.md +++ b/skills/references/cli-convention.md @@ -47,7 +47,7 @@ this shape: | `reason` | `command_result` | | `command` | dotted command name, for example `user.create`, `secret.env`, `auth.whoami` | | `status` | `completed`, `pending`, `rejected`, `failed`, or `unknown`; a record without an activity is `completed` | -| `data` | the command's payload; for API-backed commands it preserves the API response shape | +| `data` | the command's payload; for API-backed commands it preserves the API response shape, except that activity records omit each vote's nested `user`; `request` prints the response unchanged | | `activity` | present when the command submitted or observed an activity: `{"id", "status"}` with the raw `ACTIVITY_STATUS_*` value | Created resource ids live under `data.activity.result`, for example @@ -75,7 +75,7 @@ Any command can print an error record instead, with exit code `1` or `2`: | `reason` | `command_error`, or `missing_required_input` when a value was absent in non-interactive mode | | `code` | the stable classification below | | `httpStatus` | present for HTTP failures | -| `details` | recovery data: the last observed `activity` `{id, status}`, `pending` exports, or session expiry fields | +| `details` | recovery data: the last observed `activity` `{id, status}`, `pending` exports, session expiry fields, or, for profile and credential failures, the `profile`, saved `profiles`, credential `path`, or rejected `publicKey` and `organizationId`, plus `nextStep` suggesting a fix and the condition it assumes when no profile is selected, the named profile does not exist, or Turnkey rejects the key in `login` or `whoami` | | `message` | the full error chain, for humans | Error records carry no `schemaVersion`, `data`, or `activity`. diff --git a/skills/signing-git-commits/SKILL.md b/skills/signing-git-commits/SKILL.md index b9be94d..1b4d721 100644 --- a/skills/signing-git-commits/SKILL.md +++ b/skills/signing-git-commits/SKILL.md @@ -24,8 +24,9 @@ Result: commits and tags signed by a key that never leaves Turnkey, which - Signing waits synchronously, so the policy is allow-always for the agent user, scoped to the exact resource: `wallet.id` for GPG, `private_key.id` for SSH. See [policy-patterns.md](../references/policy-patterns.md#signing-key-policies). -- Local verification is the success criterion. A hosting provider's "Verified" - badge needs the public key and committer email there; this makes no claim. +- Local verification is the success criterion. A hosting provider's + "Verified" badge is an external step this workflow makes no claim about; + for a GPG key on GitHub, see [gpg-signing](../../docs/gpg-signing.md#github). - SSH signing has two paths. Direct: `gpg.ssh.program` runs `tk` (step 6), so git's process runs `tk` and git's boundary holds the Turnkey credential. Agent: `gpg.ssh.program` is `ssh-keygen` with `SSH_AUTH_SOCK` on the diff --git a/src/auth.rs b/src/auth.rs index 436a5f7..0f9584a 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -24,9 +24,13 @@ use turnkey_client::{ generated::{GetWhoamiRequest, GetWhoamiResponse}, }; use uuid::Uuid; +use zeroize::Zeroizing; use crate::{ - errors::{InvalidInput, Malformed, OrganizationMismatch}, + errors::{ + CredentialRejected, InvalidInput, Malformed, MissingCredentialFile, NoSelectedIdentity, + OrganizationMismatch, ProfileNotFound, SavedProfiles, is_unauthorized, + }, gpg::registry::{GpgKeyEntry, GpgKeyTable, KeyName, SelectError, SigningKeyName, StoredGpgKey}, keygen::{GeneratedApiKey, generate}, operations::OperationOutput, @@ -597,16 +601,30 @@ fn parse_key(private: &str, public: &str) -> Result { } pub(crate) async fn read_key(path: &Path) -> Result { - let text = fs::read_to_string(path) - .await - .with_context(|| format!("read credential {}", path.display()))?; - let key: StoredApiKey = serde_json::from_str(&text).map_err(|error| { + let text = match fs::read_to_string(path).await { + Ok(text) => Zeroizing::new(text), + Err(error) if error.kind() == ErrorKind::NotFound => { + return Err(MissingCredentialFile { + path: path.to_owned(), + } + .into()); + } + Err(error) => { + return Err(error).with_context(|| format!("read credential {}", path.display())); + } + }; + let StoredApiKey { + public_key, + private_key, + curve: _, + } = serde_json::from_str(&text).map_err(|error| { Malformed::new( format!("invalid credential JSON in {}", path.display()), error, ) })?; - parse_key(&key.private_key, &key.public_key) + let private_key = Zeroizing::new(private_key); + parse_key(&private_key, &public_key) } pub(crate) fn endpoint_override(options: &AuthOptions) -> Result> { @@ -628,8 +646,18 @@ pub async fn resolve(options: &AuthOptions) -> Result { if let Some(auth) = resolve_environment(options)? { return Ok(auth); } - let registry = LoadedRegistry::load().await?; - resolve_in_registry(options, ®istry.path, ®istry.registry).await + let LoadedRegistry { mut registry, .. } = LoadedRegistry::load().await?; + let Some(name) = options + .profile + .clone() + .or_else(|| registry.active_profile.take()) + else { + return Err(NoSelectedIdentity { + saved: SavedProfiles(registry.profiles.into_keys().collect()), + } + .into()); + }; + resolve_in_registry(options, name, ®istry).await } pub struct LoadedRegistry { @@ -658,11 +686,13 @@ impl LoadedRegistry { return Ok(Some((organization_id, "--organization-id".into()))); } if let Some(name) = &options.profile { - let profile = self - .registry - .profiles - .get(name) - .ok_or_else(|| profile_missing(name))?; + let Some(profile) = self.registry.profiles.get(name) else { + return Err(ProfileNotFound { + name: name.clone(), + saved: SavedProfiles(self.registry.profiles.keys().cloned().collect()), + } + .into()); + }; return Ok(Some((profile.organization_id, format!("profile {name}")))); } if ENV_BUNDLE @@ -700,8 +730,8 @@ impl LoadedRegistry { } Ok(auth) }; - if options.profile.is_some() { - return checked(resolve_in_registry(options, &self.path, &self.registry).await?); + if let Some(name) = &options.profile { + return checked(resolve_in_registry(options, name.clone(), &self.registry).await?); } if let Some(auth) = resolve_environment(options)? { return checked(auth); @@ -739,20 +769,16 @@ impl LoadedRegistry { async fn resolve_in_registry( options: &AuthOptions, - path: &Path, + name: String, registry: &Registry, ) -> Result { - let name = options - .profile - .clone() - .or_else(|| registry.active_profile.clone()) - .ok_or_else(|| InvalidInput("no selected identity; use --profile or tk login".into()))?; - let profile = registry.profiles.get(&name).ok_or_else(|| { - InvalidInput(format!( - "profile {name} does not exist in {}", - path.display() - )) - })?; + let Some(profile) = registry.profiles.get(&name) else { + return Err(ProfileNotFound { + name, + saved: SavedProfiles(registry.profiles.keys().cloned().collect()), + } + .into()); + }; resolve_profile(options, name, profile).await } @@ -764,11 +790,16 @@ fn resolve_environment(options: &AuthOptions) -> Result> { if bundle.iter().all(Option::is_none) { return Ok(None); } - let [org, public, private] = bundle; - let (Some(org), Some(public), Some(private)) = (org, public, private) else { - return Err(InvalidInput( - "partial credential environment: organization ID, public key, and private key are all required".into(), - ) + let [Some(org), Some(public), Some(private)] = bundle else { + let missing: Vec<&str> = ENV_BUNDLE + .into_iter() + .zip(&bundle) + .filter_map(|(name, value)| value.is_none().then_some(name)) + .collect(); + return Err(InvalidInput(format!( + "partial credential environment: set {} too, or unset the rest", + missing.join(" and ") + )) .into()); }; let [org, public, private] = [org, public, private].map(|value| { @@ -779,6 +810,7 @@ fn resolve_environment(options: &AuthOptions) -> Result> { .map_err(|_| InvalidInput("credential environment value is not valid Unicode".into())) }); let (org, public, private) = (org?, public?, private?); + let private = Zeroizing::new(private); if org.is_empty() || public.is_empty() || private.is_empty() { return Err(InvalidInput("credential environment fields must not be empty".into()).into()); } @@ -817,12 +849,11 @@ async fn resolve_profile( pub(crate) async fn saved_profile(name: &str) -> Result { let path = registry_path()?; - load(&path) - .await? - .profiles - .remove(name) - .ok_or_else(|| profile_missing(name)) - .map_err(Into::into) + let mut registry = load(&path).await?; + match registry.profiles.remove(name) { + Some(profile) => Ok(profile), + None => Err(registry.missing(name.to_owned()).into()), + } } pub(crate) async fn remove_generated_key(path: &Path) -> bool { @@ -854,8 +885,30 @@ pub(crate) async fn remove_generated_key(path: &Path) -> bool { }) } -fn profile_missing(name: &str) -> InvalidInput { - InvalidInput(format!("profile {name} does not exist")) +impl Registry { + fn missing(self, name: String) -> ProfileNotFound { + ProfileNotFound { + name, + saved: SavedProfiles(self.profiles.into_keys().collect()), + } + } +} + +fn rejected( + error: Error, + organization_id: Uuid, + public_key: CompressedPublicKey, + credential: CredentialSource, +) -> Error { + let error = error.context("Turnkey API request failed"); + if !is_unauthorized(&error) { + return error; + } + error.context(CredentialRejected { + organization_id, + public_key, + credential, + }) } pub async fn load_gpg_keys() -> Result { @@ -980,11 +1033,18 @@ pub async fn run_auth(command: AuthCommand, options: &AuthOptions) -> Result { - let auth = resolve(options).await?; - let client = build_turnkey_client(auth.stamper, &auth.api_base_url)?; - let identity = whoami(&client, auth.org_id) + let ResolvedAuth { + org_id, + api_base_url, + stamper, + source, + http: _, + } = resolve(options).await?; + let public_key = CompressedPublicKey::from(&stamper); + let client = build_turnkey_client(stamper, &api_base_url)?; + let identity = whoami(&client, org_id) .await - .context("Turnkey API request failed")?; + .map_err(|error| rejected(error, org_id, public_key, source))?; Ok(OperationOutput::result( "auth.whoami", serde_json::to_value(identity)?, @@ -1018,11 +1078,9 @@ async fn login(args: LoginArgs, options: &AuthOptions) -> Result" - )) - .into()); + let mut saved = load(&path).await?; + let Some(profile) = saved.profiles.remove(&name) else { + return Err(saved.missing(name).into()); }; let Profile { organization_id, @@ -1045,16 +1103,22 @@ async fn login(args: LoginArgs, options: &AuthOptions) -> Result Result let path = registry_path()?; let _lock = registry_lock(&path).await?; let mut registry = load(&path).await?; - let profile = registry - .profiles - .get_mut(name) - .ok_or_else(|| profile_missing(name))?; + let Some(profile) = registry.profiles.get_mut(name) else { + return Err(registry.missing(name.to_owned()).into()); + }; let previous = mem::replace(&mut profile.api_key_file, api_key_file); save(&path, ®istry).await?; Ok(previous) @@ -1154,20 +1217,18 @@ pub async fn run_profile( json!({"activeProfile": registry.active_profile, "profiles": registry.profiles}), )), SavedProfileCommand::Show { name } => { - let profile = registry - .profiles - .get(&name) - .ok_or_else(|| profile_missing(&name))?; + let Some(profile) = registry.profiles.get(&name) else { + return Err(registry.missing(name).into()); + }; Ok(OperationOutput::result( "profile.show", json!({"name": name, "profile": profile}), )) } SavedProfileCommand::Use { name } => { - let profile = registry - .profiles - .get(&name) - .ok_or_else(|| profile_missing(&name))?; + let Some(profile) = registry.profiles.get(&name) else { + return Err(registry.missing(name).into()); + }; read_key(&profile.api_key_file).await?; registry.active_profile = Some(name.clone()); save(&path, ®istry).await?; @@ -1177,10 +1238,9 @@ pub async fn run_profile( )) } SavedProfileCommand::Delete { name } => { - registry - .profiles - .remove(&name) - .ok_or_else(|| profile_missing(&name))?; + if registry.profiles.remove(&name).is_none() { + return Err(registry.missing(name).into()); + } if registry.active_profile.as_ref() == Some(&name) { registry.active_profile = None; } @@ -1191,10 +1251,9 @@ pub async fn run_profile( )) } SavedProfileCommand::Set { name, api_key_file } => { - let profile = registry - .profiles - .get_mut(&name) - .ok_or_else(|| profile_missing(&name))?; + let Some(profile) = registry.profiles.get_mut(&name) else { + return Err(registry.missing(name).into()); + }; if let Some(organization_id) = options.organization_id { profile.organization_id = organization_id; } @@ -1206,11 +1265,7 @@ pub async fn run_profile( let current = match fs::canonicalize(&api_key_file).await { Ok(current) => current, Err(error) if error.kind() == ErrorKind::NotFound => { - return Err(InvalidInput(format!( - "credential file {} does not exist", - api_key_file.display() - )) - .into()); + return Err(MissingCredentialFile { path: api_key_file }.into()); } Err(error) => return Err(error).context("resolve credential path"), }; diff --git a/src/cli.rs b/src/cli.rs index deea3fb..af097c7 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -6,7 +6,7 @@ use std::{ process::ExitCode, }; -use anyhow::Result; +use anyhow::{Error, Result}; use clap::{ ArgAction, Args, CommandFactory, Parser, Subcommand, builder::FalseyValueParser, error::ErrorKind, @@ -16,9 +16,10 @@ use tracing::debug; use crate::{ auth::{ - self, AuthCommand, AuthOptions, LoginArgs, ProfileCommand, ResolvedAuth, + self, AuthCommand, AuthOptions, CredentialSource, LoginArgs, ProfileCommand, ResolvedAuth, SavedProfileCommand, }, + errors::{CredentialRejected, NextStep, NoSelectedIdentity, ProfileNotFound}, gpg::{self, GpgCommand}, keygen::GenerateArgs, operations::{ActivityCommand, RequestArgs, run_activity}, @@ -40,6 +41,8 @@ TK_NON_INTERACTIVE=true to fail fast instead. --message-format json prints one JSON record per line, each with a `reason` field; error records also carry a `code`. JSON output never prompts. +Before a multi-step task, read its workflow: `tk skills list`, then +`tk skills show --name NAME`. Record shapes and error codes: `tk skills show --name references/cli-convention`. Exit codes: 0 success, 1 runtime error, 2 usage error."#; @@ -229,6 +232,40 @@ async fn run_operation( emit(&mut ctx, result) } +fn with_next_step(error: Error) -> Error { + let step = if let Some(ProfileNotFound { name, saved }) = error.downcast_ref() { + let create = + format!("run tk profile create --profile-name {name} --organization-id ORG_UUID"); + if saved.0.is_empty() { + create + } else { + format!("unless {name} is a typo for a saved profile, {create}") + } + } else if let Some(NoSelectedIdentity { saved }) = error.downcast_ref() { + match saved.0.as_slice() { + [] => "run tk profile create --profile-name NAME --organization-id ORG_UUID".to_owned(), + [only] => format!("run tk login --profile-name {only}"), + _ => "run tk login --profile-name NAME with one of the saved profiles".to_owned(), + } + } else if let Some(CredentialRejected { + organization_id, + public_key, + credential, + }) = error.downcast_ref() + { + let then = match credential { + CredentialSource::Profile(name) => format!("run tk login --profile-name {name}"), + CredentialSource::Environment => "run the command again".to_owned(), + }; + format!( + "if organization {organization_id} is correct, register public key {public_key} (API_KEY_CURVE_P256) on a user there, then {then}" + ) + } else { + return error; + }; + error.context(NextStep(step)) +} + async fn run_prepared( prepared: Result

, options: &AuthOptions, @@ -250,6 +287,7 @@ fn emit(ctx: &mut StdCtx, result: Result) -> ExitCode } }, Err(error) => { + let error = with_next_step(error); debug!(?error, "command failed"); let shell = ctx.shell(); diff --git a/src/errors.rs b/src/errors.rs index 1c3ed0e..2fb609e 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,13 +1,20 @@ // This module defines ErrorCode and owns its classification. #![allow(clippy::disallowed_types)] -use std::error::Error; +use std::{ + error::Error, + fmt::{self, Display, Formatter}, + path::PathBuf, +}; use serde::Serialize; -use serde_json::{Value, json}; +use serde_json::{Map, Value, json}; use turnkey_client::TurnkeyClientError; use uuid::Uuid; -use crate::{auth::SelectedIdentity, sessions::public_key::CompressedPublicKey}; +use crate::{ + auth::{CredentialSource, SelectedIdentity}, + sessions::public_key::CompressedPublicKey, +}; #[derive(Debug, thiserror::Error)] #[error("{0}")] @@ -46,6 +53,51 @@ impl Malformed { } } +#[derive(Debug)] +pub struct SavedProfiles(pub Vec); + +impl Display for SavedProfiles { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self.0.as_slice() { + [] => f.write_str("no profiles are saved"), + names => write!(f, "saved profiles: {}", names.join(", ")), + } + } +} + +#[derive(Debug, thiserror::Error)] +#[error("profile {name} does not exist; {saved}")] +pub struct ProfileNotFound { + pub name: String, + pub saved: SavedProfiles, +} + +#[derive(Debug, thiserror::Error)] +#[error("no profile is selected and no credential environment is set; {saved}")] +pub struct NoSelectedIdentity { + pub saved: SavedProfiles, +} + +#[derive(Debug, thiserror::Error)] +#[error("credential file {} does not exist", .path.display())] +pub struct MissingCredentialFile { + pub path: PathBuf, +} + +#[derive(Debug, thiserror::Error)] +#[error( + "organization {organization_id} rejected public key {public_key}: the key is not registered on a user there, or it expired or was deleted" +)] +pub struct CredentialRejected { + pub organization_id: Uuid, + pub public_key: CompressedPublicKey, + pub credential: CredentialSource, +} + +#[derive(Debug, thiserror::Error)] +#[error("try: {0}")] +pub struct NextStep(pub String); + #[derive(Debug, thiserror::Error)] #[error("the selected identity ({identity}) belongs to organization {actual}, not {expected}")] pub struct OrganizationMismatch { @@ -157,10 +209,42 @@ pub fn error_details(error: &anyhow::Error) -> Option { "warnBeforeSeconds": expiring.warn_before_seconds, })); } - error + if let Some(activity) = error .downcast_ref::() .and_then(ActivityError::activity) - .map(|activity| json!({"activity": activity})) + { + return Some(json!({"activity": activity})); + } + let mut details = Map::new(); + if let Some(NextStep(step)) = error.downcast_ref() { + details.insert("nextStep".into(), step.as_str().into()); + } + if let Some(ProfileNotFound { name, saved }) = error.downcast_ref() { + details.insert("profile".into(), name.as_str().into()); + details.insert("profiles".into(), saved.0.clone().into()); + } + if let Some(NoSelectedIdentity { saved }) = error.downcast_ref() { + details.insert("profiles".into(), saved.0.clone().into()); + } + if let Some(MissingCredentialFile { path }) = error.downcast_ref() { + details.insert("path".into(), path.display().to_string().into()); + } + if let Some(CredentialRejected { + organization_id, + public_key, + credential, + }) = error.downcast_ref() + { + details.insert("organizationId".into(), organization_id.to_string().into()); + details.insert("publicKey".into(), public_key.to_string().into()); + match credential { + CredentialSource::Profile(name) => { + details.insert("profile".into(), name.as_str().into()); + } + CredentialSource::Environment => {} + } + } + (!details.is_empty()).then_some(Value::Object(details)) } const MAX_ERROR_MESSAGE_BYTES: usize = 8 * 1024; @@ -214,10 +298,14 @@ pub fn classify(error: &anyhow::Error) -> Classification { if cause.downcast_ref::().is_some() || cause.downcast_ref::().is_some() || cause.downcast_ref::().is_some() + || cause.downcast_ref::().is_some() { return Classification::new(ErrorCode::InvalidInput, None); } - if cause.downcast_ref::().is_some() { + if cause.downcast_ref::().is_some() + || cause.downcast_ref::().is_some() + || cause.downcast_ref::().is_some() + { return Classification::new(ErrorCode::NotFound, None); } if cause.downcast_ref::().is_some() { diff --git a/src/operations.rs b/src/operations.rs index acd42ae..a8d2ebd 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -216,6 +216,23 @@ impl Status { } } +fn drop_voter_users(activity: &mut Value) { + if let Some(Value::Array(votes)) = activity.get_mut("votes") { + for vote in votes.iter_mut().filter_map(Value::as_object_mut) { + vote.remove("user"); + } + } +} + +impl OperationOutput { + fn without_voter_users(mut self) -> Self { + if let Some(activity) = self.data.get_mut("activity") { + drop_voter_users(activity); + } + self + } +} + impl Serialize for OperationOutput { fn serialize(&self, serializer: S) -> Result { let mut record = serializer @@ -533,7 +550,7 @@ pub async fn run_activity(args: ActivityCommand, auth: &ResolvedAuth) -> Result< ActivityCommand::List(args) => list(auth, args).await, ActivityCommand::Get { id } => { let value = query_activity(auth, &id.to_string()).await?; - Ok(OperationOutput::result("activity.get", value)) + Ok(OperationOutput::result("activity.get", value).without_voter_users()) } ActivityCommand::Wait { id, timeout } => wait(auth, id, timeout).await, ActivityCommand::Approve { id } => vote(auth, id, true).await, @@ -615,6 +632,9 @@ async fn list(auth: &ResolvedAuth, args: ListArgs) -> Result { pagination.after = activity_id(last)?.to_owned(); } } + for item in &mut items { + drop_voter_users(item); + } let next = if capped { items.last().map(activity_id).transpose()?.map(Value::from) } else { @@ -658,7 +678,7 @@ async fn wait(auth: &ResolvedAuth, id: Uuid, seconds: u64) -> Result { let output = OperationOutput::result(command, value); if !output.is_pending() { - return terminal(output, false); + return Ok(terminal(output, false)?.without_voter_users()); } last = output.activity; } @@ -750,11 +770,12 @@ async fn vote(auth: &ResolvedAuth, id: Uuid, approve: bool) -> Result( &format!("/public/v1/submit/{endpoint}"), )?; let value = post::(auth, endpoint, body, true).await?; - submission_result(command, value) + Ok(submission_result(command, value)?.without_voter_users()) } pub(crate) async fn query( diff --git a/tests/auth_command.rs b/tests/auth_command.rs index 2e3300f..65abc9b 100644 --- a/tests/auth_command.rs +++ b/tests/auth_command.rs @@ -199,6 +199,65 @@ async fn typed_client_http_status_is_classified_end_to_end() { assert!(parsed["message"].as_str().unwrap().contains("no such user")); } +#[test] +fn missing_and_malformed_credential_files_fail_differently() { + let temp = TempDir::new().unwrap(); + registry(&temp); + let directory = temp.path().join(".config/turnkey"); + fs::remove_file(directory.join("agent.json")).unwrap(); + let agent_key = format!("{}/agent.json", directory.display()); + let missing = failure( + command(&temp).args(["--profile", "agent", "auth", "status"]), + 1, + ); + assert_eq!( + missing, + json!({ + "reason": "command_error", + "code": "not_found", + "details": {"path": agent_key}, + "message": format!("credential file {agent_key} does not exist"), + }) + ); + + fs::write(directory.join("admin.json"), "not json").unwrap(); + let malformed = failure( + command(&temp).args(["--profile", "admin", "auth", "status"]), + 1, + ); + assert_eq!(malformed["code"], "invalid_input"); + assert_eq!(malformed.get("details"), None); +} + +#[test] +fn an_unselected_identity_names_the_next_step() { + let temp = TempDir::new().unwrap(); + let none_saved = failure(command(&temp).arg("whoami"), 1); + assert_eq!( + none_saved, + json!({ + "reason": "command_error", + "code": "invalid_input", + "details": { + "nextStep": "run tk profile create --profile-name NAME --organization-id ORG_UUID", + "profiles": [], + }, + "message": "try: run tk profile create --profile-name NAME --organization-id ORG_UUID: no profile is selected and no credential environment is set; no profiles are saved", + }) + ); + + let partial = failure( + command(&temp) + .env("TURNKEY_API_PRIVATE_KEY", "01") + .arg("whoami"), + 1, + ); + assert_eq!( + partial["message"], + "partial credential environment: set TURNKEY_ORGANIZATION_ID and TURNKEY_API_PUBLIC_KEY too, or unset the rest" + ); +} + #[test] fn invalid_key_length_is_an_error_without_panic() { let temp = TempDir::new().unwrap(); @@ -263,13 +322,6 @@ fn profile_create_and_login_reject_local_mismatches() { let empty_login = failure(command(&temp).args(["login", "--profile-name", ""]), 2); assert_eq!(empty_login["code"], "usage_error"); - let missing_profile = failure(command(&temp).arg("login"), 1); - assert_eq!(missing_profile["code"], "invalid_input"); - assert_eq!( - missing_profile["message"], - "profile default does not exist; run tk profile create --profile-name default --organization-id " - ); - output(command(&temp).args(["--organization-id", ORG, "profile", "create"])); let duplicate = failure( diff --git a/tests/e2e/activities.rs b/tests/e2e/activities.rs index c0e5819..f4169b8 100644 --- a/tests/e2e/activities.rs +++ b/tests/e2e/activities.rs @@ -24,6 +24,11 @@ fn activity_list_paginates_and_get_and_wait_inspect_a_completed_activity() { json!({"id": activity_id, "status": "ACTIVITY_STATUS_COMPLETED"}) ); assert_eq!(got["data"]["activity"]["id"], activity_id); + assert_eq!( + votes(&got), + [(run.root_user_id(), "VOTE_SELECTION_APPROVED")], + "{got}" + ); let waited = run.ok(run.admin().args(["activity", "wait", "--id", &activity_id])); assert_eq!(waited["command"], "activity.wait"); @@ -234,6 +239,9 @@ fn activity_list_filters_by_status_type_and_since() { for item in by_completed["data"]["items"].as_array().unwrap() { assert_eq!(item["status"], "ACTIVITY_STATUS_COMPLETED", "{item}"); assert!(item["votes"].is_array(), "{item}"); + for vote in item["votes"].as_array().unwrap() { + assert_eq!(vote.get("user"), None, "{item}"); + } } let by_type = run.ok(run.admin().args([ @@ -297,6 +305,28 @@ fn votes(record: &Value) -> Vec<(&str, &str)> { .unwrap() .iter() .map(|vote| { + let mut keys: Vec<&str> = vote + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect(); + keys.sort_unstable(); + assert_eq!( + keys, + [ + "activityId", + "createdAt", + "id", + "message", + "publicKey", + "scheme", + "selection", + "signature", + "userId", + ], + "{vote}" + ); ( vote["userId"].as_str().unwrap(), vote["selection"].as_str().unwrap(), diff --git a/tests/e2e/identity.rs b/tests/e2e/identity.rs index 116b887..ec095d3 100644 --- a/tests/e2e/identity.rs +++ b/tests/e2e/identity.rs @@ -158,8 +158,19 @@ fn login_creates_registry_and_profile_commands_behave() { "--organization-id", org, ])); - assert_eq!(unknown["code"], "invalid_input"); - assert_eq!(unknown["message"], "profile no-such-profile does not exist"); + assert_eq!( + unknown, + json!({ + "reason": "command_error", + "code": "not_found", + "details": { + "nextStep": "unless no-such-profile is a typo for a saved profile, run tk profile create --profile-name no-such-profile --organization-id ORG_UUID", + "profile": "no-such-profile", + "profiles": [name], + }, + "message": format!("try: unless no-such-profile is a typo for a saved profile, run tk profile create --profile-name no-such-profile --organization-id ORG_UUID: profile no-such-profile does not exist; saved profiles: {name}"), + }) + ); let malformed = run.err(run.cli().args([ "profile", "set", @@ -293,6 +304,16 @@ fn login_with_unregistered_credential_fails_and_selects_nothing() { let error = run.err(run.cli().args(["login", "--profile-name", &name])); assert_eq!(error["reason"], "command_error"); assert_unauthenticated(&error); + let public_key = hex::encode(key.compressed_public_key()); + assert_eq!( + error["details"], + json!({ + "nextStep": format!("if organization {} is correct, register public key {public_key} (API_KEY_CURVE_P256) on a user there, then run tk login --profile-name {name}", run.org()), + "organizationId": run.org(), + "publicKey": public_key, + "profile": name, + }) + ); assert_eq!( run.ok(run.cli().args(["profile", "list"]))["data"]["activeProfile"], Value::Null @@ -347,9 +368,35 @@ fn profile_create_generates_a_credential_that_logs_in_once_registered() { let unregistered = run.err(run.cli().args(["login", "--profile-name", &name])); assert_unauthenticated(&unregistered); + let unselected = run.err(run.cli().args(["auth", "status"])); + assert_eq!( + unselected, + json!({ + "reason": "command_error", + "code": "invalid_input", + "details": { + "nextStep": format!("run tk login --profile-name {name}"), + "profiles": [name], + }, + "message": format!("try: run tk login --profile-name {name}: no profile is selected and no credential environment is set; saved profiles: {name}"), + }) + ); + let missing = run.err( + run.cli() + .args(["login", "--profile-name", "no-such-profile"]), + ); assert_eq!( - run.err(run.cli().args(["auth", "status"]))["code"], - "invalid_input" + missing, + json!({ + "reason": "command_error", + "code": "not_found", + "details": { + "nextStep": "unless no-such-profile is a typo for a saved profile, run tk profile create --profile-name no-such-profile --organization-id ORG_UUID", + "profile": "no-such-profile", + "profiles": [name], + }, + "message": format!("try: unless no-such-profile is a typo for a saved profile, run tk profile create --profile-name no-such-profile --organization-id ORG_UUID: profile no-such-profile does not exist; saved profiles: {name}"), + }) ); let (user_id, _) = run.create_user("fresh-login"); @@ -445,7 +492,16 @@ fn profile_set_switches_the_credential_file() { ]) .arg(run.home().join("absent.json")), ); - assert_eq!(missing["code"], "invalid_input", "{missing}"); + let absent = run.home().join("absent.json").display().to_string(); + assert_eq!( + missing, + json!({ + "reason": "command_error", + "code": "not_found", + "details": {"path": absent}, + "message": format!("credential file {absent} does not exist"), + }) + ); assert_eq!( run.ok(run .cli() diff --git a/tests/e2e/sessions.rs b/tests/e2e/sessions.rs index 86be483..e1c0829 100644 --- a/tests/e2e/sessions.rs +++ b/tests/e2e/sessions.rs @@ -75,7 +75,7 @@ fn session_request_generates_a_pending_credential() { run.cli() .args(["session", "request", "--profile-name", "no-such-profile"]), ); - assert_eq!(unknown["code"], "invalid_input", "{unknown}"); + assert_eq!(unknown["code"], "not_found", "{unknown}"); } #[test]