Skip to content

feat(provider): add Passbolt provider via go-passbolt-cli#127

Open
Tails wants to merge 1 commit into
cachix:mainfrom
ParapluOU:feat/passbolt-provider
Open

feat(provider): add Passbolt provider via go-passbolt-cli#127
Tails wants to merge 1 commit into
cachix:mainfrom
ParapluOU:feat/passbolt-provider

Conversation

@Tails

@Tails Tails commented Jul 10, 2026

Copy link
Copy Markdown

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.

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 domenkozar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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"];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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])?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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>> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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! {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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"] }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Comment thread docs/astro.config.ts
{ label: "Environment Variables", slug: "providers/env" },
{ label: "Pass", slug: "providers/pass" },
{ label: "Proton Pass", slug: "providers/protonpass" },
{ label: "Passbolt", slug: "providers/passbolt" },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 starlightLlmsTxt description block in this file (still ends at "Bitwarden Secrets Manager")
  • docs/src/pages/index.astro: the providerMetadata array has no passbolt entry, and the page's "11 backends" counts and provider enumerations are now stale
  • README.md: the Providers bullet list and the secretspec config init example 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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants