Format-preserving single-declaration edits on Spec

[djbclark]

Closes #370.

Opening this against the shape you described on #357 — everything lands on Spec, Config stays internal, and no toml_edit or Config appears in any public signature.

What it does

Spec can read a secretspec.toml and can build a new one, but cannot write an existing one back out. So editing one declaration in a real file means either regenerating the whole document from the parsed model — losing every comment and reordering every profile, since Config.profiles is a HashMap — or hand-rolling toml_edit outside the library.

Five methods, the surface proposed in the issue:

impl Spec {
    pub fn add_secret_to_text(&self, profile: &str, name: &str, secret: Secret) -> Result<Spec>;
    pub fn remove_secret_from_text(&self, profile: &str, name: &str) -> Result<Spec>;
    pub fn declares_secret_in_text(&self, profile: &str, name: &str) -> bool;
    pub fn preserved_text(&self) -> Option<&str>;
    pub fn to_toml(&self) -> Result<String>;
}

Spec::from_toml(s)?.preserved_text() == Some(s), and add-then-remove of the same key returns the original bytes.

How it stays safe

The edits are toml_edit surgery on the retained text; config/compiled are then re-derived by reparsing the result through the same validated path every other Spec already goes through. The semantic view is always derived from the text, never hand-mutated alongside it — one synchronization point instead of one per method, so the two cannot disagree, and an edit that would not validate fails at the edit rather than at some later load.

Per the issue, SpecBuilder's general edit surface is not made format-preserving. into_builder()/to_builder() stays a hard boundary: only Spec carries source text.

The two wrinkles I flagged, and how they're handled

1. extends. Spec::from_toml rejects a non-empty project.extends, having nowhere to resolve paths from, so reparsing edited text through it would fail on every inheriting project. Config::from_text_in is the extends-aware variant, seeded with the base_dir Spec already records. ConfigGraphLoader gained an in-memory entry point, and its extends walk is now shared with the path-based one rather than duplicated.

2. Root file only. Config::try_from folds parents into the child, so retaining the merged text and writing it back would silently inline every inherited declaration into a file that had merely referenced them. A test asserts the parent's declaration does not appear in the child's text while secrets() still sees it.

Module and feature

The toml_edit surgery moves out of cli into a manifest_edit module behind a new manifest-edit feature, which cli enables — nothing changes for existing users, but an embedder can take the editing surface without clap and inquire.

toml_edit gains its serde feature so a whole Secret renders through the same representation the parser reads, keeping written and accepted keys from drifting as the schema grows. The value serializer refuses nested tables — which ref, refs, extract, generate and a presence group's required all produce — so declarations serialize via to_document and are flattened to an inline table. Cargo.lock gains only serde_core and serde_spanned, both already in the tree via toml; no new crates.

Requiredness is untouched. As I said on the issue, #334 settled it — add_secret_to_manifest keeps its existing description-only signature, and anything richer goes through the Secret that add_secret_to_text already takes.

Two deviations from the issue's sketch, both deliberate

• to_toml returns Result<String>, not String. TOML serialization is fallible and I would rather surface that than unwrap inside the library.
• The three editing methods are #[cfg(feature = "manifest-edit")], since they need toml_edit. preserved_text and to_toml are unconditional.

Happy to change either, or the naming, if you'd prefer something else.

Tests

New tests cover the byte-exact round-trip; that comments, declaration order, and a full [profiles.x.NAME] table all survive an edit; that the returned Spec is revalidated rather than merely rewritten, and that the spec it derived from is untouched; that an invalid edit fails at the edit; that a declaration carrying providers, as_path and a nested ref round-trips; duplicate-add and absent-remove errors; declares_secret_in_text parsed rather than substring-matched, so a name inside a comment or another secret's description does not fool it; and a builder-built spec reporting no text rather than inventing one.

Four more cover inheritance against real files on disk: root-only retention, editing a spec that extends another, an inherited declaration not being editable in the child, and the round-trip holding through inheritance.

Full suite green on this branch. The 21 provider::sops::* failures in my environment are all The 'sops' CLI is not installed and occur identically on an unmodified dfa4b10.

[djbclark]

I've put this API through a real consumer before review rather than after, so
here's a dogfooding report from a downstream fork that maintains a
root-privileged manifest editor.

Context: the consumer is a privilege-separated broker that owns a
root-owned secretspec.toml in a protected vault. Its add/undeclare verbs
were calling the old manifest_edit helpers directly. I ported them to the
Spec surface this PR proposes.

What worked without adjustment

• Secret::new / Secret::required / Secret::optional turn out to express
  exactly the tri-state I had been passing around as Option<bool> — leave
  requiredness to the profile default, or pin it either way. I had assumed
  adopting the Secret-shaped API would cost me that distinction, and it
  doesn't. Worth noting for anyone else weighing the same migration.
• The byte-exactness holds under the test I care about most: add a declaration
  and remove it again, and the document is restored byte for byte. My undo path
  compares manifests as bytes, so a round trip that merely preserved meaning
  would be useless to me. This is now asserted downstream.
• Revalidating the edited document as a whole is a real improvement for a
  privileged writer: a declaration that would not load is refused before
  anything reaches the vault, instead of surfacing at the next check.

Spec::from_toml is load-bearing for a privileged editor, somewhat by accident

It's the constructor I want in a root process specifically because it never
touches the filesystem and refuses project.extends — so an edit cannot be
induced into reading whatever a parent path points at. Right now that property
falls out of the extends refusal rather than being stated as a guarantee. It
might be worth saying so in the doc comment: "does not read the filesystem" is
the reason a caller in a privileged context picks this over
TryFrom<&Path>, and it's currently something you have to derive by reading
reparse.

One call site I deliberately did not port

A guard that asks whether a different document — a tracked template the
editor doesn't hold as a Spec — declares a given name, and which must fail
closed: an unparseable template is not evidence that a name is absent from it,
and the guard exists to protect exactly the names it might have failed to read.

Spec::declares_secret_in_text answers bool, so an unrepresentable document
reads as "not declared". In practice that branch is near-unreachable, since
constructing the Spec already parsed the source — this is not a bug report.
But the predicate is about the spec's own text, and my question is about
someone else's; routing it through a Spec would also mean full validation and
an extends refusal on a document that is allowed to inherit.

So that one stays on manifest_edit::declares_secret, which takes text and
returns Result. This works only because the PR promotes manifest_edit to a
public module behind its own feature. Flagging it in case that surface is ever
considered for narrowing: the Spec methods and the text functions serve
genuinely different callers, and at least one real consumer needs both.

Happy to share the downstream diff if it's useful.