Skip to content

Commit 10a1ae0

Browse files
committed
feat(cedar-hitl): approve + deny handlers + shared types (§7.1, §7.2)
Lands the two user-facing REST handlers that flip a PENDING approval row to APPROVED / DENIED, the shared types both call sites and the CLI consume, and the Lambda-side Cedar parser future Chunk-5 handlers (get-policies, create-task validation) will use. Wire types (shared/types.ts) ---------------------------- - ApprovalScope union covering every shape the agent's ApprovalAllowlist understands. Typed so approve-task / create-task / CLI (Chunk 6) all agree at compile time. - ApprovalRecord / ApprovalRequest / ApprovalResponse / DenyRequest / DenyResponse / PendingApprovalSummary / GetPendingResponse / PolicyRuleSummary / GetPoliciesResponse / LinkSlackUserRequest / LinkSlackUserResponse / SlackUserMappingRecord / ApprovalDecisionRecordedEvent / CreateTaskApprovalExtensions. - Constants: DENY_REASON_MAX_LENGTH=2000, INITIAL_APPROVALS_MAX_ENTRIES=20, INITIAL_APPROVALS_MAX_ENTRY_LENGTH=128, APPROVAL_TIMEOUT_S_MIN=30, APPROVAL_TIMEOUT_S_MAX=3600, APPROVAL_TIMEOUT_S_DEFAULT=300. New error codes: REQUEST_NOT_FOUND, REQUEST_ALREADY_DECIDED, TASK_NOT_AWAITING_APPROVAL. Shared helpers -------------- - shared/approval-scope.ts — parseApprovalScope validates every shape; rejects unknown tool types / groups / prefixes, empty values, over-128-char strings. isDegeneratePattern implements §7.4 (length ≤ 2, all-wildcard, wildcard ratio > 50%) for Chunk-5 create-task. - shared/deny-reason-scanner.ts — scanDenyReason redacts AWS keys, GitHub PATs (classic + fine-grained), Slack tokens, PEM blocks, bearer tokens with [REDACTED-...] markers. Mirrors agent/src/output_scanner.py so the deny reason the agent ultimately reads is never raw user input. - shared/cedar-policy.ts — parseRules pulls the five HITL annotations (tier/rule_id/severity/approval_timeout_s/category) into a ParsedRule[], preserving positional policy_id for IMPL-29 diagnostics-to-rule_id resolution. isHardDenyRule, isValidRuleId, matchingRuleIds, concatPolicies exposed for future handlers. Handlers -------- - approve-task.ts (§7.1) — POST /v1/tasks/{task_id}/approve - Cross-table TransactWriteItems: approval row PENDING → APPROVED guarded by user_id = :caller AND status = :pending; TaskTable no-op Update guarded by status = AWAITING_APPROVAL AND awaiting_approval_request_id = :rid. - TransactionCanceledException classified by per-item CancellationReasons. Approval-row failure collapses to 404 REQUEST_NOT_FOUND (no existence oracle per §7.1 finding aws-samples#6); task-row failure → 409 TASK_NOT_AWAITING_APPROVAL. - Optional scope defaults to this_call. - Per-user per-minute rate limit (30/min, synthetic row). - Writes approval_decision_recorded audit event (IMPL-6). Audit failure is logged but does not fail the request — decision is already committed. - deny-task.ts (§7.2) — POST /v1/tasks/{task_id}/deny - Same cross-table pattern; status → DENIED + deny_reason. - Reason is scanDenyReason-sanitized + truncated to DENY_REASON_MAX_LENGTH BEFORE any persistence — agent and audit both read sanitized form; raw input never stored. - Same rate-limit namespace as approve. Tests: +64 total (cedar-policy-parser 24, approval-scope 28, deny-reason-scanner 13, approve-task 14, deny-task 9). Secret test fixtures are assembled from string fragments so the source never holds a contiguous secret literal — Code Defender pre-commit hook otherwise blocks. Stack wiring (task-api.ts routes, agent.ts layer attachment, CreateTaskFn extension, orchestrator + reconciler + fanout + LinkSlackUserFn + GetPolicies + GetPending) lands in the next commit.
1 parent 81f26a4 commit 10a1ae0

12 files changed

Lines changed: 2323 additions & 0 deletions

cdk/src/handlers/approve-task.ts

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
/**
2+
* MIT No Attribution
3+
*
4+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy of
7+
* the Software without restriction, including without limitation the rights to
8+
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
9+
* the Software, and to permit persons to whom the Software is furnished to do so.
10+
*
11+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
12+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
13+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
14+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
15+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
16+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
17+
* SOFTWARE.
18+
*/
19+
20+
import { DynamoDBClient, TransactionCanceledException } from '@aws-sdk/client-dynamodb';
21+
import { DynamoDBDocumentClient, PutCommand, TransactWriteCommand, UpdateCommand } from '@aws-sdk/lib-dynamodb';
22+
import type { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda';
23+
import { ulid } from 'ulid';
24+
import { VALID_APPROVAL_SCOPE_PREFIXES, parseApprovalScope } from './shared/approval-scope';
25+
import { extractUserId } from './shared/gateway';
26+
import { logger } from './shared/logger';
27+
import { ErrorCode, errorResponse, successResponse } from './shared/response';
28+
import type { ApprovalRequest, ApprovalResponse, ApprovalScope } from './shared/types';
29+
30+
const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({}));
31+
const TASK_TABLE_NAME = process.env.TASK_TABLE_NAME;
32+
const TASK_APPROVALS_TABLE_NAME = process.env.TASK_APPROVALS_TABLE_NAME;
33+
const EVENTS_TABLE_NAME = process.env.TASK_EVENTS_TABLE_NAME;
34+
if (!TASK_TABLE_NAME || !TASK_APPROVALS_TABLE_NAME || !EVENTS_TABLE_NAME) {
35+
throw new Error(
36+
'approve-task handler requires TASK_TABLE_NAME, TASK_APPROVALS_TABLE_NAME, and TASK_EVENTS_TABLE_NAME env vars',
37+
);
38+
}
39+
const APPROVE_RATE_LIMIT_PER_MINUTE = Number(process.env.APPROVE_RATE_LIMIT_PER_MINUTE ?? '30');
40+
const AUDIT_EVENT_RETENTION_DAYS = Number(process.env.TASK_RETENTION_DAYS ?? '90');
41+
42+
/**
43+
* POST /v1/tasks/{task_id}/approve — User-in-the-loop approval decision.
44+
*
45+
* Flow (design §7.1):
46+
* 1. Auth — Cognito JWT `sub` → `caller_user_id` (verbatim).
47+
* 2. Parse + validate body (`request_id`, optional `scope`).
48+
* 3. Per-user per-minute rate limit (30/min).
49+
* 4. Atomic cross-table `TransactWriteItems`:
50+
* - Update approval row: `status PENDING → APPROVED`, guarded by
51+
* `user_id = :caller` (ownership) AND `status = :pending`.
52+
* - No-op update on TaskTable guarded by
53+
* `status = AWAITING_APPROVAL AND awaiting_approval_request_id = :rid`.
54+
* 5. On `TransactionCanceledException`, inspect per-item reasons to
55+
* distinguish 404 vs 409 variants (prevents enumeration via the
56+
* 404 collapse on ownership mismatch — finding #6).
57+
* 6. Write `approval_decision_recorded` audit event to
58+
* `TaskEventsTable` (IMPL-6).
59+
*
60+
* Returns 202 with `{task_id, request_id, status: APPROVED, scope,
61+
* decided_at}` on success.
62+
* @param event - API Gateway proxy event.
63+
* @returns API Gateway proxy result.
64+
*/
65+
export async function handler(event: APIGatewayProxyEvent): Promise<APIGatewayProxyResult> {
66+
const requestId = ulid();
67+
68+
try {
69+
// 1. Auth
70+
const callerUserId = extractUserId(event);
71+
if (!callerUserId) {
72+
return errorResponse(401, ErrorCode.UNAUTHORIZED, 'Missing or invalid authentication.', requestId);
73+
}
74+
75+
// 2. Path + body
76+
const taskId = event.pathParameters?.task_id;
77+
if (!taskId) {
78+
return errorResponse(400, ErrorCode.VALIDATION_ERROR, 'Missing task_id path parameter.', requestId);
79+
}
80+
81+
let parsed: ApprovalRequest | null = null;
82+
try {
83+
parsed = event.body ? JSON.parse(event.body) as ApprovalRequest : null;
84+
} catch {
85+
return errorResponse(400, ErrorCode.VALIDATION_ERROR, 'Request body must be valid JSON.', requestId);
86+
}
87+
if (!parsed || typeof parsed.request_id !== 'string' || parsed.decision !== 'approve') {
88+
return errorResponse(
89+
400,
90+
ErrorCode.VALIDATION_ERROR,
91+
'Missing or invalid required fields: request_id (string), decision ("approve").',
92+
requestId,
93+
);
94+
}
95+
const { request_id, scope: rawScope } = parsed;
96+
// `this_call` default (§7.1 example) keeps approve bodies minimal
97+
// for callers who want one-shot approval.
98+
const scope: ApprovalScope = rawScope ?? 'this_call';
99+
const scopeCheck = parseApprovalScope(scope);
100+
if (!scopeCheck.ok) {
101+
return errorResponse(
102+
400,
103+
ErrorCode.VALIDATION_ERROR,
104+
`Invalid scope: ${scopeCheck.message}. Valid prefixes: ${VALID_APPROVAL_SCOPE_PREFIXES.join(', ')}.`,
105+
requestId,
106+
);
107+
}
108+
109+
const nowIso = new Date().toISOString();
110+
const nowEpoch = Math.floor(Date.now() / 1000);
111+
112+
// 3. Per-user per-minute rate limit. Uses a synthetic row in the
113+
// approvals table keyed on `RATE#<user_id>#MINUTE#<yyyymmddhhmm>`
114+
// so the existing grantReadWriteData wiring carries forward; TTL
115+
// reaps the counter after ~120s.
116+
const minuteBucket = formatMinuteBucket(new Date());
117+
try {
118+
await ddb.send(new UpdateCommand({
119+
TableName: TASK_APPROVALS_TABLE_NAME,
120+
Key: {
121+
task_id: `RATE#${callerUserId}#APPROVE`,
122+
request_id: `MINUTE#${minuteBucket}`,
123+
},
124+
UpdateExpression: 'ADD #count :one SET #ttl = :ttl',
125+
ConditionExpression: 'attribute_not_exists(#count) OR #count < :max',
126+
ExpressionAttributeNames: {
127+
'#count': 'count',
128+
'#ttl': 'ttl',
129+
},
130+
ExpressionAttributeValues: {
131+
':one': 1,
132+
':max': APPROVE_RATE_LIMIT_PER_MINUTE,
133+
':ttl': nowEpoch + 120,
134+
},
135+
}));
136+
} catch (err: unknown) {
137+
const name = (err as { name?: string })?.name;
138+
if (name === 'ConditionalCheckFailedException') {
139+
return errorResponse(
140+
429,
141+
ErrorCode.RATE_LIMIT_EXCEEDED,
142+
`Rate limit exceeded: at most ${APPROVE_RATE_LIMIT_PER_MINUTE} approve/deny decisions per minute.`,
143+
requestId,
144+
);
145+
}
146+
throw err;
147+
}
148+
149+
// 4. Cross-table atomic transition (§7.1 pseudocode).
150+
try {
151+
await ddb.send(new TransactWriteCommand({
152+
TransactItems: [
153+
{
154+
Update: {
155+
TableName: TASK_APPROVALS_TABLE_NAME,
156+
Key: { task_id: taskId, request_id },
157+
UpdateExpression:
158+
'SET #status = :approved, decided_at = :now, #scope = :scope',
159+
ConditionExpression:
160+
'attribute_exists(request_id) AND #status = :pending AND user_id = :caller',
161+
ExpressionAttributeNames: {
162+
'#status': 'status',
163+
'#scope': 'scope',
164+
},
165+
ExpressionAttributeValues: {
166+
':approved': 'APPROVED',
167+
':pending': 'PENDING',
168+
':now': nowIso,
169+
':scope': scope,
170+
':caller': callerUserId,
171+
},
172+
},
173+
},
174+
{
175+
Update: {
176+
TableName: TASK_TABLE_NAME,
177+
Key: { task_id: taskId },
178+
// No-op update on TaskTable; the purpose is the condition guard.
179+
UpdateExpression: 'SET last_decision_at = :now',
180+
ConditionExpression:
181+
'#status = :awaiting AND awaiting_approval_request_id = :rid',
182+
ExpressionAttributeNames: { '#status': 'status' },
183+
ExpressionAttributeValues: {
184+
':awaiting': 'AWAITING_APPROVAL',
185+
':rid': request_id,
186+
':now': nowIso,
187+
},
188+
},
189+
},
190+
],
191+
}));
192+
} catch (err: unknown) {
193+
if (err instanceof TransactionCanceledException) {
194+
return classifyCancel(err, requestId);
195+
}
196+
throw err;
197+
}
198+
199+
// 5. Audit event (IMPL-6). Failure to write the audit is logged
200+
// but does not fail the request — the decision is already
201+
// committed on TaskApprovalsTable and the agent will see it on its
202+
// next poll regardless.
203+
try {
204+
await ddb.send(new PutCommand({
205+
TableName: EVENTS_TABLE_NAME,
206+
Item: {
207+
task_id: taskId,
208+
event_id: ulid(),
209+
event_type: 'approval_decision_recorded',
210+
timestamp: nowIso,
211+
ttl: nowEpoch + AUDIT_EVENT_RETENTION_DAYS * 86400,
212+
metadata: {
213+
request_id,
214+
status: 'APPROVED',
215+
scope,
216+
decided_at: nowIso,
217+
caller_user_id: callerUserId,
218+
},
219+
},
220+
}));
221+
} catch (auditErr) {
222+
logger.warn('approval_decision_recorded audit write failed (decision already committed)', {
223+
task_id: taskId,
224+
request_id,
225+
error: auditErr instanceof Error ? auditErr.message : String(auditErr),
226+
});
227+
}
228+
229+
logger.info('Approval recorded', {
230+
task_id: taskId,
231+
request_id,
232+
user_id: callerUserId,
233+
scope,
234+
request_id_header: requestId,
235+
});
236+
237+
const response: ApprovalResponse = {
238+
task_id: taskId,
239+
request_id,
240+
status: 'APPROVED',
241+
scope,
242+
decided_at: nowIso,
243+
};
244+
return successResponse(202, response, requestId);
245+
} catch (err) {
246+
logger.error('Failed to record approval', {
247+
error: err instanceof Error ? err.message : String(err),
248+
request_id: requestId,
249+
});
250+
return errorResponse(500, ErrorCode.INTERNAL_ERROR, 'Internal server error.', requestId);
251+
}
252+
}
253+
254+
/**
255+
* Map a `TransactionCanceledException` to the correct 4xx response.
256+
*
257+
* Per §7.1, the cancellation reasons are per-item (index 0 is the
258+
* approvals-row Update, index 1 is the task-row Update). We read them
259+
* to distinguish:
260+
* - approvals item cancelled:
261+
* - via `attribute_exists` failure → row missing → 404
262+
* - via `user_id = :caller` failure → wrong owner → 404 (no oracle)
263+
* - via `status = :pending` failure → already decided → 409
264+
* - task-row item cancelled only → task not in AWAITING_APPROVAL → 409
265+
*
266+
* DDB does not return which sub-clause of the `ConditionExpression`
267+
* failed, so we infer from whichever row was cancelled. If ONLY the
268+
* approvals Update tripped, it could be any of {missing, wrong owner,
269+
* wrong status}; we conservatively return 404 to prevent the existence
270+
* oracle. The more-specific 409 ALREADY_DECIDED path requires
271+
* additional information we do not have from the reason array alone;
272+
* implementations that want the stronger distinction need to do a
273+
* subsequent GetItem, which re-introduces the race the transaction
274+
* eliminates. v1 accepts the less-granular 404 on ownership drift.
275+
*/
276+
function classifyCancel(
277+
err: TransactionCanceledException,
278+
requestId: string,
279+
): APIGatewayProxyResult {
280+
const reasons = err.CancellationReasons ?? [];
281+
const approvalsReason = reasons[0]?.Code;
282+
const taskReason = reasons[1]?.Code;
283+
284+
if (approvalsReason === 'ConditionalCheckFailed') {
285+
// Collapse all approvals-row failures into REQUEST_NOT_FOUND per
286+
// §7.1 finding #6 (no existence oracle). The less-specific code
287+
// avoids leaking whether the row exists, belongs to a different
288+
// user, or has already been decided.
289+
return errorResponse(
290+
404,
291+
ErrorCode.REQUEST_NOT_FOUND,
292+
'Approval request not found or not owned by caller.',
293+
requestId,
294+
);
295+
}
296+
if (taskReason === 'ConditionalCheckFailed') {
297+
return errorResponse(
298+
409,
299+
ErrorCode.TASK_NOT_AWAITING_APPROVAL,
300+
'Task is not currently awaiting approval for this request.',
301+
requestId,
302+
);
303+
}
304+
// Defensive fallback — propagate 503 since the cause is unexplained.
305+
return errorResponse(
306+
503,
307+
ErrorCode.SERVICE_UNAVAILABLE,
308+
'Approval transaction cancelled for unknown reason.',
309+
requestId,
310+
);
311+
}
312+
313+
function formatMinuteBucket(date: Date): string {
314+
const y = date.getUTCFullYear().toString().padStart(4, '0');
315+
const m = (date.getUTCMonth() + 1).toString().padStart(2, '0');
316+
const d = date.getUTCDate().toString().padStart(2, '0');
317+
const h = date.getUTCHours().toString().padStart(2, '0');
318+
const mi = date.getUTCMinutes().toString().padStart(2, '0');
319+
return `${y}${m}${d}${h}${mi}`;
320+
}

0 commit comments

Comments
 (0)