fix: harden auth, validation, and sandboxing (audit #12-#23)

.gitignore

@@ -94,5 +94,5 @@ coverage/
 pkg/
 wasm-pack.log
 
-# generated API reference (build artifact — never commit/publish pre-managed-infra)
+# generated API reference (build artifact — never commit/publish pre-managed-infra)
 api/openapi.html

cli/src/cli_args.rs

@@ -53,6 +53,11 @@ pub enum Commands {
         did_web_url: Option<String>,
         #[arg(long)]
         admin_user: Option<String>,
+        /// Prefer the `ADMIN_PASSWORD` environment variable instead: a value
+        /// passed here lands in shell history and is readable by other local
+        /// users via `ps`/`/proc/<pid>/cmdline` for the process lifetime.
+        /// `bootstrap` is the scripting/CI entrypoint (no interactive prompt);
+        /// interactive operators should run `odal` instead.
         #[arg(long)]
         admin_pass: Option<String>,
         /// Mint an additional key even if the node is already bootstrapped
@@ -278,8 +283,9 @@ pub enum KeyCommands {
     /// variable or the interactive prompt (omit the argument) so it does not
     /// land in shell history or `ps`/`/proc/<pid>/cmdline`.
     Use {
-        /// The `odal_sk_…` secret to save (prefer the env var / prompt instead).
-        #[arg(value_name = "SECRET")]
+        /// The `odal_sk_…` secret to save. If omitted, it is read from
+        /// `ODAL_API_SECRET` or prompted for without echoing to the terminal —
+        /// keeping the secret out of shell history and the process table.
         secret: Option<String>,
     },
 }

cli/src/core/passport/import.rs

@@ -83,7 +83,7 @@ async fn import_json_records(
                     "Record {}: HTTP {} — {}",
                     i + 1,
                     status,
-                    &body[..body.len().min(200)]
+                    crate::stateless::render::truncate(&body, 200)
                 ));
             }
             Err(e) => {
@@ -174,7 +174,7 @@ async fn summarize_import_response(
     if !status.is_success() {
         anyhow::bail!(
             "Import failed (HTTP {status}): {}",
-            &body[..body.len().min(300)]
+            crate::stateless::render::truncate(body, 300)
         );
     }
     let resp: serde_json::Value =
@@ -197,7 +197,7 @@ async fn poll_import_job(client: &OdalClient, cfg: &Config, job_id: &str) -> Res
         if !status.is_success() {
             anyhow::bail!(
                 "Failed to poll import job (HTTP {status}): {}",
-                &body[..body.len().min(200)]
+                crate::stateless::render::truncate(&body, 200)
             );
         }
         let resp: serde_json::Value =

cli/src/core/passport/publish.rs

@@ -50,7 +50,7 @@ async fn publish_one(client: &OdalClient, vault_url: &str, id: &str) -> Result<P
     } else {
         let err = format!(
             "Failed to publish {id} (HTTP {status}): {}",
-            &body[..body.len().min(300)]
+            crate::stateless::render::truncate(&body, 300)
         );
         Ok(PublishSummary {
             published: 0,
@@ -118,7 +118,7 @@ async fn publish_all(client: &OdalClient, vault_url: &str) -> Result<PublishSumm
             Ok((status, resp_body)) => {
                 let err = format!(
                     "{name}: HTTP {status} — {}",
-                    &resp_body[..resp_body.len().min(200)]
+                    crate::stateless::render::truncate(&resp_body, 200)
                 );
                 errors.push(err.clone());
                 items.push(PassportPublishResult {

cli/src/stateless/render.rs

@@ -17,11 +17,16 @@ use crate::core::types::{
 
 // ── Helpers ──────────────────────────────────────────────────────────────────
 
-fn truncate(s: &str, max: usize) -> String {
-    if s.len() <= max {
+/// Truncate to at most `max` characters (not bytes), appending `…` when cut.
+///
+/// Counts and slices by `char` so a multi-byte UTF-8 sequence landing on the
+/// cutoff can never panic the CLI (the byte-index `&s[..n]` form does).
+pub(crate) fn truncate(s: &str, max: usize) -> String {
+    if s.chars().count() <= max {
         s.to_owned()
     } else {
-        format!("{}…", &s[..max.saturating_sub(1)])
+        let head: String = s.chars().take(max.saturating_sub(1)).collect();
+        format!("{head}…")
     }
 }
 

crates/dpp-common/src/request_id.rs

@@ -9,6 +9,18 @@ use axum::{
 };
 use tower_http::request_id::{MakeRequestId, RequestId};
 
+/// Maximum error-body size buffered to inject `requestId`. Larger bodies are
+/// passed through unmodified (the id remains in the `x-request-id` header).
+const MAX_INJECT_BYTES: usize = 64 * 1024;
+
+/// Whether a `Content-Type` names a JSON media type: plain `application/json`
+/// or any RFC 6839 `+json` structured suffix — including this platform's
+/// `application/problem+json` error type, which `contains("application/json")`
+/// alone does **not** match.
+fn is_json_content_type(content_type: &str) -> bool {
+    content_type.contains("application/json") || content_type.contains("+json")
+}
+
 /// Generates a UUIDv7-based request ID for `SetRequestIdLayer`.
 #[derive(Clone, Default)]
 pub struct UuidRequestId;
@@ -48,17 +60,43 @@ pub async fn inject_request_id(request: Request, next: Next) -> impl IntoRespons
         .headers()
         .get(axum::http::header::CONTENT_TYPE)
         .and_then(|v| v.to_str().ok())
-        .is_some_and(|v| v.contains("application/json"));
+        .is_some_and(is_json_content_type);
 
     if !is_json {
         return response;
     }
 
     let (parts, body) = response.into_parts();
 
-    let bytes = match axum::body::to_bytes(body, 64 * 1024).await {
+    // A large error body (e.g. a batch-import validation report) can't be
+    // buffered to inject `requestId` without risking unbounded memory. When
+    // `Content-Length` already says it exceeds the limit, pass it through
+    // unmodified rather than dropping it — the id is still in the `x-request-id`
+    // header regardless.
+    if parts
+        .headers
+        .get(axum::http::header::CONTENT_LENGTH)
+        .and_then(|v| v.to_str().ok())
+        .and_then(|v| v.parse::<usize>().ok())
+        .is_some_and(|len| len > MAX_INJECT_BYTES)
+    {
+        return Response::from_parts(parts, body);
+    }
+
+    let bytes = match axum::body::to_bytes(body, MAX_INJECT_BYTES).await {
         Ok(b) => b,
-        Err(_) => return Response::from_parts(parts, Body::empty()),
+        Err(e) => {
+            // A streamed body with no Content-Length that overran the limit; it
+            // is consumed and can't be recovered, so return empty — but never
+            // silently, so the truncation is visible in logs.
+            tracing::warn!(
+                error = %e,
+                status = %parts.status,
+                "error response body exceeded {MAX_INJECT_BYTES} bytes; \
+                 requestId not injected and body dropped"
+            );
+            return Response::from_parts(parts, Body::empty());
+        }
     };
 
     let mut json: serde_json::Value = match serde_json::from_slice(&bytes) {
@@ -73,3 +111,24 @@ pub async fn inject_request_id(request: Request, next: Next) -> impl IntoRespons
     let new_body = serde_json::to_vec(&json).unwrap_or_else(|_| bytes.to_vec());
     Response::from_parts(parts, Body::from(new_body))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn json_content_type_matches_problem_json() {
+        assert!(is_json_content_type("application/json"));
+        assert!(is_json_content_type("application/json; charset=utf-8"));
+        // The platform's actual error media type — the case the old
+        // `contains("application/json")` check silently missed.
+        assert!(is_json_content_type("application/problem+json"));
+        assert!(is_json_content_type(
+            "application/problem+json; charset=utf-8"
+        ));
+        assert!(is_json_content_type("application/ld+json"));
+        // Non-JSON must not match.
+        assert!(!is_json_content_type("text/html"));
+        assert!(!is_json_content_type("text/plain; charset=utf-8"));
+    }
+}

crates/dpp-dal/src/pg/repo_api_key.rs

@@ -25,8 +25,9 @@ impl PgApiKeyRepo {
     }
 
     fn key_from_row(r: &sqlx::postgres::PgRow) -> ApiKey {
-        // `scopes TEXT[]` may be NULL for keys written before scopes were
-        // enforced; `from_scopes` maps NULL/empty to Admin (pre-scope behaviour).
+        // `scopes TEXT[]` may be NULL for legacy pre-scope rows; `from_scopes`
+        // fails closed, mapping NULL/empty (and any unrecognized value) to the
+        // least-privilege Read rather than Admin.
         let scope = ApiKeyScope::from_scopes(
             &r.try_get::<Option<Vec<String>>, _>("scopes")
                 .ok()

crates/dpp-dal/src/pg/repo_passport.rs

@@ -23,6 +23,30 @@ use dpp_domain::{
 
 use super::{PgDal, db_err};
 
+/// Fields `patch_fields` refuses to modify: passport identity, lifecycle state,
+/// retention lock, signatures, and seal. Each is governed by the publish
+/// pipeline / `update_status` and backs a scalar column that this JSONB-merge
+/// path does not rewrite — allowing them here would both bypass the state
+/// machine and desync the doc from its enforcing column (e.g. flipping
+/// `retentionLocked` in the doc while the `retention_locked` column stays
+/// `false`). Mirrors the `PassportRepository` default-impl guard in dpp-core.
+/// Serialized (camelCase) names.
+const PROTECTED_PATCH_FIELDS: [&str; 13] = [
+    "id",
+    "sector",
+    "status",
+    "retentionLocked",
+    "retentionUntil",
+    "jwsSignature",
+    "publicJwsSignature",
+    "seal",
+    "version",
+    "publishedAt",
+    "createdAt",
+    "supersedesId",
+    "schemaVersion",
+];
+
 /// Apply a passport update (scalar columns + `doc`) inside a caller-supplied
 /// transaction. Shared by [`PgPassportRepo::update`] and the transactional
 /// outbox's `commit_publish`, so the publish-write and the outbox insert commit
@@ -139,6 +163,13 @@ impl PassportRepository for PgPassportRepo {
     ///
     /// O(n) over active passports — acceptable for single-tenant MVP scale.
     async fn find_published_by_gtin(&self, gtin: &str) -> Result<Option<Passport>, DppError> {
+        // A GTIN is purely numeric. Reject anything else so LIKE metacharacters
+        // (`%`/`_`) in an untrusted value can't widen the pattern to match — and
+        // return — an arbitrary passport. A non-numeric value can never match a
+        // real GS1 Digital Link URL anyway.
+        if gtin.is_empty() || !gtin.bytes().all(|b| b.is_ascii_digit()) {
+            return Ok(None);
+        }
         // Battery GS1 DL URL: https://id.odal-node.io/01/{gtin}/21/{serialId}
         let row = sqlx::query(
             "SELECT doc FROM odal.passport \
@@ -216,6 +247,28 @@ impl PassportRepository for PgPassportRepo {
         id: PassportId,
         delta: serde_json::Value,
     ) -> Result<Passport, DppError> {
+        // Reject protected/state-machine fields up front: they are set only via
+        // the publish pipeline / update_status and back scalar columns this path
+        // does not rewrite, so patching them would bypass the state machine and
+        // desync the doc from its enforcing column.
+        if let Some(obj) = delta.as_object() {
+            let mut forbidden: Vec<&str> = PROTECTED_PATCH_FIELDS
+                .iter()
+                .copied()
+                .filter(|k| obj.contains_key(*k))
+                .collect();
+            if !forbidden.is_empty() {
+                forbidden.sort_unstable();
+                return Err(DppError::Validation(
+                    format!(
+                        "patch_fields cannot modify protected field(s): {}",
+                        forbidden.join(", ")
+                    )
+                    .into(),
+                ));
+            }
+        }
+
         let mut tx = self.dal.begin().await?;
         // Row lock makes concurrent patches serialise instead of clobbering.
         let row = sqlx::query("SELECT doc FROM odal.passport WHERE id = $1 FOR UPDATE")
@@ -237,17 +290,14 @@ impl PassportRepository for PgPassportRepo {
             }
         }
         let passport = Self::from_doc(doc.clone())?;
-        sqlx::query(
-            r#"UPDATE odal.passport SET
-                 status = COALESCE($2->>'status', status),
-                 doc    = $2
-               WHERE id = $1"#,
-        )
-        .bind(Self::uuid_of(id))
-        .bind(&doc)
-        .execute(&mut *tx)
-        .await
-        .map_err(db_err)?;
+        // Doc-only write: the scalar columns are all protected fields (rejected
+        // above), so they can never drift from the doc via this path.
+        sqlx::query("UPDATE odal.passport SET doc = $2 WHERE id = $1")
+            .bind(Self::uuid_of(id))
+            .bind(&doc)
+            .execute(&mut *tx)
+            .await
+            .map_err(db_err)?;
         tx.commit().await.map_err(db_err)?;
         Ok(passport)
     }

crates/dpp-dal/src/pg/repo_registry_sync.rs

@@ -19,6 +19,22 @@ use dpp_types::{RegistrySyncCounts, RegistrySyncOutbox, RegistrySyncRow, Registr
 
 use super::{PgDal, db_err, repo_passport::update_passport_in_tx};
 
+/// Turn an `UPDATE` that matched no row into a `NotFound` rather than a silent
+/// `Ok(())` — a status update against an absent `passport_id` is a real error,
+/// not a no-op success.
+fn require_updated(
+    res: &sqlx::postgres::PgQueryResult,
+    passport_id: PassportId,
+) -> Result<(), DppError> {
+    if res.rows_affected() == 0 {
+        return Err(DppError::NotFound(format!(
+            "registry_sync row for passport {}",
+            passport_id.0
+        )));
+    }
+    Ok(())
+}
+
 /// PostgreSQL implementation of [`RegistrySyncOutbox`].
 pub struct PgRegistrySyncRepo {
     dal: PgDal,
@@ -53,10 +69,21 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo {
         let mut tx = self.dal.begin().await?;
         // Same transaction as the passport write — the atomicity guarantee.
         update_passport_in_tx(&mut tx, passport).await?;
+        // A previously *rejected* row is re-queued with the corrected payload so
+        // a fixed passport can actually be resubmitted (and `due()`, which only
+        // selects 'pending', picks it back up). Rows already 'registered',
+        // 'pending', or carrying a status-intent are left untouched.
         sqlx::query(
             r#"INSERT INTO odal.registry_sync (passport_id, payload, status)
                VALUES ($1, $2, 'pending')
-               ON CONFLICT (passport_id) DO NOTHING"#,
+               ON CONFLICT (passport_id) DO UPDATE SET
+                 payload = EXCLUDED.payload,
+                 status = 'pending',
+                 attempts = 0,
+                 next_attempt_at = now(),
+                 message = NULL,
+                 updated_at = now()
+               WHERE odal.registry_sync.status = 'rejected'"#,
         )
         .bind(passport.id.0)
         .bind(&payload)
@@ -109,7 +136,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo {
         passport_id: PassportId,
         registry_id: String,
     ) -> Result<(), DppError> {
-        sqlx::query(
+        let res = sqlx::query(
             r#"UPDATE odal.registry_sync SET
                  status = 'registered',
                  registry_id = $2,
@@ -124,15 +151,15 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo {
         .execute(self.dal.pool())
         .await
         .map_err(db_err)?;
-        Ok(())
+        require_updated(&res, passport_id)
     }
 
     async fn mark_rejected(
         &self,
         passport_id: PassportId,
         message: String,
     ) -> Result<(), DppError> {
-        sqlx::query(
+        let res = sqlx::query(
             r#"UPDATE odal.registry_sync SET
                  status = 'rejected',
                  message = $2,
@@ -145,7 +172,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo {
         .execute(self.dal.pool())
         .await
         .map_err(db_err)?;
-        Ok(())
+        require_updated(&res, passport_id)
     }
 
     async fn mark_attempt_failed(
@@ -156,7 +183,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo {
         // Exponential backoff on the *new* attempt count, capped at 1h, with
         // 0.75–1.25× jitter to avoid thundering-herd retries. `attempts` in the
         // expression is the pre-increment value the row already holds.
-        sqlx::query(
+        let res = sqlx::query(
             r#"UPDATE odal.registry_sync SET
                  attempts = attempts + 1,
                  message = $2,
@@ -171,7 +198,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo {
         .execute(self.dal.pool())
         .await
         .map_err(db_err)?;
-        Ok(())
+        require_updated(&res, passport_id)
     }
 
     async fn pending_for(