Commit 140b89d
fix(project): resolve audit findings, review findings, and code-scanning alerts (#332)
* fix(project): resolve medium/low audit findings across cdk, cli, agent, docs
Follow-up to #324 (high-severity batch) closing out the remaining
findings from the 2026-06-11 codebase audit, plus review feedback
from that PR.
Security hardening:
- Mirror the empty/whitespace-secret HMAC guard from the GitHub and
Linear webhook verifiers into slack-verify.ts and
webhook-create-task.ts so all four verifiers fail closed on a
misconfigured empty signing secret (PR #324 review follow-up)
- Sanitize GitHub issue/comment content and wrap it in untrusted
delimiters on the agent's local/dry-run prompt path (context.py)
- Validate repoFullName/sha shape before URL and S3-key interpolation
in github-deployment-status.ts
Correctness and resilience:
- Post-once marker for Linear final-status comments so partial-batch
stream retries cannot post duplicates (Linear has no comment edit)
- CLI: memoize in-flight Cognito refresh (concurrent-refresh race),
guard corrupt credentials.json with a friendly CliError, bound
waitForTask with transient-error retry and a wall-clock ceiling,
events --all pagination, validated --limit
- Agent: explicit fail-closed deny for non-dict tool_input, default
branch detection falls back to main on OSError, push_resolve push
failures surface as a PR comment instead of silent success
- validateMaxBudgetUsd rejects NaN/Infinity
Observability:
- CloudWatch alarm on the fan-out DLQ depth (silent notification
outages were invisible) and an async-invoke DLQ for the screenshot
webhook processor
- bgagent watch renders structured approval-milestone metadata
Contracts and tests:
- constants-parity test pins CLI literals to contracts/constants.json
- New hermetic tests for repo.py/post_hooks.py git/gh argv, wait.ts,
debug redaction, and the four hardened webhook verifiers
- Drop two unused @aws-sdk agentcore deps from the CLI (yarn.lock
pruned); add files/engines to cli and docs package manifests
Docs: workflow-model vocabulary in agent/README and AGENTS.md, ADR-014
marked accepted, curly-quote and mise-command fixes in guides,
.DS_Store gitignore case fix; Starlight mirrors regenerated.
* fix(project): address code-review findings and open code-scanning alerts
Follow-up to the medium/low audit batch (ea4c94a), resolving the
confirmed findings from the high-effort plugin review of that commit
plus the three open CodeQL clear-text-logging alerts.
Review findings:
- Restore the original transient-backoff curve (2**attempt): the
extraction to cli/src/retry.ts had silently halved every retry
delay, doubling pressure on a degraded backend; a new test pins the
exact per-attempt jitter window so the curve can't drift again
- events --all --limit N now caps TOTAL events client-side instead of
forwarding limit as the server page size (which returned the whole
stream in N-event pages)
- Only Cognito auth-rejection errors map to "Session expired"; a
transient network blip during the (now shared) refresh tells the
user to retry instead of re-login
- waitForTask timeout/transient-exhaustion exits with code 2 (CliError
now carries exitCode) so scripts can tell "CLI gave up waiting"
from a genuinely FAILED task (exit 1); ceiling check moved to loop
top to cover the transient branch
- Gate verbose-log redaction behind isVerbose() — no more deep-copy
of every request/response body on the non-verbose hot path
- One isUsableHmacSecret() chokepoint (shared/hmac-secret.ts) replaces
the eight hand-copied empty-secret guards across the four webhook
verifiers; one saveDispatchMarker() helper owns the never-throw
post-once marker semantics in the fan-out plane
- Agent: GitHub issue content is sanitized at fetch_github_issue (the
source) so the GitHubIssue model never carries raw untrusted
strings; shared FakeRunCmd/make_task_config test helpers moved to
conftest.py; vestigial watch.ts re-export removed
Code-scanning alerts (py/clear-text-logging, js/clear-text-logging):
- shell.log() and server._warn_cw() emit redacted lines via a shared
os.write sink (same pattern as _debug_cw); warn messages were
previously printed unredacted — tests switched capsys → capfd
- bgagent admin invite-user writes the credential share-block to a
0600 file under ~/.bgagent/invites/ instead of printing the
password to stdout (scrollback/CI capture outlive "share once")
* fix(fanout): retry transient Linear post failures; add construct tests; enforce model-level sanitization
Addresses the three PR #332 review findings:
- Linear final-status comments are no longer dropped on transient
failures: postIssueComment/addIssueReaction now return a classified
LinearPostResult ({ ok } | { ok: false, retryable }) — network
errors, timeouts, 5xx and 429 are retryable; auth, GraphQL errors
and token-resolution failures stay terminal. dispatchToLinear throws
on retryable failures so routeEvent records an infra rejection and
the record lands in batchItemFailures for a Lambda retry. Safe by
construction: the post-once marker is only persisted after a
successful post, so the retry posts the missing comment or
short-circuits on the marker.
- Construct-test gaps closed: fanout-consumer.test.ts pins the
FanOutDlqDepthAlarm (metric binding, threshold 1, notBreaching) so a
refactor can't silently drop the only persistent signal of a fan-out
outage; new github-screenshot-integration.test.ts asserts the
WebhookProcessorDlq exists (14-day retention, enforceSSL) and is
wired as the processor Lambda's async-invoke DeadLetterConfig.
- GitHubIssue/IssueComment sanitization is now structural: field
validators run sanitize_external_content at construction, so every
construction path (fetch_github_issue, model_validate from cache,
tests, future fetchers) yields a sanitized instance — the docstring
promise "consumers must not re-sanitize" is enforced by the type
rather than one caller's discipline. fetch_github_issue drops its
now-redundant call sites; idempotency pinned by tests.
* chore(project): address findings
* chore(cli): hoist 0o600 into SECRET_FILE_MODE constant
The chmod calls added for the durable-permissions fix tripped
@typescript-eslint/no-magic-numbers (the rule exempts object-literal
properties like { mode: 0o600 } but not bare call arguments). One named
constant in config.ts now owns the secret-file mode for credentials and
invite files.
---------
Co-authored-by: bgagent <bgagent@noreply.github.com>1 parent ebaa346 commit 140b89d
80 files changed
Lines changed: 3058 additions & 373 deletions
File tree
- agent
- src
- tests
- cdk
- src
- constructs
- handlers
- test
- constructs
- handlers
- cli
- src
- bin
- commands
- test
- commands
- docs
- abca-plugin/skills
- deploy
- onboard-repo
- setup
- status
- troubleshoot
- decisions
- guides
- src/content/docs
- decisions
- developer-guide
- getting-started
- scripts
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
| 49 | + | |
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
| |||
64 | 64 | | |
65 | 65 | | |
66 | 66 | | |
67 | | - | |
| 67 | + | |
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
| |||
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
103 | | - | |
| 103 | + | |
104 | 104 | | |
105 | 105 | | |
106 | 106 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
356 | 356 | | |
357 | 357 | | |
358 | 358 | | |
359 | | - | |
360 | | - | |
| 359 | + | |
| 360 | + | |
361 | 361 | | |
362 | 362 | | |
363 | 363 | | |
| |||
373 | 373 | | |
374 | 374 | | |
375 | 375 | | |
376 | | - | |
377 | | - | |
378 | | - | |
379 | | - | |
380 | | - | |
381 | | - | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
382 | 384 | | |
383 | 385 | | |
384 | 386 | | |
385 | | - | |
| 387 | + | |
386 | 388 | | |
387 | 389 | | |
388 | 390 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
36 | | - | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
37 | 44 | | |
38 | 45 | | |
39 | 46 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
2 | 21 | | |
3 | 22 | | |
4 | 23 | | |
5 | 24 | | |
6 | 25 | | |
7 | 26 | | |
8 | 27 | | |
9 | | - | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
10 | 38 | | |
11 | 39 | | |
12 | 40 | | |
| |||
31 | 59 | | |
32 | 60 | | |
33 | 61 | | |
34 | | - | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
35 | 70 | | |
36 | 71 | | |
37 | 72 | | |
| |||
43 | 78 | | |
44 | 79 | | |
45 | 80 | | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
46 | 92 | | |
47 | 93 | | |
48 | 94 | | |
49 | | - | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
50 | 103 | | |
51 | 104 | | |
52 | | - | |
| 105 | + | |
| 106 | + | |
53 | 107 | | |
54 | 108 | | |
55 | | - | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
56 | 112 | | |
57 | 113 | | |
58 | 114 | | |
| |||
61 | 117 | | |
62 | 118 | | |
63 | 119 | | |
| 120 | + | |
64 | 121 | | |
65 | 122 | | |
66 | 123 | | |
67 | 124 | | |
68 | 125 | | |
69 | 126 | | |
| 127 | + | |
70 | 128 | | |
71 | 129 | | |
72 | 130 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| 57 | + | |
57 | 58 | | |
58 | 59 | | |
59 | 60 | | |
| |||
67 | 68 | | |
68 | 69 | | |
69 | 70 | | |
70 | | - | |
| 71 | + | |
71 | 72 | | |
72 | 73 | | |
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
76 | 77 | | |
77 | | - | |
78 | | - | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
79 | 84 | | |
80 | 85 | | |
81 | 86 | | |
| |||
169 | 174 | | |
170 | 175 | | |
171 | 176 | | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
172 | 188 | | |
173 | 189 | | |
174 | 190 | | |
| |||
0 commit comments