refactor: dedupe provider bootstrap plumbing and CI release steps

Cleanups from a reuse/simplification/efficiency/altitude review of the
branch, with no behavior change:

- ProviderAlias.env is a plain map (empty means no bootstrap
  credentials), removing the Option normalization and the Some/None
  handling at every consumer
- one registry scheme lookup (registration_for_scheme) shared by spec
  checks, bootstrap_vars, display names, and provider construction
- resolve_bootstrap_overlay looks the alias up once and passes env down;
  sorted_bootstrap_entries is the single ordering rule for fetches,
  validation errors, and login prompts
- store_bootstrap_credential audits through audit_write_result like
  every other write path
- provider bootstrap variable names are consts shared between the
  registration and the read sites so the two cannot drift
- both release workflows publish assets via
  scripts/upload-release-asset.sh; the PHP extension build/stage mapping
  lives only in secretspec-php/scripts/build-ext.sh (now honoring
  CARGO_TARGET_DIR), called by php-ext.yml and ci-sdks.sh

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: domenkozar

.github/workflows/ffi-build.yml

@@ -83,24 +83,14 @@ jobs:
       # On a version tag, attach the cdylib (named for its target, with a sha256
       # sidecar) to the GitHub Release so the PHP SDK's `secretspec-install-lib`
       # command can fetch the right one for a plain `composer require` install.
-      # cargo-dist's release.yml creates the release for this same tag, so wait
-      # for it to exist, then upload with --clobber (both workflows may retry).
       - name: Publish cdylib to the release
         if: startsWith(github.ref, 'refs/tags/v')
         shell: bash
         env:
           GH_TOKEN: ${{ github.token }}
         run: |
           set -euo pipefail
-          tag="${GITHUB_REF_NAME}"
           ext="${{ matrix.artifact }}"; ext="${ext##*.}"
           asset="secretspec-ffi-${{ matrix.target }}.${ext}"
           cp "target/${{ matrix.target }}/release/${{ matrix.artifact }}" "$asset"
-          # sha256 sidecar (BSD shasum on macOS, GNU sha256sum on Linux; both here).
-          if command -v sha256sum >/dev/null; then sha256sum "$asset" > "$asset.sha256"
-          else shasum -a 256 "$asset" > "$asset.sha256"; fi
-          # Wait for cargo-dist to create the release (concurrent workflow).
-          for _ in $(seq 1 30); do
-            gh release view "$tag" >/dev/null 2>&1 && break || sleep 20
-          done
-          gh release upload "$tag" "$asset" "$asset.sha256" --clobber
+          bash scripts/upload-release-asset.sh "${GITHUB_REF_NAME}" "$asset"

.github/workflows/php-ext.yml

@@ -71,23 +71,17 @@ jobs:
       - name: Install Rust (pinned by rust-toolchain.toml)
         run: rustup toolchain install
 
-      - name: Build the extension
-        # Each runner is the target's native arch, so a plain release build emits
-        # the target's binary (no cross-compilation).
-        run: cargo build --release -p secretspec-php-native
-
-      - name: Stage the extension under its distribution name
+      - name: Build and stage the extension under its distribution name
         id: stage
         shell: bash
+        # Each runner is the target's native arch, so a plain release build emits
+        # the target's binary (no cross-compilation). build-ext.sh owns the
+        # platform-to-library-name mapping and stages lib/secretspec.so.
         run: |
           set -euo pipefail
-          case "$(uname -s)" in
-            Darwin) built="libsecretspec_php_native.dylib" ;;
-            *)      built="libsecretspec_php_native.so" ;;
-          esac
-          # dlopen resolves by path, not suffix, so the macOS .dylib ships as .so.
+          bash secretspec-php/scripts/build-ext.sh
           asset="secretspec-php-native-${{ matrix.php }}-nts-${{ matrix.target }}.so"
-          cp "target/release/$built" "$asset"
+          cp secretspec-php/lib/secretspec.so "$asset"
           echo "asset=$asset" >> "$GITHUB_OUTPUT"
 
       - name: Smoke test (load the extension, check it registers)
@@ -110,13 +104,4 @@ jobs:
         shell: bash
         env:
           GH_TOKEN: ${{ github.token }}
-        run: |
-          set -euo pipefail
-          asset="${{ steps.stage.outputs.asset }}"
-          if command -v sha256sum >/dev/null; then sha256sum "$asset" > "$asset.sha256"
-          else shasum -a 256 "$asset" > "$asset.sha256"; fi
-          # cargo-dist's release.yml creates the release for this tag; wait for it.
-          for _ in $(seq 1 30); do
-            gh release view "${GITHUB_REF_NAME}" >/dev/null 2>&1 && break || sleep 20
-          done
-          gh release upload "${GITHUB_REF_NAME}" "$asset" "$asset.sha256" --clobber
+        run: bash scripts/upload-release-asset.sh "${GITHUB_REF_NAME}" "${{ steps.stage.outputs.asset }}"

CHANGELOG.md

@@ -67,6 +67,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   instead" message — the same hard error any other invalid primary gets —
   instead of warning and falling through to the rest of the chain. As a
   fallback entry it is still skipped with a warning, like any broken link.
+- Rust SDK: `ProviderAlias::env` is a plain map whose empty state means "no
+  bootstrap credentials", rather than an `Option`, so the two ways of spelling
+  an alias without credentials cannot diverge.
 
 ### Fixed
 - Provider fallback chains (`providers = [...]`) are now tried strictly in

scripts/ci-sdks.sh

@@ -98,11 +98,8 @@ echo "==> PHP (ext-ffi fallback, dlopens the cdylib via SECRETSPEC_FFI_LIB)"
 echo "==> PHP (secretspec-php-native extension, ext-php-rs)"
 # Build the extension in debug and load it directly; when it is present the SDK
 # prefers it over ext-ffi. This also proves the extension registers its functions.
-cargo build -p secretspec-php-native
-case "$(uname -s)" in
-  Darwin) php_ext="$target_dir/debug/libsecretspec_php_native.dylib" ;;
-  *)      php_ext="$target_dir/debug/libsecretspec_php_native.so" ;;
-esac
-( cd secretspec-php && php -d extension="$php_ext" ./vendor/bin/phpunit )
+CARGO_TARGET_DIR="$target_dir" SECRETSPEC_PHP_PROFILE=debug \
+  bash secretspec-php/scripts/build-ext.sh
+( cd secretspec-php && php -d extension="$PWD/lib/secretspec.so" ./vendor/bin/phpunit )
 
 echo "==> All SDK suites passed"

scripts/upload-release-asset.sh

@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+#
+# Attach release assets (each with a sha256 sidecar) to the GitHub Release for
+# a tag:
+#
+#     upload-release-asset.sh <tag> <asset> [asset...]
+#
+# cargo-dist's release.yml creates the release for the same tag concurrently,
+# so wait for it to exist, then upload with --clobber (either workflow may
+# retry). Needs GH_TOKEN with contents: write.
+set -euo pipefail
+
+tag="$1"
+shift
+
+files=()
+for asset in "$@"; do
+  # sha256 sidecar (BSD shasum on macOS, GNU sha256sum on Linux; both here).
+  if command -v sha256sum >/dev/null; then sha256sum "$asset" > "$asset.sha256"
+  else shasum -a 256 "$asset" > "$asset.sha256"; fi
+  files+=("$asset" "$asset.sha256")
+done
+
+# Wait for cargo-dist to create the release (concurrent workflow).
+for _ in $(seq 1 30); do
+  gh release view "$tag" >/dev/null 2>&1 && break || sleep 20
+done
+gh release upload "$tag" "${files[@]}" --clobber

secretspec-php/scripts/build-ext.sh

@@ -12,6 +12,7 @@ set -euo pipefail
 pkg_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 repo_root="$(cd "$pkg_dir/.." && pwd)"
 profile="${SECRETSPEC_PHP_PROFILE:-release}"
+target_dir="${CARGO_TARGET_DIR:-$repo_root/target}"
 
 case "$(uname -s)" in
   Darwin)               built="libsecretspec_php_native.dylib"; staged="secretspec.so" ;;
@@ -26,5 +27,5 @@ build_flag=()
 mkdir -p "$pkg_dir/lib"
 # dlopen (which PHP uses to load extensions) resolves by path, not by suffix, so
 # staging the macOS .dylib under a .so name loads fine.
-cp -f "$repo_root/target/$profile/$built" "$pkg_dir/lib/$staged"
+cp -f "$target_dir/$profile/$built" "$pkg_dir/lib/$staged"
 echo "built $pkg_dir/lib/$staged"

secretspec/src/cli/mod.rs

@@ -528,7 +528,7 @@ pub fn main() -> Result<()> {
                         // as a bare-string alias.
                         let alias_value = crate::config::ProviderAlias {
                             uri: uri.clone(),
-                            env: (!bootstrap.is_empty()).then_some(bootstrap),
+                            env: bootstrap,
                         };
 
                         // Load or create config

secretspec/src/config.rs

@@ -172,16 +172,17 @@ pub struct ProviderAlias {
     /// The provider URI (e.g. `keyring://`, `bws://project-uuid`).
     pub uri: String,
     /// Bootstrap credentials: environment variable name to the [`BootstrapSource`]
-    /// that supplies it. `None` for a bare string alias.
-    pub env: Option<HashMap<String, BootstrapSource>>,
+    /// that supplies it. Empty for a bare string alias, so "declares no
+    /// bootstrap credentials" has exactly one representation.
+    pub env: HashMap<String, BootstrapSource>,
 }
 
 impl ProviderAlias {
     /// A bare alias carrying only a URI and no bootstrap credentials.
     pub fn from_uri(uri: impl Into<String>) -> Self {
         Self {
             uri: uri.into(),
-            env: None,
+            env: HashMap::new(),
         }
     }
 }
@@ -201,10 +202,8 @@ impl From<&str> for ProviderAlias {
 impl std::fmt::Display for ProviderAlias {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.uri)?;
-        if let Some(env) = &self.env
-            && !env.is_empty()
-        {
-            let mut vars: Vec<&str> = env.keys().map(String::as_str).collect();
+        if !self.env.is_empty() {
+            let mut vars: Vec<&str> = self.env.keys().map(String::as_str).collect();
             vars.sort();
             write!(f, " (bootstrap: {})", vars.join(", "))?;
         }
@@ -214,17 +213,16 @@ impl std::fmt::Display for ProviderAlias {
 
 impl Serialize for ProviderAlias {
     fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
-        match &self.env {
+        if self.env.is_empty() {
             // A bare alias serializes back to the plain-string form, so an alias
             // that was written as a string round-trips unchanged.
-            None => serializer.serialize_str(&self.uri),
-            Some(env) => {
-                use serde::ser::SerializeStruct;
-                let mut table = serializer.serialize_struct("ProviderAlias", 2)?;
-                table.serialize_field("uri", &self.uri)?;
-                table.serialize_field("env", env)?;
-                table.end()
-            }
+            serializer.serialize_str(&self.uri)
+        } else {
+            use serde::ser::SerializeStruct;
+            let mut table = serializer.serialize_struct("ProviderAlias", 2)?;
+            table.serialize_field("uri", &self.uri)?;
+            table.serialize_field("env", &self.env)?;
+            table.end()
         }
     }
 }
@@ -261,12 +259,7 @@ impl<'de> Deserialize<'de> for ProviderAlias {
                 let table = Table::deserialize(serde::de::value::MapAccessDeserializer::new(map))?;
                 Ok(ProviderAlias {
                     uri: table.uri,
-                    // Normalize an explicit empty table (`env = {}`) to `None`:
-                    // "declares no bootstrap credentials" then has a single
-                    // representation, so consumers checking `env.is_some()`
-                    // (e.g. the one-hop rule) cannot disagree with consumers
-                    // iterating the entries.
-                    env: table.env.filter(|env| !env.is_empty()),
+                    env: table.env.unwrap_or_default(),
                 })
             }
         }
@@ -2092,7 +2085,7 @@ mod provider_alias_tests {
     fn bare_string_parses_as_uri_without_env() {
         let map = parse(r#"keyring = "keyring://""#);
         assert_eq!(map["keyring"], ProviderAlias::from("keyring://"));
-        assert_eq!(map["keyring"].env, None);
+        assert!(map["keyring"].env.is_empty());
     }
 
     #[test]
@@ -2102,8 +2095,7 @@ mod provider_alias_tests {
         assert_eq!(alias.uri, "bws://proj");
         let source = alias
             .env
-            .as_ref()
-            .and_then(|env| env.get("BWS_ACCESS_TOKEN"))
+            .get("BWS_ACCESS_TOKEN")
             .expect("env carries the variable");
         assert_eq!(source, &BootstrapSource::from("keyring"));
     }
@@ -2113,7 +2105,7 @@ mod provider_alias_tests {
         let map = parse(
             r#"vault = { uri = "vault://kv", env = { VAULT_ROLE_ID = { provider = "onepassword", ref = { vault = "Infra", item = "approle", field = "role_id" } } } }"#,
         );
-        let source = map["vault"].env.as_ref().unwrap()["VAULT_ROLE_ID"].clone();
+        let source = map["vault"].env["VAULT_ROLE_ID"].clone();
         assert_eq!(source.provider, "onepassword");
         let reference = source.reference.expect("ref present");
         assert_eq!(reference.vault.as_deref(), Some("Infra"));
@@ -2135,10 +2127,7 @@ mod provider_alias_tests {
         for source in [bare, with_ref] {
             let alias = ProviderAlias {
                 uri: "vault://kv".to_string(),
-                env: Some(HashMap::from([(
-                    "VAULT_ROLE_ID".to_string(),
-                    source.clone(),
-                )])),
+                env: HashMap::from([("VAULT_ROLE_ID".to_string(), source.clone())]),
             };
             let map = HashMap::from([("vault".to_string(), alias.clone())]);
             let serialized = toml::to_string(&map).unwrap();
@@ -2154,12 +2143,10 @@ mod provider_alias_tests {
 
     #[test]
     fn empty_env_table_is_equivalent_to_no_env() {
-        // `env = {}` declares nothing, so it must normalize to `None`: the
-        // one-hop rule checks `env.is_some()` and would otherwise reject the
-        // alias as a bootstrap source while login reports nothing to store.
+        // `env = {}` declares nothing: the alias equals its bare-string form
+        // and serializes back to it.
         let map = parse(r#"keyring = { uri = "keyring://", env = {} }"#);
         assert_eq!(map["keyring"], ProviderAlias::from("keyring://"));
-        assert_eq!(map["keyring"].env, None);
     }
 
     #[test]
@@ -2188,10 +2175,10 @@ mod provider_alias_tests {
     fn alias_with_env_round_trips_through_toml() {
         let alias = ProviderAlias {
             uri: "bws://proj".to_string(),
-            env: Some(HashMap::from([(
+            env: HashMap::from([(
                 "BWS_ACCESS_TOKEN".to_string(),
                 BootstrapSource::from("keyring"),
-            )])),
+            )]),
         };
         let map = HashMap::from([("bws".to_string(), alias.clone())]);
         let serialized = toml::to_string(&map).unwrap();
@@ -2218,12 +2205,6 @@ API_KEY = { description = "key", required = true }
         let providers = config.providers.expect("[providers] present");
         assert_eq!(providers["keyring"], ProviderAlias::from("keyring://"));
         assert_eq!(providers["bws"].uri, "bws://proj");
-        assert!(
-            providers["bws"]
-                .env
-                .as_ref()
-                .unwrap()
-                .contains_key("BWS_ACCESS_TOKEN")
-        );
+        assert!(providers["bws"].env.contains_key("BWS_ACCESS_TOKEN"));
     }
 }

secretspec/src/provider/bws.rs

@@ -114,14 +114,19 @@ pub struct BwsProvider {
     bootstrap_env: BootstrapEnv,
 }
 
+/// The credential variable this provider reads through the bootstrap overlay.
+/// Shared between the registration's `bootstrap_vars` and the read site, so
+/// the declared list cannot drift from what the provider actually consults.
+const BWS_ACCESS_TOKEN: &str = "BWS_ACCESS_TOKEN";
+
 crate::register_provider! {
     struct: BwsProvider,
     config: BwsConfig,
     name: "bws",
     description: "Bitwarden Secrets Manager",
     schemes: ["bws"],
     examples: ["bws://a9230ec4-5507-4870-b8b5-b3f500587e4c"],
-    bootstrap_vars: ["BWS_ACCESS_TOKEN"],
+    bootstrap_vars: [BWS_ACCESS_TOKEN],
 }
 
 impl BwsProvider {
@@ -137,7 +142,7 @@ impl BwsProvider {
 
     /// Resolves the BWS access token from the environment or bootstrap overlay.
     fn access_token(&self) -> Option<String> {
-        env_or_overlay_var(&self.bootstrap_env, "BWS_ACCESS_TOKEN")
+        env_or_overlay_var(&self.bootstrap_env, BWS_ACCESS_TOKEN)
     }
 
     /// Strips the BWS access token from error messages to avoid leaking credentials.

secretspec/src/provider/mod.rs

@@ -412,6 +412,15 @@ fn split_spec(spec: &str) -> (&str, &str) {
     }
 }
 
+/// The registry entry whose schemes contain `scheme`. The one definition of
+/// "which registration a scheme resolves to", shared by every lookup below and
+/// by [`provider_from_url`], so they cannot drift on the matching rule.
+fn registration_for_scheme(scheme: &str) -> Option<&'static ProviderRegistration> {
+    PROVIDER_REGISTRY
+        .iter()
+        .find(|reg| reg.schemes.contains(&scheme))
+}
+
 /// Whether `spec` names a registered provider: a bare name (`keyring`), a
 /// `scheme:path` shorthand (`dotenv:.env.production`), or a full URI. Checks
 /// the leading scheme token against the registry without constructing a
@@ -430,9 +439,7 @@ pub(crate) fn spec_names_known_provider(spec: &str) -> Result<bool> {
                 .to_string(),
         ));
     }
-    Ok(PROVIDER_REGISTRY
-        .iter()
-        .any(|reg| reg.schemes.contains(&scheme)))
+    Ok(registration_for_scheme(scheme).is_some())
 }
 
 /// The bootstrap variables the provider named by `spec` reads through the
@@ -441,11 +448,7 @@ pub(crate) fn spec_names_known_provider(spec: &str) -> Result<bool> {
 /// provider would silently ignore.
 pub(crate) fn bootstrap_vars_for_spec(spec: &str) -> &'static [&'static str] {
     let (scheme, _) = split_spec(spec);
-    PROVIDER_REGISTRY
-        .iter()
-        .find(|reg| reg.schemes.contains(&scheme))
-        .map(|reg| reg.bootstrap_vars)
-        .unwrap_or(&[])
+    registration_for_scheme(scheme).map_or(&[], |reg| reg.bootstrap_vars)
 }
 
 /// The registered display name for the provider `spec` names, falling back to
@@ -454,9 +457,7 @@ pub(crate) fn bootstrap_vars_for_spec(spec: &str) -> &'static [&'static str] {
 /// bootstrap credentials, so a display-only build could fail or do I/O).
 pub(crate) fn provider_display_name_for_spec(spec: &str) -> String {
     let (scheme, _) = split_spec(spec);
-    PROVIDER_REGISTRY
-        .iter()
-        .find(|reg| reg.schemes.contains(&scheme))
+    registration_for_scheme(scheme)
         .map(|reg| reg.info.name.to_string())
         .unwrap_or_else(|| scheme.to_string())
 }
@@ -1103,10 +1104,7 @@ pub(crate) fn provider_from_url(
 ) -> Result<Box<dyn Provider>> {
     let scheme = url.scheme();
 
-    // Find the provider registration for this scheme
-    let registration = PROVIDER_REGISTRY
-        .iter()
-        .find(|reg| reg.schemes.contains(&scheme))
+    let registration = registration_for_scheme(scheme)
         .ok_or_else(|| SecretSpecError::ProviderNotFound(scheme.to_string()))?;
 
     let pwp = (registration.factory)(url, bootstrap)?;