feat(sdk): expose Secrets::config() publicly (hidden) by djbclark · Pull Request #357 · cachix/secretspec

djbclark · 2026-08-16T00:36:04Z

Summary

Secrets::config() was pub(crate), gated behind
#[cfg(any(feature = "cli", test))], for secretspec codegen's use. Any
other embedder needing the manifest without a provider — an out-of-tree
plugin inspecting declared secrets, for instance (#64) — had no way to reach
it short of taking the whole cli feature or re-parsing the manifest itself.

Change

config() is now #[doc(hidden)] pub and unconditional. #[doc(hidden)]
keeps it off the public SDK surface (ordinary callers resolve secrets, not
manifests) while making it reachable. This is additive and cannot break the
C ABI the FFI bindings expose.

Validation

cargo check -p secretspec --all-features
cargo check -p secretspec --no-default-features
cargo test --package secretspec --all-features -- secrets:: — 37 passed
cargo fmt --all -- --check
cargo clippy -p secretspec --no-default-features --features cli -- -D warnings — no new warnings

config() was pub(crate), gated behind #[cfg(any(feature = "cli", test))], for secretspec codegen's use. Any other embedder needing the manifest without a provider -- an out-of-tree plugin inspecting declared secrets, for instance -- had no way to reach it short of taking the whole cli feature or re-parsing the manifest itself. Changed to #[doc(hidden)] pub: additive, so it cannot break the C ABI the FFI bindings expose, and hidden from the public SDK surface since ordinary callers resolve secrets rather than manifests.

domenkozar · 2026-08-16T00:55:36Z

Needs to check it doesn't conflict with #334

djbclark · 2026-08-16T02:05:39Z

Thanks for the pointer, @domenkozar — checked #334 against this directly rather than just against the diff.

Merging #334 into a branch with this PR is clean: Secrets::config() auto-merges with no conflict, only a trivial CHANGELOG.md text overlap. #334 does move Config behind __private though, so I also checked the actual blast radius against a downstream CLI I maintain that embeds secretspec as a library and calls config() for JSON Schema emission.

Turned out small. secretspec-derive is already updated by #334 itself — no work needed there. The one real change needed was swapping a &Config-based schema build for the public Spec::load_from(...) + codegen::build_ir(&spec) path, which is arguably cleaner than what it replaces. I also hit one genuine conflict of the kind you're asking about here: #334 renames the internal build_ir helper, and a test added by #355 still called the old signature — confirmed and fixed.

Diff of the concrete fix, if useful: frdminc@bf0b25c

cargo test --all-features passes clean with #334 + this PR + #355 + #356 all merged together.

domenkozar · 2026-08-16T17:06:29Z

@djbclark I mean that the public api will be Spec and Spec::from(path) instead of the internal Config.

domenkozar · 2026-08-16T19:35:50Z

closing in favor of #334, please open an issue with your use case if that doesn't fit

Records every thread this fork has opened or spoken in on cachix/secretspec, with the gh commands to re-check them. Surfaced that issue cachix#64 (out-of-tree providers via gRPC) is our only open upstream thread -- we closed our own cachix#345 as a duplicate of it, so it now carries the fork's entire exec:// provider-plugin interest, and nothing was tracking it. Also records the two upstream items we owe but have not filed: - Spec has no path back to TOML. The maintainer closed cachix#356 and cachix#357 sixty seconds apart with the same sentence and an explicit statement that the public API will be Spec rather than the internal Config, so the draft lands the capability on Spec rather than re-proposing a free-function module a third time. - check writes its entire report to stderr. Reproduces identically on upstream main; secrets.rs is upstream-owned, so a fork-local-only fix would be a permanent merge-conflict site. Adds a "shape debt" section for the deliberate __private stopgap that source-schema will take, so the ugliness is revisited when upstream answers rather than forgotten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Records the cachix#334 consolidation and what it means for the fork: cachix#356 and cachix#357 both closed in favor of it, the maintainer's stated direction that the public API is Spec rather than the internal Config, and the decision to reshape our manifest editing onto Spec rather than re-propose a free-function module a third time. Captures the constraints that make the remaining work predictable -- Spec cannot introspect declarations, so the upstream merge will break emit_schema and __private is the only door today -- plus the traps that cost time this session: cargo test --all cannot run without a PHP toolchain, and piping cargo through tail hides its exit code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

87 upstream commits, carrying the cachix#334 Rust-first `Spec` API and the cachix#313 Azure App Configuration provider. Upstream closed cachix#356 and cachix#357 in favor of cachix#334, stating the public API will be `Spec` rather than the internal `Config`, so the fork is reshaped to match rather than arguing the point. Conflict resolutions: - `provider/mod.rs`: upstream split it into address/credentials/factory/path/ preflight/registry/runtime/traits/url. Our only delta there was `supports_delete` (PR cachix#354), which upstream merged — verified present in traits.rs, the `Arc` impl, and preflight.rs — so the file is now identical to upstream. - Eight provider backends had `supports_delete` twice, ours auto-merged alongside upstream's merged copy of the same PR. Taken at upstream's exact text, removing the duplicates and eight future conflict sites. - `test.yml`: upstream moved docs checks into their own job and dropped devenv from `tests`. Took their structure; kept only the fork's `sudo-secretspec-cli` exclusion, extended to Clippy since that step is Linux-only and the crate does not build there. - `CHANGELOG.md`: both sides opened an `## [Unreleased]`. Merged subsection-by-subsection, upstream first within each; no bullet lost. `emit_schema` broke as predicted. `build_ir` now takes `&Spec` and lives under `__private`, and `codegen::schema` is `pub(crate)` + cli-gated, so there is no supported path to schema emission for a library consumer. Resolved with a `codegen-schema` feature mirroring the existing `manifest-edit` one, and the broker now loads a `Spec` from the protected manifest path instead of reaching for `Secrets::config()`. That let `secrets.rs` revert to exact upstream parity, retiring a permanent conflict site on a file upstream touches often. The `__private` dependency is a deliberate stopgap; the clean shape is a `Spec::schema_json(profile)` method upstream, tracked in sudo-secretspec/UPSTREAM-CONTACT.md under "shape debt". Also folded two doc comments orphaned by the earlier manifest_edit extraction back into the functions they document. Tests: 1583 passed, 21 failed — all `provider::sops::*`, every one reporting "The 'sops' CLI is not installed". Baseline before the merge was 1213/22. Clippy warning count is identical to upstream's own baseline (16 = 16). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…act ledger Posted both asks the maintainer invited on cachix#356/cachix#357, kept as two issues per operator decision so the focused one is not diluted: - cachix#370 — format-preserving single-declaration edits on `Spec` - cachix#371 — `Spec::schema_json(profile)`, no library path to a JSON Schema Fact-checked every claim in the to_toml draft against dfa4b10 before posting and corrected two: `generate_toml_with_comments` is at cli/mod.rs:480, not :479, and `SpecBuilder::build()` does not simply end at `from_config_document` — it also sets `base_dir`. Everything else held (add_secret_to_manifest's 4-arg signature at :618, `toml_edit = "0.23"` gated behind `cli`, `Spec` having no `to_toml`/`Display`/`toml_edit`, `Config.profiles` being a `HashMap`, `from_toml` rejecting `extends`). Ledger: both issues moved into the open table; the "owed" section now holds only the `check` stdout report, whose fix has landed locally but is not yet offered upstream. Corrected the shape-debt entry, which described a fix we did not ship: `build_ir` is genuinely reachable via `__private`, but `schema::emit` is `pub(crate)` + cli-gated and needs a fork-local `codegen-schema` feature, so the debt is a patch on upstream internals rather than mere use of a disclaimed surface. Recorded that the broker no longer touches `Secrets::config()`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

domenkozar marked this pull request as draft August 16, 2026 17:24

domenkozar closed this Aug 16, 2026

This was referenced Aug 17, 2026

Format-preserving single-declaration edits on Spec — following up on your note in #356/#357 #370

Open

No supported path from Spec to a JSON Schema — secretspec schema has no library equivalent #371

Open

Navigation Menu

Uh oh!

feat(sdk): expose Secrets::config() publicly (hidden) - #357

feat(sdk): expose Secrets::config() publicly (hidden)#357
djbclark wants to merge 1 commit into
cachix:mainfrom
frdminc:upstream-pr/secrets-config-visibility

djbclark commented Aug 16, 2026

Uh oh!

domenkozar commented Aug 16, 2026

Uh oh!

djbclark commented Aug 16, 2026

Uh oh!

domenkozar commented Aug 16, 2026

Uh oh!

domenkozar commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

djbclark commented Aug 16, 2026

Summary

Change

Validation

Uh oh!

domenkozar commented Aug 16, 2026

Uh oh!

djbclark commented Aug 16, 2026

Uh oh!

domenkozar commented Aug 16, 2026

Uh oh!

domenkozar commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants