Skip to main content

max / makenotwork

server: escalate/log the silent error-swallow sites - Free-cart and single-item free-claim license-key failures now escalate to WAM for manual issuance (mirroring the paid path), instead of `.ok()`/log-only — a claimed item with no key was silent data loss. Cart-cleanup failure now logs rather than silently dropping. - git_ssh can_user_push / is_collaborator still fail closed on a DB error but now log it, so a DB blip that denies a legitimate push/read leaves a trace. - SyncKit sync + userinfo JWT decode still return a uniform 401 but log the specific error kind (expired vs tampered vs malformed) for triage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-02 16:59 UTC
Commit: 0e17f53edb6e717583773582569ba8a954f6b833
Parent: b208bdf
4 files changed, +63 insertions, -7 deletions
@@ -111,7 +111,13 @@ async fn exec_git_operation(
111 111 if !is_owner {
112 112 let can_push = db::repo_collaborators::can_user_push(pool, repo.id, user_id)
113 113 .await
114 - .unwrap_or(false);
114 + .unwrap_or_else(|e| {
115 + // Fail closed, but don't do it silently: a DB error here
116 + // denies a legitimate push with no trace (audit Run 17
117 + // Observability).
118 + tracing::warn!(repo_id = %repo.id, user_id = %user_id, error = ?e, "can_user_push check failed; denying push");
119 + false
120 + });
115 121 if !can_push {
116 122 anyhow::bail!("permission denied: you do not have push access to {}/{}", owner, repo_name);
117 123 }
@@ -141,7 +147,13 @@ async fn exec_git_operation(
141 147 if repo.visibility == db::Visibility::Private && !is_owner {
142 148 let is_collab = db::repo_collaborators::is_collaborator(pool, repo.id, user_id)
143 149 .await
144 - .unwrap_or(false);
150 + .unwrap_or_else(|e| {
151 + // Fail closed (treat as not-a-collaborator) but log: a DB
152 + // error here hides a private repo from a legitimate
153 + // collaborator with no trace (audit Run 17 Observability).
154 + tracing::warn!(repo_id = %repo.id, user_id = %user_id, error = ?e, "is_collaborator check failed; denying read");
155 + false
156 + });
145 157 if !is_collab {
146 158 anyhow::bail!("repository not found");
147 159 }
@@ -186,13 +186,33 @@ async fn claim_free_cart_items(
186 186 }
187 187 if item.enable_license_keys {
188 188 let key_code = helpers::generate_key_code();
189 - db::license_keys::create_license_key(
189 + if let Err(e) = db::license_keys::create_license_key(
190 190 &state.db, item.item_id, user_id, None, &key_code,
191 191 item.default_max_activations,
192 - ).await.ok();
192 + ).await {
193 + // Mirror the paid path (webhook/checkout_helpers.rs): a buyer who
194 + // claimed the item but got no key is silent data loss, so escalate
195 + // to WAM for manual issuance rather than swallowing it (audit Run
196 + // 17 Observability). Free claims have no transaction id, so key the
197 + // ticket on the item.
198 + tracing::error!(user_id = %user_id, item_id = %item.item_id, error = ?e, "failed to generate license key for free claim");
199 + if let Some(ref wam) = state.wam {
200 + let title = format!("License key not issued (free claim): item {}", item.item_id);
201 + let body = format!(
202 + "User {user_id} claimed free item {} but license key generation \
203 + failed: {e}\n\nManually issue a key.",
204 + item.item_id,
205 + );
206 + wam.create_ticket(&title, Some(&body), "critical", "license-key-gen-failed", Some(&item.item_id.to_string())).await;
207 + }
208 + }
193 209 }
194 210 }
195 - db::cart::remove_from_cart_bulk(&state.db, user_id, &to_remove).await.ok();
211 + if let Err(e) = db::cart::remove_from_cart_bulk(&state.db, user_id, &to_remove).await {
212 + // Non-fatal: the items were claimed; a failed cart cleanup just leaves
213 + // stale rows the user can remove. Log rather than drop silently.
214 + tracing::warn!(user_id = %user_id, error = ?e, "failed to clear claimed free items from cart");
215 + }
196 216 Ok(())
197 217 }
198 218
@@ -212,10 +212,23 @@ pub(in crate::routes::stripe) async fn create_checkout(
212 212 );
213 213 }
214 214 Err(e) => {
215 + // Escalate, don't just log: a claimed item with no key is
216 + // silent data loss. Mirror the paid path and cart free-claim
217 + // (audit Run 17 Observability). No transaction id on a free
218 + // claim, so key the ticket on the item.
215 219 tracing::error!(
216 220 buyer_id = %user.id, item_id = %item_uuid, error = ?e,
217 221 "failed to generate license key for free claim"
218 222 );
223 + if let Some(ref wam) = state.wam {
224 + let title = format!("License key not issued (free claim): item {item_uuid}");
225 + let body = format!(
226 + "User {} claimed free item {item_uuid} but license key \
227 + generation failed: {e}\n\nManually issue a key.",
228 + user.id,
229 + );
230 + wam.create_ticket(&title, Some(&body), "critical", "license-key-gen-failed", Some(&item_uuid.to_string())).await;
231 + }
219 232 }
220 233 }
221 234 }
@@ -95,7 +95,13 @@ pub fn decode_sync_token(secret: &str, token: &str) -> Result<SyncClaims, AppErr
95 95 &DecodingKey::from_secret(secret.as_bytes()),
96 96 &validation,
97 97 )
98 - .map_err(|_| AppError::Unauthorized)?;
98 + .map_err(|e| {
99 + // Uniform 401 to the client, but log the specific failure kind (expired
100 + // vs invalid-signature vs malformed) so a spike is triageable (audit Run
101 + // 17 Observability).
102 + tracing::warn!(kind = ?e.kind(), "sync token decode failed");
103 + AppError::Unauthorized
104 + })?;
99 105
100 106 // Reject `iat > now + clock_skew`. 60s skew matches the jsonwebtoken
101 107 // crate's default `leeway` and absorbs typical NTP drift without
@@ -271,7 +277,12 @@ pub fn decode_oauth_access_token(secret: &str, token: &str) -> Result<OAuthAcces
271 277 &DecodingKey::from_secret(secret.as_bytes()),
272 278 &validation,
273 279 )
274 - .map_err(|_| AppError::Unauthorized)?;
280 + .map_err(|e| {
281 + // Uniform 401 to the client; log the failure kind for triage (audit Run
282 + // 17 Observability).
283 + tracing::warn!(kind = ?e.kind(), "userinfo token decode failed");
284 + AppError::Unauthorized
285 + })?;
275 286
276 287 let now = chrono::Utc::now().timestamp();
277 288 if data.claims.iat > now + 60 {