feat(provider): add Passbolt provider via go-passbolt-cli#127
Conversation
Add a `passbolt://` provider backed by the official go-passbolt-cli, for
the self-hosted Passbolt password manager. Convention secrets map to a
resource named `secretspec/{project}/{profile}/{key}` with the value in
the `password` field; a secret's `ref` can instead point at an existing
resource by id or name and pick a field (password/username/uri/description).
Credentials come either from the CLI's own configuration or from
secretspec-owned env vars (SECRETSPEC_PASSBOLT_SERVER,
SECRETSPEC_PASSBOLT_PRIVATE_KEY_FILE / SECRETSPEC_PASSBOLT_PRIVATE_KEY,
SECRETSPEC_PASSBOLT_PASSPHRASE) that the provider forwards to the CLI —
the passphrase and inline key via the child's environment, never the
argv — so no credentials live in secretspec.toml or the provider URI.
Includes docs (provider page, reference, concepts, quick-start) and a
CHANGELOG entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
domenkozar
left a comment
There was a problem hiding this comment.
Thanks for the contribution! I ran a deep review of this PR (multi-agent, each finding independently verified against the code, the go-passbolt-cli/go-passbolt sources, and where possible the v0.5.1 binary). Inline comments below, roughly ordered by severity in each file.
One finding has no diff line to anchor on, so it goes here:
docs: reference/configuration.md secret-references tables not extended. The Secret References section of docs/src/content/docs/reference/configuration.md has a store-shape table and a per-provider "How providers interpret the coordinates" table listing every ref-capable provider. Passbolt is missing from both, so a user consulting the page that passbolt.md itself links to for ref semantics would conclude refs are unsupported.
| let coords = self.resolve_coords(addr)?; | ||
| let field = validate_field(coords.field.as_deref().unwrap_or(DEFAULT_FIELD))?; | ||
| let flag = Self::field_flag(field); | ||
| let secret = value.expose_secret(); |
There was a problem hiding this comment.
security: set() puts the secret value itself on the child argv: --password <secret> on both the update path (line 463) and the create path (line 478). The value is world readable via ps / /proc/<pid>/cmdline for the full lifetime of the CLI call, which spans a complete server login. The pass, lastpass, and protonpass providers pipe secret values via stdin for exactly this reason (lastpass.rs documents it explicitly).
go-passbolt-cli currently accepts the value only as a flag (no stdin/env channel in internal/cmd/resource create/update), so this may be unavoidable today. In that case it should at least be a documented limitation: the module docs and provider page currently advertise argv hygiene ("via the child's environment, never the argv") that covers the auth passphrase but not the secret value itself.
| ))); | ||
| } | ||
|
|
||
| let mut args = vec!["create", "resource", "--name", &coords.item, &flag, secret]; |
There was a problem hiding this comment.
correctness: a name-based ref whose name resolves to nothing falls through to this create path and silently creates a brand new detached resource. The is_uuid guard above only protects id refs, but the docs page promises "secretspec never creates a detached resource for an externally managed reference", and its own example uses a name ref (item = "Payments service account").
Concretely: the referenced resource gets renamed or isn't shared with this user yet, then secretspec set creates an orphan duplicate instead of erroring. With field = "username" / "uri" / "description" it's worse: the create runs without --password and go-passbolt-cli fails with the unrelated "required flag "password" not set".
At this point in set() a convention name and a user-authored ref name are indistinguishable, so the fix is to decide before resolve_coords collapses them, e.g. match on Address::Native like onepassword.rs does (its refs never create, for id or name).
| /// Resource fields a native `ref` may address via its `field` coordinate. These | ||
| /// are the standard Passbolt resource attributes; `password` is the secret, the | ||
| /// rest are metadata that a `ref` can still pin (e.g. reading a stored `uri`). | ||
| const KNOWN_FIELDS: &[&str] = &["password", "username", "uri", "description", "name"]; |
There was a problem hiding this comment.
security: KNOWN_FIELDS includes "name", but every doc surface (provider page, reference, CHANGELOG, the rustdoc below) limits field to password/username/uri/description. Nothing upstream validates coordinate values, so ref = { item = "...", field = "name" } passes validate_field and makes set() run passbolt update resource --id X --name <secret>, renaming the resource to the secret value. For v4 resource types (still the common default) names are cleartext server-side metadata, so that's a secret leak, and it breaks any subsequent lookup by the old name either way. On the create path the duplicate --name still puts the secret on the argv, though the CLI then fails its required-password check before creating anything.
Dropping "name" from KNOWN_FIELDS fixes the write hazard; find_id_by_name only needs the struct field, not the coordinate.
| /// HTTP 404 wrapped in the `getting resource:` context. | ||
| fn is_not_found(msg: &str) -> bool { | ||
| let lower = msg.to_lowercase(); | ||
| lower.contains("404") |
There was a problem hiding this comment.
correctness: matching "404" / "not found" / "does not exist" / "could not find" anywhere in the lowercased stderr is too broad for the one place that converts errors into Ok(None) (get_resource). go-passbolt formats every API error as API error (code %d): %s, so a wrong ?server= base path that 404s, a revoked share (the Passbolt API intentionally answers 404 with "The resource does not exist." for unauthorized resources), and go-passbolt's sentinel errors resource type not found / metadata key not found all get masked as "secret not set" (the masking hits id-addressed lookups; name lookups error loudly in find_id_by_name). secretspec check then prompts the user to re-enter values against a broken or unauthorized setup instead of surfacing the real error.
The in-tree precision bar is exact tool phrases (onepassword.rs matches "isn't an item" / "doesn't have a field"); matching the specific composed shape here (e.g. getting resource plus code 404) would keep unrelated failures loud.
| // Update the resource in place when it already exists (by id or by | ||
| // resolved name); this is the path a human-provisioned resource takes. | ||
| if let Some(id) = self.resolve_id(&coords.item)? { | ||
| self.run(&["update", "resource", "--id", &id, &flag, secret])?; |
There was a problem hiding this comment.
correctness: setting an empty value on an existing resource is a silent success no-op. go-passbolt's helper.UpdateResource skips empty-string fields by design ("Empty strings are not applied (partial update)"), so the CLI exits 0 while keeping the old value, and set() reports success without persisting anything.
secretspec's CLI set path rejects empty values before reaching the provider, but two flows deliver an empty string today: secretspec check's interactive prompt (no empty guard before backend.set()) and import. Combined with the read side filtering empty fields to None (line 57), a resource whose password is empty makes check never converge: it reports the secret missing, prompts, saves a no-op, and the re-validation in the same run (and every later run) reports it missing again. Worth rejecting empty values here (or documenting the CLI limitation) so set can't return Ok without writing.
| uri | ||
| } | ||
|
|
||
| fn get(&self, addr: Address<'_>) -> Result<Option<SecretString>> { |
There was a problem hiding this comment.
efficiency: without a get_many override, every convention secret pays a full-account passbolt list resource (each CLI spawn redoes the whole login) plus a get resource. secretspec run with N secrets fires N identical account-wide listings concurrently, so 20 secrets against an account with a few thousand resources means 40 process spawns, 40 full logins, and 19 redundant listings (folder-scoped when ?folder= is set). onepassword, protonpass, bws, and awssm all override get_many with one listing plus parallel fetches for this reason.
A get_many here could also serve username/uri ref fields straight from the listing (those columns don't require secret decryption), keeping the second spawn only for password/description.
| cli_binary_path: String, | ||
| } | ||
|
|
||
| crate::register_provider! { |
There was a problem hiding this comment.
consistency: the other remote CLI providers (onepassword, protonpass, lastpass) register a preflight: probe in register_provider! plus an auth_scope_key override, so a locked or misconfigured CLI fails once with one clear error before any per-secret work. Without them, a bad passphrase and 10 secrets means 10 concurrent CLI processes each attempting a full login: 10 duplicate raw errors and 10 wasted logins, plus noise for any rate limiting or fail2ban rules in front of the server. A cheap probe wired into preflight would match the sibling behavior.
| args.push(folder); | ||
| } | ||
| let output = self.run(&args)?; | ||
| let resources: Vec<PassboltResource> = serde_json::from_str(&output) |
There was a problem hiding this comment.
nit/reuse: SecretSpecError already has Json(#[from] serde_json::Error), so this map_err (and the one in get_resource below) can be plain ?, like onepassword.rs's item decode. That also classifies these failures as json for SecretSpecError::kind() instead of provider_operation_failed.
| STRIPE_SECRET_KEY = { description = "Stripe key", ref = { item = "a9230ec4-5507-4870-b8b5-b3f500587e4c" }, providers = ["passbolt://?server=https://pass.example.com"] } | ||
|
|
||
| # By resource name, reading a non-default field | ||
| SERVICE_USER = { description = "Service account user", ref = { item = "Payments service account", field = "username" }, providers = ["passbolt"] } |
There was a problem hiding this comment.
docs: this example fails at runtime. A bare name in a per-secret providers chain must be a declared alias (project [providers] table or user config), not a provider name; only full scheme:// URIs pass through directly. Copying this line as-is yields "Provider alias 'passbolt' is not defined". Either use providers = ["passbolt://"] like the STRIPE example above effectively does, or add the [providers] alias table to the example.
| { label: "Environment Variables", slug: "providers/env" }, | ||
| { label: "Pass", slug: "providers/pass" }, | ||
| { label: "Proton Pass", slug: "providers/protonpass" }, | ||
| { label: "Passbolt", slug: "providers/passbolt" }, |
There was a problem hiding this comment.
docs drift: the sidebar entry is here, but the other provider listings from the CLAUDE.md "Adding Provider Documentation" checklist were missed:
- the providers sentence in the
starlightLlmsTxtdescription block in this file (still ends at "Bitwarden Secrets Manager") docs/src/pages/index.astro: theproviderMetadataarray has no passbolt entry, and the page's "11 backends" counts and provider enumerations are now staleREADME.md: the Providers bullet list and thesecretspec config initexample output (quick-start.mdx got the new line, the README didn't)
After this lands, llms.txt, the landing page, and the GitHub README all advertise a provider list that no longer matches what the binary prints.
There was a problem hiding this comment.
Excellent work, thanks. I had done the pull request to document the possibility of using secretspec for Passbolt, but I'll take up the challenge and go through these implementation drawbacks and make it proper.
And many thanks for secretspec, it's awesome. Have been using it for every project since becoming aware of it!
Add a
passbolt://provider backed by the official go-passbolt-cli, for the self-hosted Passbolt password manager. Convention secrets map to a resource namedsecretspec/{project}/{profile}/{key}with the value in thepasswordfield; a secret'srefcan instead point at an existing resource by id or name and pick a field (password/username/uri/description).Credentials come either from the CLI's own configuration or from secretspec-owned env vars (SECRETSPEC_PASSBOLT_SERVER, SECRETSPEC_PASSBOLT_PRIVATE_KEY_FILE / SECRETSPEC_PASSBOLT_PRIVATE_KEY, SECRETSPEC_PASSBOLT_PASSPHRASE) that the provider forwards to the CLI — the passphrase and inline key via the child's environment, never the argv — so no credentials live in secretspec.toml or the provider URI.
Includes docs (provider page, reference, concepts, quick-start) and a CHANGELOG entry.