Skip to content

Commit 2c096ce

Browse files
krokokobgagentisadekscursoragent
authored
feat(security): custom semgrep rules for silent-success masking (AI004) (aws-samples#311)
* feat(security): custom semgrep rules for silent-success masking (AI004) Bare-except and empty-catch are gated, but the more dangerous AI004 variant — swallowing a failure and returning a plausible-but-empty default ([], {}, null/None, "") — had no detector. Such fallbacks hide failures from callers and let plausible-but-wrong code through green. Add custom semgrep rules for TS (catch { return []|null|{}|undefined }) and Python (except: return None|[]|{}|""), with semgrep-test fixtures covering rethrow/re-raise, finally, multi-except, and return-in-try cases. Findings anchor on the offending return line and carry agent-targeted remediation text. The new `security:sast:masking` task validates the rules, scans the repo, and writes SARIF to test-reports/ so findings stay agent-routable (pairs with CA-06). It runs inside `mise run security` but is ADVISORY (no --error) until the 40-finding baseline in .semgrep/BASELINE.md is triaged; intentional degraded-mode fallbacks use inline justified nosemgrep comments (existing repo convention). Closes aws-samples#257 * feat(security): flip silent-success masking gate to blocking (aws-samples#257) Triage all 40 baseline AI004 findings with justified inline nosemgrep allowlists for intentional fail-open and degraded-mode fallbacks, add --error to security:sast:masking, and remove the advisory baseline doc. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: bgagent <bgagent@noreply.github.com> Co-authored-by: Sphia Sadek <isadeks@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 9a58797 commit 2c096ce

28 files changed

Lines changed: 417 additions & 26 deletions

‎.semgrep/silent-success-masking.py‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
# Test fixtures for py-silent-success-masking (run: semgrep test .semgrep/).
2+
3+
4+
def fetch_items() -> list:
5+
raise NotImplementedError
6+
7+
8+
def masked_empty_list() -> list:
9+
try:
10+
return fetch_items()
11+
except Exception:
12+
# ruleid: py-silent-success-masking
13+
return []
14+
15+
16+
def masked_none():
17+
try:
18+
return fetch_items()
19+
except ValueError as exc:
20+
print(exc)
21+
# ruleid: py-silent-success-masking
22+
return None
23+
24+
25+
def masked_empty_dict() -> dict:
26+
try:
27+
return {"items": fetch_items()}
28+
except Exception:
29+
# ruleid: py-silent-success-masking
30+
return {}
31+
32+
33+
def masked_empty_string() -> str:
34+
try:
35+
return str(fetch_items())
36+
except Exception:
37+
# ruleid: py-silent-success-masking
38+
return ""
39+
40+
41+
def masked_dict_constructor() -> dict:
42+
try:
43+
return {"items": fetch_items()}
44+
except Exception:
45+
# ruleid: py-silent-success-masking
46+
return dict()
47+
48+
49+
def masked_bare_except() -> list:
50+
try:
51+
return fetch_items()
52+
except: # noqa: E722
53+
# ruleid: py-silent-success-masking
54+
return []
55+
56+
57+
def ok_reraise() -> list:
58+
try:
59+
return fetch_items()
60+
except Exception as exc:
61+
print(exc)
62+
# ok: py-silent-success-masking
63+
raise
64+
65+
66+
def ok_typed_raise() -> list:
67+
try:
68+
return fetch_items()
69+
except Exception as exc:
70+
# ok: py-silent-success-masking
71+
raise RuntimeError("fetch failed") from exc
72+
73+
74+
def ok_meaningful_fallback() -> dict:
75+
try:
76+
return {"items": fetch_items()}
77+
except Exception:
78+
# ok: py-silent-success-masking
79+
return {"error": True}
80+
81+
82+
def masked_with_finally() -> list:
83+
try:
84+
return fetch_items()
85+
except Exception:
86+
# ruleid: py-silent-success-masking
87+
return []
88+
finally:
89+
print("done")
90+
91+
92+
def masked_multi_except():
93+
try:
94+
return fetch_items()
95+
except ValueError:
96+
# ruleid: py-silent-success-masking
97+
return []
98+
except KeyError:
99+
# ruleid: py-silent-success-masking
100+
return None
101+
102+
103+
# A conditional re-raise does not clear the fallthrough default: callers that
104+
# hit the non-fatal path still cannot distinguish failure from empty success.
105+
def masked_conditional_reraise(fatal: bool) -> list:
106+
try:
107+
return fetch_items()
108+
except Exception:
109+
if fatal:
110+
raise
111+
# ruleid: py-silent-success-masking
112+
return []
113+
114+
115+
def ok_return_in_try_body(items: list) -> list:
116+
try:
117+
if not items:
118+
# ok: py-silent-success-masking
119+
return []
120+
return [i.strip() for i in items]
121+
except Exception as exc:
122+
raise RuntimeError("strip failed") from exc

‎.semgrep/silent-success-masking.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Test fixtures for ts-silent-success-masking (run: semgrep test .semgrep/).
2+
/* eslint-disable */
3+
4+
declare function fetchItems(): Promise<string[]>;
5+
declare function parse(s: string): Record<string, unknown>;
6+
declare function log(msg: string): void;
7+
8+
async function maskedEmptyArray(): Promise<string[]> {
9+
try {
10+
return await fetchItems();
11+
} catch (err) {
12+
// ruleid: ts-silent-success-masking
13+
return [];
14+
}
15+
}
16+
17+
function maskedNull(s: string): Record<string, unknown> | null {
18+
try {
19+
return parse(s);
20+
} catch {
21+
// ruleid: ts-silent-success-masking
22+
return null;
23+
}
24+
}
25+
26+
function maskedEmptyObject(s: string): Record<string, unknown> {
27+
try {
28+
return parse(s);
29+
} catch (err) {
30+
log(String(err));
31+
// ruleid: ts-silent-success-masking
32+
return {};
33+
}
34+
}
35+
36+
function maskedUndefined(s: string): Record<string, unknown> | undefined {
37+
try {
38+
return parse(s);
39+
} catch {
40+
// ruleid: ts-silent-success-masking
41+
return undefined;
42+
}
43+
}
44+
45+
function maskedEmptyString(s: string): string {
46+
try {
47+
return JSON.stringify(parse(s));
48+
} catch {
49+
// ruleid: ts-silent-success-masking
50+
return "";
51+
}
52+
}
53+
54+
function okRethrow(s: string): Record<string, unknown> {
55+
try {
56+
return parse(s);
57+
} catch (err) {
58+
log(String(err));
59+
// ok: ts-silent-success-masking
60+
throw err;
61+
}
62+
}
63+
64+
function okTypedThrow(s: string): Record<string, unknown> {
65+
try {
66+
return parse(s);
67+
} catch (err) {
68+
// ok: ts-silent-success-masking
69+
throw new Error(`parse failed: ${String(err)}`);
70+
}
71+
}
72+
73+
function okMeaningfulFallback(s: string): Record<string, unknown> {
74+
try {
75+
return parse(s);
76+
} catch {
77+
// ok: ts-silent-success-masking
78+
return { error: true };
79+
}
80+
}
81+
82+
function maskedWithFinally(s: string): string[] {
83+
try {
84+
return [s];
85+
} catch {
86+
// ruleid: ts-silent-success-masking
87+
return [];
88+
} finally {
89+
log("done");
90+
}
91+
}
92+
93+
// A conditional rethrow does not clear the fallthrough default: callers that
94+
// hit the non-fatal path still cannot distinguish failure from empty success.
95+
function maskedConditionalRethrow(s: string): Record<string, unknown> | null {
96+
try {
97+
return parse(s);
98+
} catch (err) {
99+
if (s.length > 0) {
100+
throw err;
101+
}
102+
// ruleid: ts-silent-success-masking
103+
return null;
104+
}
105+
}
106+
107+
function okReturnInTryBody(items: string[]): string[] {
108+
try {
109+
if (items.length === 0) {
110+
// ok: ts-silent-success-masking
111+
return [];
112+
}
113+
return items.map((i) => i.trim());
114+
} catch (err) {
115+
throw new Error(`trim failed: ${String(err)}`);
116+
}
117+
}

‎.semgrep/silent-success-masking.yaml‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
# Custom semgrep rules: silent-success masking (AI004 / CA-09, issue #257).
2+
#
3+
# Detects catch/except blocks that swallow a failure and return a
4+
# plausible-but-empty default ([] / {} / null / undefined / None / "" ...)
5+
# instead of surfacing the error. Bare-except and empty-catch are covered by
6+
# existing gates (ruff E722/B, eslint); this targets the more dangerous
7+
# variant where the caller sees a "successful" empty result and cannot tell
8+
# it apart from a real one.
9+
#
10+
# Allowlist mechanism: intentional degraded-mode fallbacks get an inline
11+
# // nosemgrep: <rule-id> -- <justification> (TS)
12+
# # nosemgrep: <rule-id> -- <justification> (Py)
13+
# on the flagged return line (repo convention; precedent in
14+
# scripts/check-types-sync.ts and agent/src/config.py).
15+
#
16+
# Known limitation: a `return null`/`return []` inside a callback *defined
17+
# within* a catch block (e.g. `.map(i => ... return null ...)`) is flagged
18+
# even though it does not mask the outer failure. Rare in practice; use the
19+
# nosemgrep allowlist if it occurs.
20+
#
21+
# Run: mise run security:sast:masking (blocking; emits SARIF)
22+
# Test: semgrep test .semgrep/
23+
rules:
24+
- id: ts-silent-success-masking
25+
languages: [typescript, javascript]
26+
severity: WARNING
27+
metadata:
28+
category: correctness
29+
confidence: MEDIUM
30+
abca-finding: AI004
31+
references:
32+
- https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/issues/257
33+
message: >-
34+
This catch block swallows the error and returns an empty default, so
35+
the caller cannot distinguish failure from a genuinely empty result
36+
(silent-success masking, AI004). Fix: re-throw (`throw err;`), throw a
37+
typed error that adds context, or return a result shape that encodes
38+
the failure. Logging alone is not enough — the failure must reach the
39+
caller. If this fallback is intentional degraded-mode behavior, keep it
40+
and add on the return line
41+
"// nosemgrep: ts-silent-success-masking -- <why callers may safely
42+
treat this failure as an empty success>".
43+
patterns:
44+
- pattern-either:
45+
- pattern: |
46+
try { ... } catch ($E) { ... return $RET; ... }
47+
- pattern: |
48+
try { ... } catch { ... return $RET; ... }
49+
- pattern: |
50+
try { ... } catch ($E) { ... return $RET; ... } finally { ... }
51+
- pattern: |
52+
try { ... } catch { ... return $RET; ... } finally { ... }
53+
- metavariable-regex:
54+
metavariable: $RET
55+
regex: ^(null|undefined|\[\s*\]|\{\s*\}|""|''|``)$
56+
- focus-metavariable: $RET
57+
58+
- id: py-silent-success-masking
59+
languages: [python]
60+
severity: WARNING
61+
metadata:
62+
category: correctness
63+
confidence: MEDIUM
64+
abca-finding: AI004
65+
references:
66+
- https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/issues/257
67+
message: >-
68+
This except block swallows the error and returns an empty default, so
69+
the caller cannot distinguish failure from a genuinely empty result
70+
(silent-success masking, AI004). Fix: re-raise (`raise`), raise a typed
71+
error that adds context (`raise XError(...) from exc`), or return a
72+
result shape that encodes the failure. Logging alone is not enough —
73+
the failure must reach the caller. If this fallback is intentional
74+
degraded-mode behavior, keep it and add on the return line
75+
"# nosemgrep: py-silent-success-masking -- <why callers may safely
76+
treat this failure as an empty success>".
77+
patterns:
78+
- pattern-either:
79+
- pattern: |
80+
try:
81+
...
82+
except $EXC:
83+
...
84+
return $RET
85+
- pattern: |
86+
try:
87+
...
88+
except $EXC as $E:
89+
...
90+
return $RET
91+
- pattern: |
92+
try:
93+
...
94+
except:
95+
...
96+
return $RET
97+
- pattern: |
98+
try:
99+
...
100+
except $EXC:
101+
...
102+
return $RET
103+
finally:
104+
...
105+
- pattern: |
106+
try:
107+
...
108+
except $EXC as $E:
109+
...
110+
return $RET
111+
finally:
112+
...
113+
- pattern: |
114+
try:
115+
...
116+
except:
117+
...
118+
return $RET
119+
finally:
120+
...
121+
- metavariable-regex:
122+
metavariable: $RET
123+
regex: ^(None|\[\s*\]|\{\s*\}|""|''|\(\s*\)|set\(\s*\)|dict\(\s*\)|list\(\s*\)|tuple\(\s*\))$
124+
- focus-metavariable: $RET

‎AGENTS.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,9 @@ Run `mise tasks --all` (with `MISE_EXPERIMENTAL=1`) for the full list. Common co
111111
- **`mise //docs:build`** — Sync and build docs site.
112112
- **`mise run security:secrets`** — Gitleaks at repo root.
113113
- **`mise run security:sast`** — Semgrep on the repo (root; includes **`agent/`** Python among paths).
114+
- **`mise run security:sast:masking`** — Custom semgrep rules for silent-success masking (`catch`/`except` returning empty defaults, AI004). Blocking; emits SARIF to `test-reports/`. Allowlist intentional fallbacks with an inline justified `nosemgrep: <rule-id> -- <reason>` comment.
114115
- **`mise run security:deps`** — OSV Scanner on **`yarn.lock`** (all JS workspaces) and **`agent/uv.lock`**.
115-
- **`mise run security`** — Runs **`security:secrets`**, **`security:deps`**, **`security:sast`**, **`security:grype`**, **`security:retire`**, **`security:gh-actions`**, and **`//agent:security`**.
116+
- **`mise run security`** — Runs **`security:secrets`**, **`security:deps`**, **`security:sast`**, **`security:sast:masking`**, **`security:grype`**, **`security:retire`**, **`security:gh-actions`**, and **`//agent:security`**.
116117
- **`mise run security:retire`** — Retire.js on CDK, CLI, and docs packages.
117118
- **`mise run security:gh-actions`** — Static analysis of GitHub Actions under **`.github/`** ([zizmor](https://github.com/zizmorcore/zizmor)).
118119
- **`mise run hooks:install`** — Re-install **[prek](https://github.com/j178/prek)** git hooks (also run automatically at the end of **`mise run install`** inside a Git checkout). See [CONTRIBUTING.md](./CONTRIBUTING.md) if `core.hooksPath` blocks install.

0 commit comments

Comments
 (0)