fix(security): clear the last three silent-success-masking findings — masking gate rc=1 → 0 (#756)

agent/src/observability.py

@@ -15,6 +15,8 @@
 from opentelemetry import baggage, context, trace
 from opentelemetry.trace import StatusCode
 
+from shell import log
+
 if TYPE_CHECKING:
     from collections.abc import Generator
 
@@ -69,6 +71,12 @@ def current_otel_trace_id() -> str | None:
     propagating. Callers read this inside DDB-write try-blocks (progress_writer),
     where a raised trace error would otherwise be misclassified as a DDB failure
     and trip the shared progress circuit breaker.
+
+    The degrade is *logged*, not silent (#756): a tracer fault and "no recording
+    span" both yield ``None``, and a caller cannot tell them apart, so the WARN
+    line is the only place that difference survives. It is deliberately emitted
+    per occurrence rather than once — this runs per progress event, so repetition
+    is the signal that the tracer is broken for the whole task, not one event.
     """
     try:
         span = trace.get_current_span()
@@ -80,8 +88,13 @@ def current_otel_trace_id() -> str | None:
         # the X-Ray console renders trace ids as ``1-{8hex}-{24hex}``; to look this
         # up there, transform to that form (the timestamp is the first 8 hex chars).
         return trace.format_trace_id(ctx.trace_id)
-    except Exception:
-        # nosemgrep: py-silent-success-masking -- tracer fault must not fail the write path
+    except Exception as exc:
+        log(
+            "WARN",
+            f"current_otel_trace_id: tracer fault ({type(exc).__name__}: {exc}); "
+            "persisting the record without a correlation id",
+        )
+        # nosemgrep: py-silent-success-masking -- graceful-missing correlation field, logged above; raising would surface inside the caller's DDB-write try-block and trip the progress circuit breaker (see docstring)  # noqa: E501
         return None
 
 

agent/src/server.py

@@ -1444,6 +1444,7 @@ def _parse_terminate_microvm_id(raw: bytes) -> str:
         # Not silent: an unreadable body is worth a breadcrumb even though it
         # cannot change the outcome.
         _emit_stdout_line(f"[server/warn] /terminate hook body is not JSON ({exc}); ignoring it")
+        # nosemgrep: py-silent-success-masking -- /terminate must answer 200 for ANY body (see docstring); "" is the EXPECTED production id, raising would report a hook failure for a teardown that succeeded, and the unreadable body is logged above  # noqa: E501
         return ""
     if not isinstance(parsed, dict):
         _emit_stdout_line(

agent/tests/test_observability.py

@@ -54,6 +54,33 @@ def test_degrades_to_none_when_tracer_raises(self):
         ):
             assert observability.current_otel_trace_id() is None
 
+    def test_logs_the_tracer_fault_rather_than_degrading_silently(self, capfd):
+        # The degrade is justified but must not be invisible (#756): a tracer
+        # fault and "no recording span" both return None, so the WARN line is the
+        # only surviving evidence of which one happened. capfd (not capsys)
+        # because ``shell.log`` writes at the fd level — see its docstring.
+        with patch.object(
+            observability.trace, "get_current_span", side_effect=RuntimeError("tracer boom")
+        ):
+            assert observability.current_otel_trace_id() is None
+        out = capfd.readouterr().out
+        assert "current_otel_trace_id: tracer fault" in out
+        assert "RuntimeError" in out
+        assert "tracer boom" in out
+
+    def test_does_not_log_when_there_is_simply_no_recording_span(self, capfd):
+        # The common, uninteresting case: tracing disabled locally. An invalid
+        # span context is not a fault, so it must stay quiet — otherwise every
+        # progress event in a non-traced run emits a WARN and the real fault
+        # signal above becomes noise.
+        span = MagicMock()
+        ctx = MagicMock()
+        ctx.is_valid = False
+        span.get_span_context.return_value = ctx
+        with patch.object(observability.trace, "get_current_span", return_value=span):
+            assert observability.current_otel_trace_id() is None
+        assert "tracer fault" not in capfd.readouterr().out
+
 
 class TestPropagateCorrelationContext:
     """``propagate_correlation_context`` propagates the correlation envelope

cdk/src/handlers/registry-publish.ts

@@ -31,6 +31,7 @@ import { REGISTRY_KINDS, RESERVED_KINDS, parseConstraint } from './shared/regist
 import { RegistryPublishIncompleteError, type PublishInput, type RuntimePayload } from './shared/registry/types';
 import { ErrorCode, errorResponse, successResponse } from './shared/response';
 import type { RegistryPublishRequest, RegistryRecordResponse } from './shared/types';
+import { parseBody } from './shared/validation';
 
 const NAMESPACE_RE = /^[a-z][a-z0-9-]*$/;
 const NAME_RE = /^[a-z0-9][a-z0-9._-]*$/;
@@ -52,7 +53,11 @@ export async function handler(event: APIGatewayProxyEvent): Promise<APIGatewayPr
       return errorResponse(403, ErrorCode.FORBIDDEN, `Publishing requires the ${REGISTRY_PUBLISHER_GROUP} group.`, requestId);
     }
 
-    const body = parseBody(event.body);
+    // Shared helper, not a local copy: the duplicate this replaced had the same
+    // "missing body or invalid JSON ⇒ null" contract as the seven sibling
+    // handlers but re-implemented it without the contract's justification, so it
+    // read as an unexplained swallow (#756). One implementation, one rationale.
+    const body = parseBody<RegistryPublishRequest>(event.body);
     if (!body) {
       return errorResponse(400, ErrorCode.VALIDATION_ERROR, 'Request body must be valid JSON.', requestId);
     }
@@ -108,16 +113,6 @@ export async function handler(event: APIGatewayProxyEvent): Promise<APIGatewayPr
   }
 }
 
-function parseBody(raw: string | null): RegistryPublishRequest | null {
-  if (!raw) return null;
-  try {
-    return JSON.parse(raw) as RegistryPublishRequest;
-  } catch {
-    // nosemgrep: ts-silent-success-masking -- malformed JSON is an expected client-input class, not a swallowed fault; null IS the failure encoding and the caller turns it into a 400 VALIDATION_ERROR.
-    return null;
-  }
-}
-
 /** Returns an error message, or null when the request is well-formed. */
 function validate(body: RegistryPublishRequest): string | null {
   if (RESERVED_KINDS.includes(body.kind as (typeof RESERVED_KINDS)[number])) {

cdk/src/handlers/shared/linear-oauth-resolver.ts

@@ -522,7 +522,7 @@ export async function resolveLinearOauthToken(
         error: err instanceof Error ? err.message : String(err),
       });
       // Resolution fails for THIS event; the next one retries. Nothing is latched.
-      return null;
+      return null; // nosemgrep: ts-silent-success-masking -- null IS this resolver's whole failure encoding ("Returns null on any failure … so callers can gracefully no-op", see the contract on the function); it can never be confused with an empty success because no success path returns null, and the cause is logged with full context immediately above
     }
     if (!fetched) {
       logger.error('Linear OAuth secret missing or unreadable', {

cdk/src/handlers/shared/orchestration-store.ts

@@ -347,7 +347,7 @@ function parsePreScreenedAttachments(raw: unknown, orchestrationId: string): Att
       orchestration_id: orchestrationId,
       error: err instanceof Error ? err.message : String(err),
     });
-    return [];
+    return []; // nosemgrep: ts-silent-success-masking -- a corrupt stored JSON blob is PERMANENT, not transient: failing closed here would make every later read of this epic throw, so an epic mid-flight could never settle and no retry could ever clear it. The attachments are advisory context for the children; the warning above is the operator's signal
   }
 }