Skip to content

Commit 0e0b37a

Browse files
author
bgagent
committed
Merge remote-tracking branch 'origin/main' into feat/441-admission-queue-deferred-pickup
# Conflicts: # cdk/src/handlers/orchestrate-task.ts
2 parents 20c24c4 + b16f062 commit 0e0b37a

57 files changed

Lines changed: 2761 additions & 196 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/integ.yml

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@ on:
2929
types: [completed]
3030
workflow_dispatch: {}
3131

32-
# Only one integ run at a time against the shared account — overlapping deploys
33-
# would collide on the single hardcoded `backgroundagent-integ` stack name.
32+
# Only one integ run at a time against the shared account. Stack names are now
33+
# run-unique (`int-<sha>`, see the integ job) so this is no longer a correctness
34+
# requirement — it bounds shared-account cost/parallelism rather than preventing
35+
# a name collision.
3436
concurrency:
3537
group: cdk-integ
3638
cancel-in-progress: false
@@ -217,30 +219,36 @@ jobs:
217219
- name: Install dependencies
218220
run: yarn install --immutable
219221

220-
- name: Run integ tests (deploy → assert → destroy)
221-
run: mise //cdk:integ
222-
223-
# Safety net: integ-runner forces teardown on success and failure, but if
224-
# the run is cancelled or crashes mid-deploy the stack can be stranded in
225-
# the shared account. Delete it directly via CloudFormation so we never
226-
# leak billable resources.
222+
# Teardown: integ-runner forces `cdk destroy` on success and failure (via
223+
# the CDK bootstrap roles, which hold the CloudFormation permissions). No
224+
# raw-CLI safety net here: the GitHub OIDC role has no
225+
# cloudformation:DeleteStack/DescribeStacks, so such a step can only fail
226+
# (#566). If a cancelled/crashed run strands the stack, reclaim is the
227+
# stranded-stack cleanup owned by #400/#72 — and the run-unique name below
228+
# means a stranded stack never blocks a later run.
227229
#
228-
# NOTE: `cdk destroy backgroundagent-integ` would NOT work here — it
229-
# synthesizes the main app (src/main.ts), which does not contain the integ
230-
# stack, so it exits 0 having deleted nothing. Target the stack by its
231-
# literal CloudFormation name instead. delete-stack is idempotent (no-op if
232-
# already gone), so `|| true` only guards transient API errors.
233-
- name: Ensure stack torn down
234-
if: always()
230+
# INTEG_STACK_NAME maps the stack to the commit under test plus the run
231+
# number (`int-<sha>-<run>`), replacing the single fixed
232+
# `backgroundagent-integ` name. The run number makes same-commit re-runs
233+
# distinct; the `int-` prefix marks it ephemeral for #72's sweep. HEAD_SHA
234+
# is the resolved PR head (or dispatched ref) and RUN_NUMBER is a positive
235+
# integer, so both are already lowercase-alphanumeric — no sanitizing
236+
# needed to stay CloudFormation-valid.
237+
- name: Run integ tests (deploy → assert → destroy)
235238
env:
236-
AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }}
237-
AWS_DEFAULT_REGION: ${{ vars.AWS_REGION || 'us-east-1' }}
239+
HEAD_SHA: ${{ needs.resolve.outputs.head_sha }}
240+
RUN_NUMBER: ${{ github.run_number }}
238241
run: |
239242
set -euo pipefail
240-
aws cloudformation delete-stack --stack-name backgroundagent-integ || true
241-
# No `|| true` on the wait: a DELETE_FAILED must surface loudly so we
242-
# never silently leak billable resources in the shared account.
243-
aws cloudformation wait stack-delete-complete --stack-name backgroundagent-integ
243+
export INTEG_STACK_NAME="int-${HEAD_SHA:0:12}-${RUN_NUMBER}"
244+
echo "Integ stack name: $INTEG_STACK_NAME"
245+
mise //cdk:integ -- --all --require-approval never \
246+
-c stackName="integ-${{ github.run_id }}-${{ github.run_attempt }}" \
247+
-c github:sha="$(git rev-parse HEAD)" \
248+
-c github:ref="$(git branch --show-current || echo "detached")" \
249+
-c github:actor="${{ github.triggering_actor }}" \
250+
-c github:repository="$(gh repo view --json nameWithOwner --jq .nameWithOwner)" \
251+
-c github:clean="$([ -z "$(git status --porcelain)" ] && echo true || echo false)"
244252
245253
# Post the final integ-smoke status back to the PR head so the check flips from
246254
# pending to success/failure. Skipped for workflow_dispatch (no PR to gate).

agent/pyproject.toml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ dependencies = [
4949
"jsonschema==4.26.0", #https://pypi.org/project/jsonschema/
5050
]
5151

52+
# The uv-missing-dependency-cooldown rule wants a global exclude-newer window.
53+
# We intentionally do NOT set one: deps are exact-pinned (==) and lockfile-managed
54+
# via upgrade-main.yml, and a global cooldown conflicts with fresh exact pins
55+
# (fastapi/bedrock-agentcore/boto3 were published within the last week) — it makes
56+
# `uv sync` unsatisfiable and re-breaks on every pin bump. The rule's threat model
57+
# (surprise malicious/unstable version) is already covered by exact pins + reviewed
58+
# lock upgrades. See #532.
59+
# nosemgrep: uv-missing-dependency-cooldown
5260
[tool.uv]
5361
constraint-dependencies = [
5462
"pyjwt>=2.13.0", # PYSEC-2026-175/177/178/179 — transitive via mcp; remove when mcp bumps floor (#267)

agent/src/hooks.py

Lines changed: 166 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,102 @@ def _tool_input_preview(tool_input: Any, max_len: int = TOOL_INPUT_PREVIEW_MAX)
9999
return _truncate(_strip_ansi(rendered), max_len)
100100

101101

102+
# ---------------------------------------------------------------------------
103+
# Egress-denial detection (#251, Phase 1)
104+
# ---------------------------------------------------------------------------
105+
#
106+
# Best-effort, heuristic scan of a tool's output for the signatures a blocked
107+
# outbound connection leaves behind — a non-allowlisted host hitting the DNS
108+
# Firewall, a refused connection, or a name-resolution failure. When one
109+
# matches we emit an ``egress_denied`` blocker naming the host to allowlist.
110+
# This is intentionally a string-signature scan (not a network probe): the
111+
# model's Bash tool calls (``npm install``, ``git clone``, ``curl``) surface
112+
# their stderr here, and those tools use stable, well-known phrasings. Patterns
113+
# that capture a host use a named ``host`` group; detection-only patterns omit
114+
# it (host stays None → the event still fires, just without ``resource``).
115+
_EGRESS_DENIAL_PATTERNS: tuple[re.Pattern[str], ...] = (
116+
# curl / libcurl, git-over-https
117+
re.compile(r"[Cc]ould not resolve host:?\s+(?P<host>[A-Za-z0-9._-]+)"),
118+
re.compile(r"[Ff]ailed to connect to\s+(?P<host>[A-Za-z0-9._-]+)"),
119+
# Node (npm/yarn), python (requests/urllib), getaddrinfo
120+
re.compile(r"getaddrinfo (?:ENOTFOUND|EAI_AGAIN)\s+(?P<host>[A-Za-z0-9._-]+)"),
121+
# requests/urllib3: the real host appears in ``HTTPSConnectionPool(host='x')``
122+
# or after ``Failed to resolve 'x'``. Match those anchors specifically —
123+
# a bare ``NameResolutionError.*?<fqdn>`` lazy-match would instead capture
124+
# the ``urllib3.connection.HTTPSConnection`` class path that precedes the host.
125+
re.compile(r"HTTPS?ConnectionPool\(host='(?P<host>[A-Za-z0-9._-]+)'"),
126+
re.compile(r"Failed to resolve '(?P<host>[A-Za-z0-9._-]+)'"),
127+
re.compile(
128+
r"nodename nor servname provided|Temporary failure in name resolution"
129+
r"|Name or service not known|Connection refused|Network is unreachable"
130+
r"|NameResolutionError|Failed to establish a new connection",
131+
),
132+
)
133+
134+
135+
# #251 carry-path: the most recent agent-detected blocker, as a canonical
136+
# ``BLOCKED[<kind>]: …`` reason string (see ``format_blocker_reason``). Blockers
137+
# are detected mid-turn in the hooks (egress in PostToolUse, fail-closed in
138+
# PreToolUse) but the terminal ``error_message`` is assembled later in the
139+
# pipeline. This latch bridges the two: the pipeline promotes it into
140+
# ``TaskResult.error`` ONLY when the task errored without a more specific
141+
# reason, so the CDK classifier surfaces a precise remedy. Process-lifetime
142+
# (== one task) like ``_INJECTED_NUDGES``; last-writer-wins is fine — the most
143+
# recent blocker is the most relevant to a terminal failure.
144+
_LAST_BLOCKER_REASON: str | None = None
145+
146+
147+
def _record_blocker_reason(kind: str, detail: str, resource: str | None = None) -> None:
148+
"""Latch the canonical reason for the most recent agent-detected blocker."""
149+
global _LAST_BLOCKER_REASON
150+
from progress_writer import format_blocker_reason
151+
152+
_LAST_BLOCKER_REASON = format_blocker_reason(kind, detail, resource)
153+
154+
155+
def last_blocker_reason() -> str | None:
156+
"""Return the latched canonical blocker reason for terminal promotion (#251)."""
157+
return _LAST_BLOCKER_REASON
158+
159+
160+
def reset_blocker_reason() -> None:
161+
"""Clear the in-process blocker latch.
162+
163+
Called at the start of each ``run_task`` (the latch is a process-scalar,
164+
not task_id-keyed, so it must be reset per task to avoid leaking a prior
165+
task's blocker) and by test fixtures for isolation.
166+
"""
167+
global _LAST_BLOCKER_REASON
168+
_LAST_BLOCKER_REASON = None
169+
170+
171+
# Back-compat alias for test fixtures that import the old name.
172+
_reset_blocker_reason_for_tests = reset_blocker_reason
173+
174+
175+
def detect_egress_denial(text: str) -> tuple[bool, str | None]:
176+
"""Scan tool output for an egress-denial signature (#251).
177+
178+
Returns ``(detected, host_or_None)``: ``(True, host)`` on the first matching
179+
signature (``host`` populated when a pattern captured one so the emitted
180+
``egress_denied`` event can name the exact domain to allowlist; ``None`` for
181+
detection-only signatures), or ``(False, None)`` when nothing matches.
182+
183+
Best-effort and deliberately conservative: it only fires on the
184+
well-known phrasings the common CLIs (curl/git/npm/pip) emit, so a task
185+
log that merely *mentions* "connection refused" in prose could produce a
186+
false positive — acceptable for an observability signal that never blocks.
187+
"""
188+
if not text:
189+
return False, None
190+
for pattern in _EGRESS_DENIAL_PATTERNS:
191+
match = pattern.search(text)
192+
if match:
193+
host = match.groupdict().get("host") if match.groupdict() else None
194+
return True, host
195+
return False, None
196+
197+
102198
def _deny_response(reason: str) -> dict:
103199
"""Build a PreToolUse DENY response with a sanitized reason.
104200
@@ -214,6 +310,30 @@ async def pre_tool_use_hook(
214310
"write_policy_decision_cached",
215311
**decision.cache_hit_metadata,
216312
)
313+
# #251 (decision E): a fail-closed deny means the Cedar engine errored
314+
# or was unavailable — an ENVIRONMENTAL fault (misconfiguration), NOT
315+
# an intentional hard-deny. Emit a ``policy_fail_closed`` blocker event
316+
# so it renders distinctly and carries a remediation hint. We branch on
317+
# the structured ``decision.fail_closed`` flag, never a reason-string
318+
# prefix. Intentional hard-denies / cache-denies (fail_closed=False)
319+
# never emit. Emitted strictly BEFORE the deny; the deny itself is
320+
# unchanged, so this adds observability without altering the outcome.
321+
if getattr(decision, "fail_closed", False):
322+
_record_blocker_reason("policy_fail_closed", decision.reason)
323+
if progress is not None:
324+
_try_progress(
325+
progress,
326+
"write_agent_blocked",
327+
kind="policy_fail_closed",
328+
detail=decision.reason,
329+
remediation_hint=(
330+
"The Cedar policy engine errored or was unavailable "
331+
"(fail-closed). Check the agent policy-engine logs and the "
332+
"policy bundle; this is a misconfiguration, not an "
333+
"intentional deny."
334+
),
335+
retryable=False,
336+
)
217337
log("POLICY", f"DENIED: {tool_name}{decision.reason}")
218338
return _deny_response(decision.reason)
219339

@@ -918,13 +1038,19 @@ async def post_tool_use_hook(
9181038
hook_context: Any,
9191039
*,
9201040
trajectory: _TrajectoryWriter | None = None,
1041+
progress: Any = None,
9211042
) -> dict:
9221043
"""PostToolUse hook: screen tool output for secrets/PII.
9231044
9241045
Returns a dict with hookSpecificOutput. When sensitive content is
9251046
detected the response includes ``updatedMCPToolOutput`` containing the
9261047
redacted version (steered enforcement — content is sanitized, not
9271048
blocked).
1049+
1050+
``progress`` is optional (preserves the Phase 1 test call shape). When
1051+
present, an egress-denial signature in the tool output emits an
1052+
``egress_denied`` blocker event (#251) — best-effort observability,
1053+
never alters the screening decision.
9281054
"""
9291055
_PASS_THROUGH: dict = {"hookSpecificOutput": {"hookEventName": "PostToolUse"}}
9301056
_FAIL_CLOSED: dict = {
@@ -950,6 +1076,43 @@ async def post_tool_use_hook(
9501076
if not isinstance(tool_response, str):
9511077
tool_response = str(tool_response)
9521078

1079+
# #251: best-effort egress-denial detection. A blocked outbound connection
1080+
# (non-allowlisted host hitting the DNS Firewall, refused connection, name
1081+
# resolution failure) surfaces in the tool's stderr here. Emit an
1082+
# ``egress_denied`` blocker naming the host to allowlist. Observability
1083+
# only — never changes the pass-through / redaction decision below.
1084+
egress_detected, host = detect_egress_denial(tool_response)
1085+
if egress_detected:
1086+
detail = (
1087+
f"A connection to {host!r} was blocked"
1088+
if host
1089+
else "An outbound connection was blocked"
1090+
) + f" during {tool_name}."
1091+
# Latch for terminal promotion ONLY when a host was captured. The latch
1092+
# feeds the authoritative TaskResult.error carry-path, so a host-less,
1093+
# detection-only signature ("Connection refused" / "Network is
1094+
# unreachable") — which commonly fires for an internal localhost race,
1095+
# not an egress denial — must not masquerade as the terminal cause. The
1096+
# observability event below still fires unconditionally (it never blocks
1097+
# and is clearly a heuristic signal in the live stream).
1098+
if host:
1099+
_record_blocker_reason("egress_denied", detail, host)
1100+
if progress is not None:
1101+
_try_progress(
1102+
progress,
1103+
"write_agent_blocked",
1104+
kind="egress_denied",
1105+
detail=detail,
1106+
remediation_hint=(
1107+
f"Allowlist {host!r} in the DNS Firewall rule group"
1108+
if host
1109+
else "Allowlist the required host in the DNS Firewall rule group"
1110+
)
1111+
+ " if it is a legitimate dependency. The agent never widens egress itself.",
1112+
retryable=False,
1113+
resource=host,
1114+
)
1115+
9531116
try:
9541117
result = scan_tool_output(tool_response)
9551118
except Exception as exc:
@@ -1404,7 +1567,9 @@ async def _post(
14041567
hook_input: HookInput, tool_use_id: str | None, ctx: HookContext
14051568
) -> HookJSONOutput:
14061569
try:
1407-
result = await post_tool_use_hook(hook_input, tool_use_id, ctx, trajectory=trajectory)
1570+
result = await post_tool_use_hook(
1571+
hook_input, tool_use_id, ctx, trajectory=trajectory, progress=progress
1572+
)
14081573
return SyncHookJSONOutput(**result)
14091574
except Exception as exc:
14101575
log("ERROR", f"PostToolUse wrapper crashed: {type(exc).__name__}: {exc}")

agent/src/observability.py

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,54 @@ def current_otel_trace_id() -> str | None:
6464
bundle) so operators can join the task to its CloudWatch/X-Ray trace. Returns
6565
``None`` when there is no recording span (e.g. tracing disabled locally) or
6666
the context is invalid, so callers can treat it as a graceful-missing field.
67+
68+
Never raises: a broken/misconfigured tracer degrades to ``None`` rather than
69+
propagating. Callers read this inside DDB-write try-blocks (progress_writer),
70+
where a raised trace error would otherwise be misclassified as a DDB failure
71+
and trip the shared progress circuit breaker.
6772
"""
68-
span = trace.get_current_span()
69-
ctx = span.get_span_context()
70-
if not ctx.is_valid:
73+
try:
74+
span = trace.get_current_span()
75+
ctx = span.get_span_context()
76+
if not ctx.is_valid:
77+
return None
78+
# format_trace_id renders the 128-bit id as zero-padded 32-char hex — the
79+
# OTEL format, so it joins directly in CloudWatch Transaction Search. Note
80+
# the X-Ray console renders trace ids as ``1-{8hex}-{24hex}``; to look this
81+
# up there, transform to that form (the timestamp is the first 8 hex chars).
82+
return trace.format_trace_id(ctx.trace_id)
83+
except Exception:
84+
# nosemgrep: py-silent-success-masking -- trace id is a graceful-missing
85+
# correlation field; a tracer fault must not fail the caller's write path.
7186
return None
72-
# format_trace_id renders the 128-bit id as zero-padded 32-char hex — the
73-
# OTEL format, so it joins directly in CloudWatch Transaction Search. Note
74-
# the X-Ray console renders trace ids as ``1-{8hex}-{24hex}``; to look this
75-
# up there, transform to that form (the timestamp is the first 8 hex chars).
76-
return trace.format_trace_id(ctx.trace_id)
7787

7888

79-
def set_session_id(session_id: str) -> None:
80-
"""Propagate *session_id* via OTEL baggage for AgentCore session correlation.
89+
def propagate_correlation_context(
90+
session_id: str,
91+
user_id: str = "",
92+
# ``user_id`` uses ""-means-absent (Cognito sub, mirrors AgentConfig.user_id
93+
# which is never None); ``repo`` uses None-means-absent (mirrors the optional
94+
# TaskRecord.repo). Both conventions flow from upstream config types; the
95+
# ``if x:`` guards below flatten either to "don't set the baggage key".
96+
repo: str | None = None,
97+
) -> None:
98+
"""Propagate the correlation envelope via OTEL baggage.
99+
100+
*session_id* correlates custom spans to the AgentCore session; *user_id*
101+
and *repo* (#245) carry the platform identity and target repo so baggage
102+
survives across pipeline phases on the task thread. Empty/None fields are
103+
not set — so this runs (and is useful) even when *session_id* is empty but
104+
the identity is known. *repo* is None for repo-less workflows (#248 Phase 3).
81105
82106
The attached context is intentionally not detached: the background thread
83107
runs a single task then exits, so the context is garbage-collected with the
84108
thread.
85109
"""
86-
ctx = baggage.set_baggage("session.id", session_id)
110+
ctx = context.get_current()
111+
if session_id:
112+
ctx = baggage.set_baggage("session.id", session_id, context=ctx)
113+
if user_id:
114+
ctx = baggage.set_baggage("user.id", user_id, context=ctx)
115+
if repo:
116+
ctx = baggage.set_baggage("repo.url", repo, context=ctx)
87117
context.attach(ctx) # token not stored — thread-scoped lifetime

0 commit comments

Comments
 (0)