diff --git a/frontend/src/__tests__/execute-mode-toggle.test.ts b/frontend/src/__tests__/execute-mode-toggle.test.ts new file mode 100644 index 000000000..5b67425df --- /dev/null +++ b/frontend/src/__tests__/execute-mode-toggle.test.ts @@ -0,0 +1,181 @@ +/** + * Execute-mode toggle tests (issue #289). + * + * Verifies that the purchase modal shows the direct-execute toggle only + * when the session holds execute-any:purchases or execute-own:purchases, + * and that the toggle is absent (not rendered) for sessions that only + * hold the base execute:purchases verb or have no permissions at all. + */ + +import { openPurchaseModal, getExecuteMode } from '../recommendations'; + +jest.mock('../api', () => ({ + getRecommendations: jest.fn().mockResolvedValue({ summary: {}, recommendations: [], regions: [] }), + refreshRecommendations: jest.fn(), + getConfig: jest.fn().mockResolvedValue({ global: {} }), + listAccounts: jest.fn().mockResolvedValue([]), + listAccountServiceOverrides: jest.fn().mockResolvedValue([]), +})); + +jest.mock('../api/recommendations', () => ({ + getRecommendationsFreshness: jest.fn().mockResolvedValue({ + last_collected_at: new Date().toISOString(), + last_collection_error: null, + }), + refreshRecommendations: jest.fn().mockResolvedValue({}), +})); + +jest.mock('../state', () => ({ + getCurrentProvider: jest.fn().mockReturnValue('all'), + setCurrentProvider: jest.fn(), + getCurrentAccountIDs: jest.fn().mockReturnValue([]), + setCurrentAccountIDs: jest.fn(), + getRecommendations: jest.fn().mockReturnValue([]), + getRecommendationByID: jest.fn().mockReturnValue(undefined), + setRecommendations: jest.fn(), + getSelectedRecommendationIDs: jest.fn().mockReturnValue(new Set()), + clearSelectedRecommendations: jest.fn(), + addSelectedRecommendation: jest.fn(), + removeSelectedRecommendation: jest.fn(), + getRecommendationsSort: jest.fn().mockReturnValue({ column: 'savings', direction: 'desc' }), + setRecommendationsSort: jest.fn(), + getRecommendationsColumnFilters: jest.fn().mockReturnValue({}), + setRecommendationsColumnFilter: jest.fn(), + clearAllRecommendationsColumnFilters: jest.fn(), + getVisibleRecommendations: jest.fn().mockReturnValue([]), + setVisibleRecommendations: jest.fn(), + getCostPeriod: jest.fn().mockReturnValue('monthly'), + setCostPeriod: jest.fn(), + getHiddenColumns: jest.fn().mockReturnValue(new Set()), + setHiddenColumns: jest.fn(), + getCurrentUser: jest.fn(), +})); + +jest.mock('../toast', () => ({ + showToast: jest.fn().mockReturnValue({ dismiss: jest.fn() }), +})); + +import * as state from '../state'; + +type UserRole = 'admin' | 'user' | 'readonly'; +const mockUser = (role: UserRole | null) => { + (state.getCurrentUser as jest.Mock).mockReturnValue( + role === null ? null : { id: 'u', email: 'u@example.com', role }, + ); +}; + +const minimalRec = { + id: 'r1', + provider: 'aws', + service: 'ec2', + region: 'us-east-1', + resource_type: 'm5.xlarge', + engine: '', + count: 1, + term: 1, + payment: 'all-upfront', + upfront_cost: 1000, + savings: 200, + selected: false, + purchased: false, +}; + +const setupPurchaseModal = () => { + const modal = document.createElement('div'); + modal.id = 'purchase-modal'; + modal.setAttribute('aria-modal', 'true'); + const details = document.createElement('div'); + details.id = 'purchase-details'; + modal.appendChild(details); + document.body.appendChild(modal); + + // execute-purchase-btn is referenced in updateExecuteMode + const btn = document.createElement('button'); + btn.id = 'execute-purchase-btn'; + btn.textContent = 'Send for Approval'; + document.body.appendChild(btn); + + return { modal, details }; +}; + +describe('Execute-mode toggle (issue #289)', () => { + beforeEach(() => { + jest.clearAllMocks(); + document.body.innerHTML = ''; + }); + + test('admin sees the execute-mode toggle in the purchase modal', async () => { + mockUser('admin'); + setupPurchaseModal(); + await openPurchaseModal([minimalRec as never]); + const details = document.getElementById('purchase-details')!; + const toggle = details.querySelector('.execute-mode-toggle'); + expect(toggle).not.toBeNull(); + // Both radio buttons must be present + expect(details.querySelector('#execute-mode-approval')).not.toBeNull(); + expect(details.querySelector('#execute-mode-direct')).not.toBeNull(); + }); + + test('user without execute-own/execute-any sees only the approval note (no toggle)', async () => { + mockUser('user'); + setupPurchaseModal(); + await openPurchaseModal([minimalRec as never]); + const details = document.getElementById('purchase-details')!; + expect(details.querySelector('.execute-mode-toggle')).toBeNull(); + expect(details.querySelector('.approval-required-note')).not.toBeNull(); + }); + + test('readonly user sees only the approval note (no toggle)', async () => { + mockUser('readonly'); + setupPurchaseModal(); + await openPurchaseModal([minimalRec as never]); + const details = document.getElementById('purchase-details')!; + expect(details.querySelector('.execute-mode-toggle')).toBeNull(); + expect(details.querySelector('.approval-required-note')).not.toBeNull(); + }); + + test('getExecuteMode defaults to "" (approval path) after modal open', async () => { + mockUser('admin'); + setupPurchaseModal(); + await openPurchaseModal([minimalRec as never]); + expect(getExecuteMode()).toBe(''); + }); + + test('selecting Execute Now radio sets execute mode to "direct"', async () => { + mockUser('admin'); + setupPurchaseModal(); + await openPurchaseModal([minimalRec as never]); + + const directRadio = document.getElementById('execute-mode-direct') as HTMLInputElement; + expect(directRadio).not.toBeNull(); + directRadio.click(); + directRadio.dispatchEvent(new Event('change', { bubbles: true })); + + expect(getExecuteMode()).toBe('direct'); + + // The submit button label should update. + const btn = document.getElementById('execute-purchase-btn') as HTMLButtonElement; + expect(btn.textContent).toBe('Execute Purchase Now'); + }); + + test('switching back to Send for Approval resets execute mode', async () => { + mockUser('admin'); + setupPurchaseModal(); + await openPurchaseModal([minimalRec as never]); + + // Switch to direct + const directRadio = document.getElementById('execute-mode-direct') as HTMLInputElement; + directRadio.click(); + directRadio.dispatchEvent(new Event('change', { bubbles: true })); + expect(getExecuteMode()).toBe('direct'); + + // Switch back to approval + const approvalRadio = document.getElementById('execute-mode-approval') as HTMLInputElement; + approvalRadio.click(); + approvalRadio.dispatchEvent(new Event('change', { bubbles: true })); + expect(getExecuteMode()).toBe(''); + + const btn = document.getElementById('execute-purchase-btn') as HTMLButtonElement; + expect(btn.textContent).toBe('Send for Approval'); + }); +}); diff --git a/frontend/src/api/purchases.ts b/frontend/src/api/purchases.ts index 16941e21a..c96c6e397 100644 --- a/frontend/src/api/purchases.ts +++ b/frontend/src/api/purchases.ts @@ -15,12 +15,27 @@ import type { * choice (1..100), recorded on the execution for audit; backend * math uses the already-scaled counts in the recommendations list. * Omit or pass 100 for "full capacity" (default). + * + * execute_mode controls the approval path (issue #289): + * - undefined / omitted: standard approval-required flow. + * - "direct": bypass the approval email and execute immediately. + * Requires the session to hold execute-any:purchases or + * execute-own:purchases; the backend returns 403 otherwise. */ -export async function executePurchase(recommendations: Recommendation[], capacityPercent?: number): Promise { - const body: { recommendations: Recommendation[]; capacity_percent?: number } = { recommendations }; +export async function executePurchase( + recommendations: Recommendation[], + capacityPercent?: number, + executeMode?: string, +): Promise { + const body: { recommendations: Recommendation[]; capacity_percent?: number; execute_mode?: string } = { + recommendations, + }; if (capacityPercent !== undefined && capacityPercent !== 100) { body.capacity_percent = capacityPercent; } + if (executeMode) { + body.execute_mode = executeMode; + } return apiRequest('/purchases/execute', { method: 'POST', body: JSON.stringify(body) diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index e051f26ac..e9f81bcf9 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -284,6 +284,9 @@ export interface PurchaseResult { // alice@acme.com") per the CR pass on PR #294 / issue #288. Absent when // recipient resolution itself failed (no approvers configured). approval_recipient?: string; + // True when the request was handled via the direct-execute path (issue + // #289). Absent (undefined) on the standard approval-required flow. + direct_execute?: boolean; results?: Array<{ recommendation_id: string; status: string; diff --git a/frontend/src/app.ts b/frontend/src/app.ts index 124b7e0dc..adc673246 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -6,7 +6,7 @@ import * as api from './api'; import * as state from './state'; import { showLoginModal, showAdminSetupModal, showResetPasswordModal, updateUserUI } from './auth'; import { loadDashboard, setupDashboardHandlers } from './dashboard'; -import { setupRecommendationsHandlers, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, getFanOutBuckets, clearFanOutBuckets, type FanOutBucket } from './recommendations'; +import { setupRecommendationsHandlers, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, getFanOutBuckets, clearFanOutBuckets, getExecuteMode, clearExecuteMode, type FanOutBucket } from './recommendations'; import { switchTab, applyTabFromPath, initRouter, switchSettingsSubTab, getSettingsSubTabFromPath } from './navigation'; import { savePlan, setupPlanHandlers, closePlanModal, openNewPlanModal, closePurchaseModal } from './plans'; import { saveGlobalSettings, setupSettingsHandlers, resetSettings } from './settings'; @@ -307,6 +307,12 @@ async function handleExecutePurchase(): Promise { return; } + // Read the execute mode set by the modal toggle (issue #289). + // "direct" means the session has execute-any/execute-own and chose to + // bypass approval; "" is the default approval-required path. + const executeMode = getExecuteMode(); + const isDirect = executeMode === 'direct'; + // Disable the button BEFORE awaiting the confirm dialog and the network // call so a double-click or rapid re-click can't fire a second POST and // mint a duplicate pending execution (#644). The button is re-enabled on @@ -317,22 +323,30 @@ async function handleExecutePurchase(): Promise { executeBtn.textContent = 'Sending...'; } - // Default approval-required path: clicking sends an approval request to - // the configured approver(s) — it does NOT spend money. The actual - // upfront charge fires only after an approver clicks the email link. - // Issue #289 will introduce a session-permission branch where holders - // of `execute-any:purchases` can opt into direct execution; until that - // lands, every user is on this approval path. - const ok = await confirmDialog({ - title: `Send ${localRecs.length} purchase${localRecs.length === 1 ? '' : 's'} for approval?`, - body: 'This will email an approval request to the configured approver. Cloud commitments are charged only after the approver clicks the link in that email.', - confirmLabel: 'Send for approval', - destructive: false, - }); + const defaultBtnLabel = isDirect ? 'Execute Purchase Now' : 'Send for Approval'; + + // Confirmation dialog varies by mode: + // - Approval path: low-friction, non-destructive. + // - Direct-execute path: red destructive dialog with cost callout and + // cancellation-window reminder (issue #289 acceptance criteria). + const ok = isDirect + ? await confirmDialog({ + title: `Execute ${localRecs.length} purchase${localRecs.length === 1 ? '' : 's'} now?`, + body: 'This will charge the full upfront amount immediately. This bypasses the approval step. AWS allows cancellation within 24 hours via the Account & Billing console.', + confirmLabel: 'Execute Purchase Now', + destructive: true, + }) + : await confirmDialog({ + title: `Send ${localRecs.length} purchase${localRecs.length === 1 ? '' : 's'} for approval?`, + body: 'This will email an approval request to the configured approver. Cloud commitments are charged only after the approver clicks the link in that email.', + confirmLabel: 'Send for approval', + destructive: false, + }); + if (!ok) { if (executeBtn) { executeBtn.disabled = false; - executeBtn.textContent = 'Send for Approval'; + executeBtn.textContent = defaultBtnLabel; } return; } @@ -365,18 +379,26 @@ async function handleExecutePurchase(): Promise { : 100; try { - const result = await api.executePurchase(apiRecs, capacityPercent); + const result = await api.executePurchase(apiRecs, capacityPercent, executeMode || undefined); closePurchaseModal(); clearPurchaseModalRecommendations(); + clearExecuteMode(); - // The backend now surfaces email-send status so the toast can be honest - // about what the user should do next. When email_sent is undefined we - // fall back to the old "check your email" message for backward compat - // with any pre-deploy caller that hasn't picked up the new field yet. - if (result.email_sent === false) { + if (isDirect) { + // Direct-execute: purchase is already committed; inform the user. + showToast({ + message: `Purchase executed immediately (id ${result.execution_id.slice(0, 8)}). Check Purchase History for the result.`, + kind: 'success', + timeout: 15_000, + }); + } else if (result.email_sent === false) { + // The backend now surfaces email-send status so the toast can be honest + // about what the user should do next. When email_sent is undefined we + // fall back to the old "check your email" message for backward compat + // with any pre-deploy caller that hasn't picked up the new field yet. const reason = result.email_reason || 'reason unavailable'; showToast({ - message: `Purchase queued as pending (id ${result.execution_id.slice(0, 8)}…) but the approval email did not send: ${reason}. Approve or cancel it from the Purchase History tab.`, + message: `Purchase queued as pending (id ${result.execution_id.slice(0, 8)}) but the approval email did not send: ${reason}. Approve or cancel it from the Purchase History tab.`, kind: 'warning', timeout: null, }); @@ -390,7 +412,7 @@ async function handleExecutePurchase(): Promise { showToast({ message: recipient ? `Approval request sent to ${recipient}.` - : 'Purchase submitted — check your email to approve.', + : 'Purchase submitted - check your email to approve.', kind: 'success', timeout: 10_000, }); @@ -405,11 +427,12 @@ async function handleExecutePurchase(): Promise { await loadDashboard(); } catch (error) { const err = error as Error; - showToast({ message: `Failed to send purchase for approval: ${err.message}`, kind: 'error' }); + const verb = isDirect ? 'execute' : 'send for approval'; + showToast({ message: `Failed to ${verb} purchase: ${err.message}`, kind: 'error' }); } finally { if (executeBtn) { executeBtn.disabled = false; - executeBtn.textContent = 'Send for Approval'; + executeBtn.textContent = defaultBtnLabel; } } } diff --git a/frontend/src/permissions.ts b/frontend/src/permissions.ts index 61164cebb..c5bc36e91 100644 --- a/frontend/src/permissions.ts +++ b/frontend/src/permissions.ts @@ -49,6 +49,8 @@ export type Action = | 'retry-any' | 'approve-own' | 'approve-any' + | 'execute-own' + | 'execute-any' | 'admin'; // Resource names. Closed enum for the same reason. diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index b0ab6f8ce..fedf31f2d 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -3249,6 +3249,22 @@ export function clearFanOutBuckets(): void { currentFanOutBuckets = null; } +// currentExecuteMode holds the mode selected by the execute-mode toggle in +// the purchase modal (issue #289). "direct" means the session holder has +// execute-any/execute-own and explicitly chose to bypass the approval email. +// "" (empty) is the default approval-required path. Cleared when the modal +// closes. Read by app.ts handleExecutePurchase to set execute_mode in the +// POST body. +let currentExecuteMode: '' | 'direct' = ''; + +export function getExecuteMode(): '' | 'direct' { + return currentExecuteMode; +} + +export function clearExecuteMode(): void { + currentExecuteMode = ''; +} + // resolveBucketPaymentSeed picks the default Payment value for a // bucket per issue #111 sub-option (ii): // - When all recs in the bucket share one non-empty cloud_account_id @@ -3920,19 +3936,106 @@ export async function openPurchaseModal(recommendations: LocalRecommendation[]): while (container.firstChild) container.removeChild(container.firstChild); - // Approval-required note: clicking the modal's primary button does NOT - // execute the purchase — it sends an approval-request email to the - // configured approver(s). The actual upfront charges fire only when an - // approver clicks the link in that email. Issue #288 closed the - // earlier "Execute Purchase" button-label gap that implied immediate - // execution; #289 will introduce a session-permission branch where - // holders of `execute-any:purchases` can opt into direct execution and - // this note will become conditional on the resolved auth path. - const approvalNote = document.createElement('p'); - approvalNote.className = 'approval-required-note'; - approvalNote.textContent = - 'Submitting will email an approval request to the configured approver — commitments are charged only after the approver clicks the link in that email.'; - container.appendChild(approvalNote); + // Reset the execute mode for this modal session so a prior direct-execute + // choice does not carry over to a freshly opened modal (issue #289). + currentExecuteMode = ''; + + // Execute-mode toggle (issue #289): shown only to sessions with + // execute-any:purchases or execute-own:purchases. Everyone else sees only + // the approval-required note with no toggle. + const canDirectExecute = + canAccess('execute-any', 'purchases') || canAccess('execute-own', 'purchases'); + + if (canDirectExecute) { + // Toggle section: "How would you like to handle this purchase?" + const toggleSection = document.createElement('div'); + toggleSection.className = 'form-section execute-mode-toggle'; + + const toggleLabel = document.createElement('p'); + toggleLabel.className = 'execute-mode-label'; + toggleLabel.textContent = 'How would you like to handle this purchase?'; + toggleSection.appendChild(toggleLabel); + + const radioGroup = document.createElement('div'); + radioGroup.className = 'execute-mode-radio-group'; + radioGroup.setAttribute('role', 'radiogroup'); + radioGroup.setAttribute('aria-label', 'Purchase execution mode'); + + // Option 1: Send for Approval (default) + const approvalRadioLabel = document.createElement('label'); + approvalRadioLabel.className = 'execute-mode-radio-label'; + const approvalRadio = document.createElement('input'); + approvalRadio.type = 'radio'; + approvalRadio.name = 'execute-mode'; + approvalRadio.value = ''; + approvalRadio.checked = true; + approvalRadio.id = 'execute-mode-approval'; + approvalRadioLabel.appendChild(approvalRadio); + approvalRadioLabel.appendChild(document.createTextNode(' Send for Approval (default)')); + radioGroup.appendChild(approvalRadioLabel); + + // Option 2: Execute Now + const directRadioLabel = document.createElement('label'); + directRadioLabel.className = 'execute-mode-radio-label'; + const directRadio = document.createElement('input'); + directRadio.type = 'radio'; + directRadio.name = 'execute-mode'; + directRadio.value = 'direct'; + directRadio.id = 'execute-mode-direct'; + directRadioLabel.appendChild(directRadio); + directRadioLabel.appendChild(document.createTextNode(' Execute Now')); + radioGroup.appendChild(directRadioLabel); + + toggleSection.appendChild(radioGroup); + container.appendChild(toggleSection); + + // Warning callout shown only when "Execute Now" is selected. + const directWarning = document.createElement('div'); + directWarning.className = 'direct-execute-warning'; + directWarning.hidden = true; + directWarning.setAttribute('role', 'alert'); + directWarning.setAttribute('aria-live', 'polite'); + container.appendChild(directWarning); + + // Wire radio changes to update state + show/hide warning. + const updateExecuteMode = (): void => { + currentExecuteMode = directRadio.checked ? 'direct' : ''; + directWarning.hidden = currentExecuteMode !== 'direct'; + if (currentExecuteMode === 'direct') { + // Compute total upfront from currently checked rows for the warning. + let totalUpfront = 0; + for (const idx of checkedPurchaseIndices) { + const r = currentPurchaseRecommendations[idx]; + if (r) totalUpfront += r.upfront_cost; + } + while (directWarning.firstChild) directWarning.removeChild(directWarning.firstChild); + const icon = document.createElement('strong'); + icon.textContent = 'Warning: '; + directWarning.appendChild(icon); + const text = document.createTextNode( + `This will charge $${totalUpfront.toLocaleString('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 })} upfront immediately. ` + + 'This bypasses the approval step. AWS allows cancellation within 24 hours via the Account & Billing console.', + ); + directWarning.appendChild(text); + } + // Update the submit button label to reflect the selected mode. + const executeBtn = document.getElementById('execute-purchase-btn') as HTMLButtonElement | null; + if (executeBtn) { + executeBtn.textContent = + currentExecuteMode === 'direct' ? 'Execute Purchase Now' : 'Send for Approval'; + } + }; + + approvalRadio.addEventListener('change', updateExecuteMode); + directRadio.addEventListener('change', updateExecuteMode); + } else { + // No direct-execute permission: show the standard approval-required note. + const approvalNote = document.createElement('p'); + approvalNote.className = 'approval-required-note'; + approvalNote.textContent = + 'Submitting will email an approval request to the configured approver - commitments are charged only after the approver clicks the link in that email.'; + container.appendChild(approvalNote); + } // Commitments table with per-row Include checkboxes, Term, and Payment selects. const commitsSection = document.createElement('div'); diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index f7be43519..c377772ae 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -477,6 +477,53 @@ func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session, return nil } +// authorizeSessionExecuteDirect returns nil when the session is permitted to +// bypass the approval email and execute a purchase immediately under the +// execute-any / execute-own RBAC rules added in issue #289. +// Returns a 403 ClientError otherwise. +// +// creatorID is the creator of the execution being submitted (resolved via +// resolveCreatorUserID before this call; "" on non-human or legacy rows). +// +// Gate logic (mirrors authorizeSessionApprove / authorizeSessionCancel): +// - admin role: always permitted. +// - execute-any: permitted regardless of creator. +// - execute-own: permitted only when creatorID == session.UserID and both +// are non-empty (prevents an empty-string collision from granting access). +// - no matching grant: 403 fail-closed; nil auth component is a 500 as +// per feedback_fail_closed_middleware.md. +func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Session, creatorID string) error { + if session.Role == "admin" { + return nil + } + if h.auth == nil { + return NewClientError(500, "authentication service not configured") + } + + hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionExecuteAny, auth.ResourcePurchases) + if err != nil { + return fmt.Errorf("permission check failed: %w", err) + } + if hasAny { + return nil + } + + hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionExecuteOwn, auth.ResourcePurchases) + if err != nil { + return fmt.Errorf("permission check failed: %w", err) + } + if !hasOwn { + return NewClientError(403, "permission denied: requires execute-any or execute-own on purchases") + } + + // execute-own: both IDs must be non-empty and must match (empty-string + // collision would otherwise grant access to any null-creator row). + if session.UserID == "" || creatorID == "" || creatorID != session.UserID { + return NewClientError(403, "permission denied: execute-own requires you to be the creator of this purchase") + } + return nil +} + func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunctionURLRequest, execID, token string) (any, error) { if err := validateUUID(execID); err != nil { return nil, err @@ -1117,6 +1164,15 @@ type ExecutePurchaseRequest struct { // counts, so backend math ignores this field for purchase work. // 0 / absent defaults to 100 ("full capacity"). CapacityPercent int `json:"capacity_percent,omitempty"` + // ExecuteMode controls whether this request bypasses the approval + // email and executes the purchase immediately. The only accepted + // non-empty value is "direct"; any other value is treated as the + // default approval-required flow. The handler re-checks the + // execute-any/execute-own RBAC gate before honouring "direct", + // even if the session already passed the execute:purchases gate in + // validateExecutePurchaseRequest, so a client that sets this field + // without the privilege receives a 403 rather than silent fallback. + ExecuteMode string `json:"execute_mode,omitempty"` } // validateExecutePurchaseRequest handles the permission check, body parse, @@ -1520,6 +1576,21 @@ func (h *Handler) executePurchase(ctx context.Context, req *events.LambdaFunctio return buildDuplicatePurchaseResponse(dupExec), nil } + // Direct-execute path (issue #289): a session with execute-any or + // execute-own on purchases can request execute_mode="direct" to bypass + // the approval email and commit the purchase immediately. + // + // authorizeSessionExecuteDirect is a hard gate — any check failure + // returns a 403 rather than falling through to the email path. This + // prevents a client that sets execute_mode="direct" but only holds the + // base execute:purchases verb from silently degrading to the email flow. + if execReq.ExecuteMode == "direct" { + if err := h.authorizeSessionExecuteDirect(ctx, session, creatorID); err != nil { + return nil, err + } + return h.directExecutePurchase(ctx, execution, session) + } + // Send approval email synchronously so the response can surface the // actual outcome. The DB write above is the source of truth — email is // best-effort and never blocks the response body; the returned @@ -1528,14 +1599,28 @@ func (h *Handler) executePurchase(ctx context.Context, req *events.LambdaFunctio emailSent, emailReason, recipient := h.sendPurchaseApprovalEmail(ctx, req, execution, execReq.Recommendations, totalUpfront, totalSavings) status := h.finalizePurchaseStatus(ctx, execution, emailSent, emailReason) + return buildApprovalPendingResponse(executionID, status, len(execReq.Recommendations), totalUpfront, totalSavings, emailSent, emailReason, recipient), nil +} + +// buildApprovalPendingResponse assembles the JSON-serialisable response body +// for the approval-pending path of executePurchase. Extracted to keep +// executePurchase cyclomatic complexity within the project limit. +func buildApprovalPendingResponse( + executionID string, + status string, + recCount int, + totalUpfront, totalSavings float64, + emailSent bool, + emailReason, recipient string, +) map[string]any { message := "Purchase execution created and pending approval" if !emailSent { - message = "Purchase execution created but approval email could not be sent — see email_reason" + message = "Purchase execution created but approval email could not be sent - see email_reason" } resp := map[string]any{ "execution_id": executionID, "status": status, - "recommendation_count": len(execReq.Recommendations), + "recommendation_count": recCount, "total_upfront_cost": totalUpfront, "estimated_savings": totalSavings, "email_sent": emailSent, @@ -1547,7 +1632,70 @@ func (h *Handler) executePurchase(ctx context.Context, req *events.LambdaFunctio if recipient != "" { resp["approval_recipient"] = recipient } - return resp, nil + return resp +} + +// directExecutePurchase is the direct-execute branch of executePurchase +// (issue #289). It is called after the execution row has been persisted as +// "pending" and after authorizeSessionExecuteDirect has confirmed the session +// holds execute-any or execute-own on purchases. +// +// Steps: +// 1. Stamp the three audit fields (executed_by_user_id, executed_at, +// pre_approval_skip_reason) onto the in-memory execution so +// SavePurchaseExecution persists them in the next call inside +// ApproveAndExecute. +// 2. Delegate to purchase.Manager.ApproveAndExecute, which atomically +// transitions the row to "approved" and then runs the purchase +// synchronously. ApproveAndExecute already stamps ApprovedBy; we pass +// the session email as the actor so the approved_by column also records +// who direct-executed. +// 3. Return a "completed" status to the caller. +// +// The audit fields are best-effort if ApproveAndExecute's SavePurchaseExecution +// races with our pre-call stamp -- but in practice ApproveAndExecute calls +// SavePurchaseExecution once after a successful TransitionExecutionStatus, at +// which point our pre-stamp is already on the row that was loaded by +// TransitionExecutionStatus. The critical audit invariant is that a non-nil +// executed_by_user_id always co-occurs with a non-nil pre_approval_skip_reason, +// and both are set atomically in the same SavePurchaseExecution call here. +func (h *Handler) directExecutePurchase(ctx context.Context, execution *config.PurchaseExecution, session *Session) (any, error) { + t0 := time.Now() + executionID := execution.ExecutionID + logging.Infof("purchase[%s]: directExecutePurchase entry (auth=session)", executionID) + + // Stamp audit fields before the status transition so they are + // present on the row the reaper / history query reads. + if session.UserID != "" { + uid := session.UserID + execution.ExecutedByUserID = &uid + } + now := time.Now() + execution.ExecutedAt = &now + skipReason := "direct-execute permission" + execution.PreApprovalSkipReason = &skipReason + if err := h.config.SavePurchaseExecution(ctx, execution); err != nil { + // Audit-gap: stamp failed but don't block the purchase. Log at + // error level so a CloudWatch alarm can catch persistent failures. + logging.Errorf("AUDIT GAP: failed to stamp direct-execute audit fields on %s: %v", executionID, err) + } + + if err := h.purchase.ApproveAndExecute(ctx, executionID, session.Email); err != nil { + logging.Errorf("purchase[%s]: directExecutePurchase failed after %s: %v", + executionID, time.Since(t0), err) + return nil, NewClientError(409, fmt.Sprintf("execution %s could not be direct-executed: %v", executionID, err)) + } + + logging.Infof("purchase[%s]: directExecutePurchase completed in %s", executionID, time.Since(t0)) + return map[string]any{ + "execution_id": executionID, + "status": "completed", + "recommendation_count": len(execution.Recommendations), + "total_upfront_cost": execution.TotalUpfrontCost, + "estimated_savings": execution.EstimatedSavings, + "direct_execute": true, + "message": "Purchase executed immediately (direct-execute permission).", + }, nil } // archeraEducationURL returns dashboardBase + "/archera-insurance", or "" when diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index e7681a438..88c3bf517 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -2833,3 +2833,178 @@ func TestHandler_approvalResponseRecipient_TrimsNonEmptyValue(t *testing.T) { assert.Equal(t, "cristi@example.com", result, "globalNotify with surrounding whitespace must be returned trimmed") } + +// --- Direct-execute path tests (issue #289) --- + +// recBody is a minimal valid recommendations JSON suitable for executePurchase +// handler tests. Reused across the direct-execute suite to keep setup compact. +const directExecRecBody = `{ + "recommendations": [ + { + "id": "rec-1", + "provider": "aws", + "service": "ec2", + "count": 1, + "term": 1, + "payment": "all-upfront", + "upfront_cost": 500.0, + "savings": 100.0 + } + ], + "execute_mode": "direct" +}` + +// setupDirectExecMocks wires the minimal store mocks needed by the +// executePurchase handler up to (but not including) the email/execute branch. +func setupDirectExecMocks(ctx context.Context, store *MockConfigStore) { + store.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) + store.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) +} + +// TestHandler_executePurchase_DirectExec_NoPermission verifies the fail-closed +// gate: a session with the base execute:purchases verb but without +// execute-any or execute-own on purchases receives a 403 when it requests +// execute_mode="direct". The handler must not fall through to the approval +// path (issue #289). +func TestHandler_executePurchase_DirectExec_NoPermission(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + userSession := &Session{ + UserID: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb", + Email: "user@example.com", + Role: "user", + } + mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil) + // Base execute:purchases grant — passes the validateExecutePurchaseRequest + // gate but does not carry execute-any or execute-own for the direct path. + mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute", "purchases").Return(true, nil) + mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute-any", "purchases").Return(false, nil) + mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute-own", "purchases").Return(false, nil) + // Scope check: no allowed_accounts restriction for this test. + mockAuth.On("GetAllowedAccountsAPI", ctx, userSession.UserID).Return([]string{}, nil) + setupDirectExecMocks(ctx, mockStore) + + handler := &Handler{config: mockStore, auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer user-token"}, + Body: directExecRecBody, + } + _, err := handler.executePurchase(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError") + assert.Equal(t, 403, ce.code) + assert.Contains(t, ce.Error(), "execute-any or execute-own") +} + +// TestHandler_executePurchase_DirectExec_ExecuteAny verifies that a session +// with execute-any:purchases can direct-execute a purchase (no ownership +// check). The handler must call ApproveAndExecute and return status=completed. +func TestHandler_executePurchase_DirectExec_ExecuteAny(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + mockPurchase := new(MockPurchaseManager) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + t.Cleanup(func() { mockPurchase.AssertExpectations(t) }) + + adminSession := &Session{ + UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + Email: "admin@example.com", + Role: "admin", + } + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + // Admin role short-circuits the permission matrix (no HasPermissionAPI call + // expected) but we still need ApproveAndExecute on the purchase mock. + mockPurchase.On("ApproveAndExecute", ctx, mock.AnythingOfType("string"), adminSession.Email).Return(nil) + setupDirectExecMocks(ctx, mockStore) + + handler := &Handler{config: mockStore, auth: mockAuth, purchase: mockPurchase} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: directExecRecBody, + } + result, err := handler.executePurchase(ctx, req) + require.NoError(t, err) + resultMap := result.(map[string]any) + assert.Equal(t, "completed", resultMap["status"]) + assert.Equal(t, true, resultMap["direct_execute"]) + assert.Equal(t, 500.0, resultMap["total_upfront_cost"]) +} + +// TestHandler_executePurchase_DirectExec_ExecuteOwn_Owner verifies that a +// session with only execute-own:purchases can direct-execute when the +// execution's creator matches the session user. +func TestHandler_executePurchase_DirectExec_ExecuteOwn_Owner(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + mockPurchase := new(MockPurchaseManager) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + t.Cleanup(func() { mockPurchase.AssertExpectations(t) }) + + ownerID := "cccccccc-cccc-cccc-cccc-cccccccccccc" + ownerSession := &Session{ + UserID: ownerID, + Email: "owner@example.com", + Role: "user", + } + mockAuth.On("ValidateSession", ctx, "owner-token").Return(ownerSession, nil) + mockAuth.On("HasPermissionAPI", ctx, ownerID, "execute", "purchases").Return(true, nil) + mockAuth.On("HasPermissionAPI", ctx, ownerID, "execute-any", "purchases").Return(false, nil) + mockAuth.On("HasPermissionAPI", ctx, ownerID, "execute-own", "purchases").Return(true, nil) + mockAuth.On("GetAllowedAccountsAPI", ctx, ownerID).Return([]string{}, nil) + mockPurchase.On("ApproveAndExecute", ctx, mock.AnythingOfType("string"), ownerSession.Email).Return(nil) + setupDirectExecMocks(ctx, mockStore) + + handler := &Handler{config: mockStore, auth: mockAuth, purchase: mockPurchase} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer owner-token"}, + Body: directExecRecBody, + } + result, err := handler.executePurchase(ctx, req) + require.NoError(t, err) + resultMap := result.(map[string]any) + assert.Equal(t, "completed", resultMap["status"]) + assert.Equal(t, true, resultMap["direct_execute"]) +} + +// TestHandler_executePurchase_DirectExec_ExecuteOwn_NonOwner verifies the +// execute-own ownership gate: a session with execute-own:purchases but a +// different UserID than the execution creator receives a 403. +// +// Because executePurchase resolves the creator from the session (via +// resolveCreatorUserID), a non-owner scenario is produced by using a +// session whose UserID is non-empty and valid but differs from the creator +// that gets stamped. In practice creatorID == session.UserID always after a +// fresh submit (the creator IS the submitter), so the execute-own non-owner +// case can only arise when execute-own is misapplied to a pre-existing +// execution (not the fresh-submit path). We test the authorizeSessionExecuteDirect +// function directly to cover the non-owner branch. +func TestHandler_authorizeSessionExecuteDirect_ExecuteOwn_NonOwner(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + sessionUserID := "dddddddd-dddd-dddd-dddd-dddddddddddd" + differentCreatorID := "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee" + session := &Session{UserID: sessionUserID, Role: "user"} + + mockAuth.On("HasPermissionAPI", ctx, sessionUserID, "execute-any", "purchases").Return(false, nil) + mockAuth.On("HasPermissionAPI", ctx, sessionUserID, "execute-own", "purchases").Return(true, nil) + + handler := &Handler{auth: mockAuth} + err := handler.authorizeSessionExecuteDirect(ctx, session, differentCreatorID) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError") + assert.Equal(t, 403, ce.code) + assert.Contains(t, ce.Error(), "execute-own requires you to be the creator") +} diff --git a/internal/auth/types.go b/internal/auth/types.go index 5519cd763..c5ecc5fc3 100644 --- a/internal/auth/types.go +++ b/internal/auth/types.go @@ -362,6 +362,28 @@ const ( // contact_email gate (PR #101), not these verbs. ActionApproveOwn = "approve-own" ActionApproveAny = "approve-any" + // ActionExecuteOwn / ActionExecuteAny gate the direct-execute shortcut + // on the Recommendations page (issue #289). A holder skips the approval + // email and immediately commits the purchase, with audit fields + // (executed_by_user_id, executed_at, pre_approval_skip_reason) stamped + // on the execution row. + // + // * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers + // both verbs. + // * RoleUser — NO default grant. This is a finance-impacting permission + // that must be explicitly granted per-user/per-role. Even trusted + // users submit via the approval flow by default; only deliberately + // privileged accounts should hold this verb. + // * RoleReadOnly — neither verb. + // + // execute-own: allows direct-execute only for executions where + // created_by_user_id == session user (the user drafted the purchase + // themselves). Like approve-own, legacy rows with NULL creator are + // unreachable for non-admins via this verb. + // execute-any: allows direct-execute regardless of creator; no ownership + // check. No default non-admin grant; add to a custom operator group. + ActionExecuteOwn = "execute-own" + ActionExecuteAny = "execute-any" ) // Predefined resources diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index 6905f4105..804be3df6 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -723,8 +723,9 @@ func (s *PostgresStore) SavePurchaseExecutionTx(ctx context.Context, tx pgx.Tx, total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22) + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25) ON CONFLICT (execution_id) DO UPDATE SET status = $3, notification_sent = $6, @@ -742,6 +743,9 @@ func (s *PostgresStore) SavePurchaseExecutionTx(ctx context.Context, tx pgx.Tx, capacity_percent = $18, retry_execution_id = $20, approval_token_expires_at = $22, + executed_by_user_id = $23, + executed_at = $24, + pre_approval_skip_reason = $25, updated_at = NOW() ` @@ -788,6 +792,9 @@ func (s *PostgresStore) SavePurchaseExecutionTx(ctx context.Context, tx pgx.Tx, execution.RetryExecutionID, execution.RetryAttemptN, execution.ApprovalTokenExpiresAt, + execution.ExecutedByUserID, + execution.ExecutedAt, + execution.PreApprovalSkipReason, ) if err != nil { @@ -810,7 +817,8 @@ func (s *PostgresStore) TransitionExecutionStatus(ctx context.Context, execution total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason ` records, err := s.queryExecutions(ctx, query, executionID, toStatus, fromStatuses) @@ -913,7 +921,8 @@ func (s *PostgresStore) GetExecutionsByStatuses(ctx context.Context, statuses [] total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason FROM purchase_executions WHERE status = ANY($1) ORDER BY scheduled_date DESC @@ -936,7 +945,8 @@ func (s *PostgresStore) GetStaleApprovedExecutions(ctx context.Context, olderTha total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason FROM purchase_executions WHERE status = 'approved' AND updated_at < NOW() - $1::interval ` @@ -976,7 +986,8 @@ func (s *PostgresStore) ListStuckExecutions(ctx context.Context, statuses []stri total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason FROM purchase_executions WHERE status = ANY($1) AND updated_at < NOW() - $2::interval @@ -995,7 +1006,8 @@ func (s *PostgresStore) GetPendingExecutions(ctx context.Context) ([]PurchaseExe total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason FROM purchase_executions WHERE status IN ('pending', 'notified') AND (expires_at IS NULL OR expires_at > NOW()) @@ -1017,7 +1029,8 @@ func (s *PostgresStore) GetPendingExecutionsTx(ctx context.Context, tx pgx.Tx) ( total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason FROM purchase_executions WHERE status IN ('pending', 'notified') AND (expires_at IS NULL OR expires_at > NOW()) @@ -1041,7 +1054,8 @@ func (s *PostgresStore) GetExecutionByID(ctx context.Context, executionID string total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason FROM purchase_executions WHERE execution_id = $1 ` @@ -1066,7 +1080,8 @@ func (s *PostgresStore) GetExecutionByPlanAndDate(ctx context.Context, planID st total_upfront_cost, estimated_savings, completed_at, error, expires_at, cloud_account_id, source, approved_by, cancelled_by, capacity_percent, created_by_user_id, retry_execution_id, retry_attempt_n, - approval_token_expires_at + approval_token_expires_at, + executed_by_user_id, executed_at, pre_approval_skip_reason FROM purchase_executions WHERE plan_id = $1 AND scheduled_date = $2 ` @@ -1150,7 +1165,7 @@ func scanExecutionRows(rows pgx.Rows) ([]PurchaseExecution, error) { for rows.Next() { var exec PurchaseExecution var recommendationsJSON []byte - var notifSent, completedAt, expiresAt, tokenExpiresAt sql.NullTime + var notifSent, completedAt, expiresAt, tokenExpiresAt, executedAt sql.NullTime // plan_id is nullable since migration 000033 (direct-execute // rows from the Recommendations page have no originating plan). var planID sql.NullString @@ -1178,6 +1193,9 @@ func scanExecutionRows(rows pgx.Rows) ([]PurchaseExecution, error) { &exec.RetryExecutionID, &exec.RetryAttemptN, &tokenExpiresAt, + &exec.ExecutedByUserID, + &executedAt, + &exec.PreApprovalSkipReason, ) if err != nil { return nil, fmt.Errorf("failed to scan execution: %w", err) @@ -1205,6 +1223,9 @@ func scanExecutionRows(rows pgx.Rows) ([]PurchaseExecution, error) { if tokenExpiresAt.Valid { exec.ApprovalTokenExpiresAt = &tokenExpiresAt.Time } + if executedAt.Valid { + exec.ExecutedAt = &executedAt.Time + } executions = append(executions, exec) } diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go index f48076627..d29e06fff 100644 --- a/internal/config/store_postgres_pgxmock_test.go +++ b/internal/config/store_postgres_pgxmock_test.go @@ -388,6 +388,7 @@ func TestPGXMock_GetExecutionByID_Success(t *testing.T) { "cloud_account_id", "source", "approved_by", "cancelled_by", "capacity_percent", "created_by_user_id", "retry_execution_id", "retry_attempt_n", "approval_token_expires_at", + "executed_by_user_id", "executed_at", "pre_approval_skip_reason", } rows := pgxmock.NewRows(cols).AddRow( "plan-1", "exec-1", "pending", 1, now, @@ -396,6 +397,7 @@ func TestPGXMock_GetExecutionByID_Success(t *testing.T) { nil, "", nil, nil, 100, nil, nil, 0, sql.NullTime{}, + nil, sql.NullTime{}, nil, ) mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg()).WillReturnRows(rows) @@ -428,6 +430,7 @@ func TestPGXMock_GetExecutionByID_WithTimestamps(t *testing.T) { "cloud_account_id", "source", "approved_by", "cancelled_by", "capacity_percent", "created_by_user_id", "retry_execution_id", "retry_attempt_n", "approval_token_expires_at", + "executed_by_user_id", "executed_at", "pre_approval_skip_reason", } successorID := "exec-3" rows := pgxmock.NewRows(cols).AddRow( @@ -440,6 +443,7 @@ func TestPGXMock_GetExecutionByID_WithTimestamps(t *testing.T) { // values exercise the scan path for both new columns. nil, &successorID, 2, sql.NullTime{}, + nil, sql.NullTime{}, nil, ) mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg()).WillReturnRows(rows) @@ -1567,6 +1571,7 @@ func stuckExecRow(execID, status string, scheduled time.Time) []any { nil, "cudly-web", nil, nil, 100, nil, nil, 0, sql.NullTime{}, + nil, sql.NullTime{}, nil, } } @@ -1578,6 +1583,7 @@ func stuckExecCols() []string { "cloud_account_id", "source", "approved_by", "cancelled_by", "capacity_percent", "created_by_user_id", "retry_execution_id", "retry_attempt_n", "approval_token_expires_at", + "executed_by_user_id", "executed_at", "pre_approval_skip_reason", } } diff --git a/internal/config/types.go b/internal/config/types.go index caf518f68..7b97666f0 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -264,6 +264,20 @@ type PurchaseExecution struct { // field is non-nil). Migration 000051 adds the column; new rows // always carry a non-nil value. ApprovalTokenExpiresAt *time.Time `json:"approval_token_expires_at,omitempty" dynamodbav:"approval_token_expires_at,omitempty"` + // ExecutedByUserID is the UUID of the session user who triggered a + // direct-execute (issue #289, execute-any/execute-own). NULL on rows + // that went through the normal approval flow. Non-null signals the + // approval step was intentionally skipped by an authorized operator. + // Migration 000058 adds the column. + ExecutedByUserID *string `json:"executed_by_user_id,omitempty" dynamodbav:"executed_by_user_id,omitempty"` + // ExecutedAt is the UTC timestamp when the direct-execute path fired. + // NULL for rows on the normal approval flow. Migration 000058. + ExecutedAt *time.Time `json:"executed_at,omitempty" dynamodbav:"executed_at,omitempty"` + // PreApprovalSkipReason is a human-readable token describing why the + // approval step was skipped. For direct-execute rows it is the literal + // string "direct-execute permission". NULL on every normal-flow row. + // Migration 000058. + PreApprovalSkipReason *string `json:"pre_approval_skip_reason,omitempty" dynamodbav:"pre_approval_skip_reason,omitempty"` } // IsCancelable reports whether an execution may still be cancelled. Only the diff --git a/internal/database/postgres/migrations/000058_purchase_executions_direct_execute_audit.down.sql b/internal/database/postgres/migrations/000058_purchase_executions_direct_execute_audit.down.sql new file mode 100644 index 000000000..1d69e2c58 --- /dev/null +++ b/internal/database/postgres/migrations/000058_purchase_executions_direct_execute_audit.down.sql @@ -0,0 +1,6 @@ +DROP INDEX IF EXISTS idx_executions_direct_execute; + +ALTER TABLE purchase_executions + DROP COLUMN IF EXISTS pre_approval_skip_reason, + DROP COLUMN IF EXISTS executed_at, + DROP COLUMN IF EXISTS executed_by_user_id; diff --git a/internal/database/postgres/migrations/000058_purchase_executions_direct_execute_audit.up.sql b/internal/database/postgres/migrations/000058_purchase_executions_direct_execute_audit.up.sql new file mode 100644 index 000000000..d23985c00 --- /dev/null +++ b/internal/database/postgres/migrations/000058_purchase_executions_direct_execute_audit.up.sql @@ -0,0 +1,30 @@ +-- Audit columns for direct-execute purchases (issue #289). +-- +-- When a session with execute-any:purchases or execute-own:purchases +-- triggers an immediate purchase (bypassing the approval email), the +-- handler stamps these three columns so finance auditors can later ask +-- "who direct-executed this purchase, when, and why was approval skipped?" +-- +-- All three are nullable TEXT / TIMESTAMPTZ so: +-- * Existing rows (normal approval flow) are untouched (NULL = approval +-- flow, non-NULL = direct-execute shortcut). +-- * Legacy rows from before this migration read as NULL and are treated +-- as normal-flow rows by the application. +-- +-- executed_by_user_id: UUID of the session user who fired the direct-execute. +-- FK to users.id ON DELETE SET NULL mirrors the approved_by / cancelled_by +-- pattern from migration 000035. +-- executed_at: UTC timestamp the direct-execute fired. +-- pre_approval_skip_reason: short literal, always "direct-execute permission" +-- today; a TEXT column leaves room for future skip reasons without a schema +-- change. + +ALTER TABLE purchase_executions + ADD COLUMN IF NOT EXISTS executed_by_user_id UUID REFERENCES users(id) ON DELETE SET NULL, + ADD COLUMN IF NOT EXISTS executed_at TIMESTAMPTZ, + ADD COLUMN IF NOT EXISTS pre_approval_skip_reason TEXT; + +-- Partial index speeds up audit queries that filter for direct-execute rows. +CREATE INDEX IF NOT EXISTS idx_executions_direct_execute + ON purchase_executions (executed_by_user_id) + WHERE executed_by_user_id IS NOT NULL;