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")

Author: bgagent

agent/src/context.py

@@ -1,11 +1,15 @@
 """Context hydration: GitHub issue fetching and prompt assembly.
 
 Security: GitHub issue/PR content is attacker-controllable (anyone who can
-open an issue can inject text). This module routes every externally-sourced
-string (issue title, body, and each comment body) through
-:func:`sanitization.sanitize_external_content` and wraps the assembled
-external block in explicit ``BEGIN/END UNTRUSTED EXTERNAL CONTENT`` delimiters
-so the model treats it as data, not instructions.
+open an issue can inject text). This module sanitizes every externally-sourced
+string (issue title, body, and each comment author/body) through
+:func:`sanitization.sanitize_external_content` **at the source** — inside
+:func:`fetch_github_issue`, as the :class:`GitHubIssue`/:class:`IssueComment`
+objects are constructed — so the model never carries unsanitized data and
+downstream consumers cannot forget to sanitize. :func:`assemble_prompt` then
+wraps the assembled external block in explicit ``BEGIN/END UNTRUSTED EXTERNAL
+CONTENT`` delimiters (presentation, applied at prompt assembly) so the model
+treats it as data, not instructions.
 
 In production (AgentCore server mode) the orchestrator's
 ``assembleUserPrompt()`` in ``context-hydration.ts`` is the prompt assembler
@@ -22,7 +26,15 @@
 
 
 def fetch_github_issue(repo_url: str, issue_number: str, token: str) -> GitHubIssue:
-    """Fetch a GitHub issue's title, body, and comments."""
+    """Fetch a GitHub issue's title, body, and comments.
+
+    Every attacker-controllable string (title, body, each comment author and
+    body) is passed through :func:`sanitize_external_content` here, as the
+    :class:`GitHubIssue`/:class:`IssueComment` objects are constructed. The
+    returned model is therefore pre-sanitized: consumers (e.g.
+    :func:`assemble_prompt`) must not sanitize again and only need to apply
+    presentation (untrusted-content delimiters).
+    """
     headers = {
         "Authorization": f"token {token}",
         "Accept": "application/vnd.github.v3+json",
@@ -47,13 +59,17 @@ def fetch_github_issue(repo_url: str, issue_number: str, token: str) -> GitHubIs
         )
         comments_resp.raise_for_status()
         comments = [
-            IssueComment(id=int(c["id"]), author=c["user"]["login"], body=c["body"] or "")
+            IssueComment(
+                id=int(c["id"]),
+                author=sanitize_external_content(c["user"]["login"]),
+                body=sanitize_external_content(c["body"] or ""),
+            )
             for c in comments_resp.json()
         ]
 
     return GitHubIssue(
-        title=issue["title"],
-        body=issue.get("body", "") or "",
+        title=sanitize_external_content(issue["title"]),
+        body=sanitize_external_content(issue.get("body", "") or ""),
         number=issue["number"],
         comments=comments,
     )
@@ -73,9 +89,12 @@ def fetch_github_issue(repo_url: str, issue_number: str, token: str) -> GitHubIs
 def assemble_prompt(config: TaskConfig) -> str:
     """Assemble the user prompt from issue context and task description.
 
-    Externally-sourced strings (issue title, body, every comment body) are
-    passed through :func:`sanitize_external_content` and the whole GitHub block
-    is wrapped in ``_UNTRUSTED_BEGIN``/``_UNTRUSTED_END`` delimiters.
+    The issue fields are already sanitized at the source
+    (:func:`fetch_github_issue` runs :func:`sanitize_external_content` as the
+    :class:`GitHubIssue`/:class:`IssueComment` objects are built), so this
+    function only applies presentation: it wraps the whole GitHub block in
+    ``_UNTRUSTED_BEGIN``/``_UNTRUSTED_END`` delimiters and does not sanitize
+    again.
 
     .. note::
         In production (AgentCore server mode), the orchestrator's
@@ -85,8 +104,8 @@ def assemble_prompt(config: TaskConfig) -> str:
         ``HydratedContext.user_prompt`` (validated from the incoming JSON).
         This Python implementation is retained only for **local batch mode**
         (``python src/entrypoint.py``) and **dry-run mode** (``DRY_RUN=1``),
-        where the orchestrator's sanitization never runs — so it sanitizes
-        here independently.
+        where the orchestrator's sanitization never runs — so the agent
+        sanitizes independently at fetch time.
     """
     parts = []
 
@@ -96,16 +115,12 @@ def assemble_prompt(config: TaskConfig) -> str:
     if config.issue:
         issue = config.issue
         parts.append(_UNTRUSTED_BEGIN)
-        parts.append(
-            f"\n## GitHub Issue #{issue.number}: {sanitize_external_content(issue.title)}\n"
-        )
-        parts.append(sanitize_external_content(issue.body) or "(no description)")
+        parts.append(f"\n## GitHub Issue #{issue.number}: {issue.title}\n")
+        parts.append(issue.body or "(no description)")
         if issue.comments:
             parts.append("\n### Comments\n")
             for c in issue.comments:
-                author = sanitize_external_content(c.author)
-                body = sanitize_external_content(c.body)
-                parts.append(f"**@{author}**: {body}\n")
+                parts.append(f"**@{c.author}**: {c.body}\n")
         parts.append(_UNTRUSTED_END)
 
     if config.task_description:

agent/src/models.py

@@ -8,7 +8,12 @@
 
 
 class IssueComment(BaseModel):
-    """Single GitHub issue comment — mirrors ``IssueComment`` in context-hydration.ts."""
+    """Single GitHub issue comment — mirrors ``IssueComment`` in context-hydration.ts.
+
+    ``author`` and ``body`` are pre-sanitized at fetch time: ``fetch_github_issue``
+    in ``context.py`` runs them through ``sanitize_external_content`` as this object
+    is constructed, so consumers must not sanitize again.
+    """
 
     model_config = ConfigDict(frozen=True, extra="forbid")
 
@@ -18,7 +23,15 @@ class IssueComment(BaseModel):
 
 
 class GitHubIssue(BaseModel):
-    """GitHub issue slice — mirrors ``GitHubIssueContext`` in context-hydration.ts."""
+    """GitHub issue slice — mirrors ``GitHubIssueContext`` in context-hydration.ts.
+
+    Externally-sourced fields (``title``, ``body``, and each comment's
+    ``author``/``body``) are pre-sanitized at fetch time: ``fetch_github_issue``
+    in ``context.py`` runs every attacker-controllable string through
+    ``sanitize_external_content`` as this model is constructed. The model never
+    carries unsanitized data, so consumers (e.g. ``assemble_prompt``) must not
+    sanitize again and only apply presentation (untrusted-content delimiters).
+    """
 
     model_config = ConfigDict(frozen=True, extra="forbid")
 

agent/src/server.py

@@ -57,6 +57,24 @@ def _redact_cached_credentials(text: str) -> str:
     return out
 
 
+def _emit_stdout_line(stamped: str) -> None:
+    """Write one line to stdout via ``os.write`` (fd 1).
+
+    Shared sink for ``_debug_cw`` / ``_warn_cw``. Using ``os.write``
+    instead of ``print``/``sys.stdout.write`` keeps lines visible in
+    local runs without tripping CodeQL's cleartext-logging sinks (which
+    model print and TextIOWrapper.write only) — callers MUST have
+    already routed content through ``_redact_cached_credentials``.
+    """
+    line = (stamped + "\n").encode("utf-8", errors="replace")
+    try:
+        while line:
+            n = os.write(1, line)
+            line = line[n:]
+    except OSError:
+        pass
+
+
 def _debug_cw(msg: str, *, task_id: str | None = None) -> None:
     """Write a debug line to a CloudWatch stream in a background thread.
 
@@ -72,16 +90,7 @@ def _debug_cw(msg: str, *, task_id: str | None = None) -> None:
     """
     msg = _redact_cached_credentials(msg)
     stamped = f"[server/debug] {msg}"
-    # Emit via os.write(1, ...) instead of print/sys.stdout.write so debug lines stay
-    # visible locally without tripping CodeQL's cleartext-logging sinks (which model
-    # print and TextIOWrapper.write only). Content is still redacted above.
-    line = (stamped + "\n").encode("utf-8", errors="replace")
-    try:
-        while line:
-            n = os.write(1, line)
-            line = line[n:]
-    except OSError:
-        pass
+    _emit_stdout_line(stamped)
 
     log_group = os.environ.get("LOG_GROUP_NAME")
     if not log_group:
@@ -119,14 +128,20 @@ def _warn_cw(msg: str, *, task_id: str | None = None) -> None:
     the ``server_warn/<task_id>`` stream so operators can alarm on
     warn traffic separately from debug noise).
 
-    The stdout ``print`` is preserved so local ``docker-compose`` runs
-    and the existing ``capsys``-based unit tests still observe the
-    line. CloudWatch delivery is fire-and-forget — failures bump the
+    The stdout emission is preserved so local ``docker-compose`` runs
+    and the ``capfd``-based unit tests still observe the line.
+    CloudWatch delivery is fire-and-forget — failures bump the
     shared ``_debug_cw_failures`` counter via ``_warn_cw_write_blocking``
     so a silently broken writer still surfaces via that single metric.
     """
+    # Redact cached credentials and emit via the same os.write path as
+    # ``_debug_cw``: warn messages can embed payload fragments, so they
+    # get the same sanitizer + non-print sink treatment (CodeQL
+    # clear-text-logging models print/TextIOWrapper.write only; content
+    # is redacted above regardless).
+    msg = _redact_cached_credentials(msg)
     stamped = f"[server/warn] {msg}"
-    print(stamped, flush=True)
+    _emit_stdout_line(stamped)
 
     log_group = os.environ.get("LOG_GROUP_NAME")
     if not log_group:

agent/src/shell.py

@@ -9,9 +9,23 @@
 
 
 def log(prefix: str, text: str):
-    """Print a timestamped, redacted log line."""
+    """Print a timestamped, redacted log line.
+
+    Emits via ``os.write(1, ...)`` rather than ``print`` for parity with
+    ``server._emit_stdout_line``: content is always routed through
+    ``redact_secrets`` first, and the fd-level sink keeps CodeQL's
+    cleartext-logging query (which models print/TextIOWrapper.write)
+    from flagging the already-sanitized line. Tests observing this
+    output must use ``capfd``, not ``capsys``.
+    """
     ts = time.strftime("%H:%M:%S")
-    print(f"[{ts}] {prefix} {redact_secrets(text)}", flush=True)
+    line = f"[{ts}] {prefix} {redact_secrets(text)}\n".encode("utf-8", errors="replace")
+    try:
+        while line:
+            n = os.write(1, line)
+            line = line[n:]
+    except OSError:
+        pass
 
 
 def log_error_cw(message: str, *, task_id: str | None = None) -> None:

agent/tests/conftest.py

@@ -1,7 +1,79 @@
 """Shared fixtures for agent unit tests."""
 
+from types import SimpleNamespace
+
 import pytest
 
+from models import TaskConfig
+
+
+class FakeRunCmd:
+    """Shared fake for ``shell.run_cmd``: records argv and returns scripted results.
+
+    Used by tests that patch ``run_cmd`` (e.g. ``repo.py``, ``post_hooks.py``).
+    Records every call's ``cmd``/``label``/``cwd``/``check`` and returns a
+    ``CompletedProcess``-like ``SimpleNamespace``.
+
+    ``returncodes`` maps a label key -> returncode (default 0); ``stdouts`` maps a
+    label key -> stdout string (default ""). Matching is **exact** by default
+    (the label must equal the key). Pass ``match_substring=True`` to match when
+    the key is a substring of the label — handy for sequence tests that key off a
+    recognizable label fragment. Exact matching is the safe default because some
+    label keys (e.g. ``"push"``) are substrings of other labels
+    (``"note-unpushed-commits"``).
+    """
+
+    def __init__(self, returncodes=None, stdouts=None, match_substring=False):
+        self.calls: list[dict] = []
+        self._returncodes = returncodes or {}
+        self._stdouts = stdouts or {}
+        self._match_substring = match_substring
+
+    def _lookup(self, mapping, label, default):
+        if self._match_substring:
+            value = default
+            for key, val in mapping.items():
+                if key in label:
+                    value = val
+            return value
+        return mapping.get(label, default)
+
+    def __call__(self, cmd, label, cwd=None, timeout=600, check=True, **kwargs):
+        self.calls.append({"cmd": cmd, "label": label, "cwd": cwd, "check": check})
+        rc = self._lookup(self._returncodes, label, 0)
+        stdout = self._lookup(self._stdouts, label, "")
+        return SimpleNamespace(returncode=rc, stdout=stdout, stderr="")
+
+    def labels(self) -> list[str]:
+        return [c["label"] for c in self.calls]
+
+    def cmd_for(self, label: str):
+        """Return the argv for the first call whose label matches *label*.
+
+        Matches by substring when ``match_substring`` is set, else exact equality.
+        """
+        for c in self.calls:
+            if (label in c["label"]) if self._match_substring else (c["label"] == label):
+                return c["cmd"]
+        return None
+
+
+def make_task_config(**overrides) -> TaskConfig:
+    """Build a TaskConfig with test-friendly defaults; ``**overrides`` win.
+
+    Shared by tests that need a repo-bound TaskConfig (``repo.py``,
+    ``post_hooks.py``). Each test supplies its own scripted fields (e.g.
+    ``is_pr_workflow``, ``issue_number``) via ``overrides``.
+    """
+    return TaskConfig(
+        repo_url=overrides.pop("repo_url", "owner/repo"),
+        aws_region=overrides.pop("aws_region", "us-east-1"),
+        task_id=overrides.pop("task_id", "task-abc"),
+        task_description=overrides.pop("task_description", "Do a thing"),
+        **overrides,
+    )
+
+
 # Env vars that agent code reads — clean them to avoid leaking host state.
 _AGENT_ENV_VARS = [
     "TASK_TABLE_NAME",