feat(expenses): admin_notes column and lambda S3 object permissions

.github/workflows/lambda-tests.yml

@@ -213,19 +213,25 @@ jobs:
           for f in $files; do
             [ -f "$f" ] || continue
             # '-- allow-destructive: <reason>' opts a file out, deliberately loudly.
+            # Read from the raw file: this opt-out is itself a comment.
             if grep -qi '^-- allow-destructive:' "$f"; then
               echo "::warning file=$f::opted out of the destructive-SQL check"
               continue
             fi
-            if grep -inE '\b(DROP[[:space:]]+SCHEMA|DROP[[:space:]]+DATABASE|TRUNCATE|DROP[[:space:]]+TABLE|DROP[[:space:]]+COLUMN)\b' "$f"; then
+            # Scan SQL, not prose. `make new-migration` scaffolds a header that
+            # says "CREATE INDEX CONCURRENTLY / VACUUM will not work here", and
+            # matching that failed every generated migration. Blanking comments
+            # rather than dropping the lines keeps grep -n line numbers honest.
+            sql=$(sed 's/--.*//' "$f")
+            if printf '%s\n' "$sql" | grep -inE '\b(DROP[[:space:]]+SCHEMA|DROP[[:space:]]+DATABASE|TRUNCATE|DROP[[:space:]]+TABLE|DROP[[:space:]]+COLUMN)\b'; then
               echo "::error file=$f::destructive statement. Migrations run against production BEFORE the new lambda code deploys, so the currently deployed code would hit the changed schema. See the expand/contract rules in apps/backend/db/README.md. Add '-- allow-destructive: <reason>' only for a contract-phase migration whose expand phase is already live."
               fail=1
             fi
-            if grep -inE 'CONCURRENTLY' "$f"; then
+            if printf '%s\n' "$sql" | grep -inE 'CONCURRENTLY'; then
               echo "::error file=$f::CONCURRENTLY cannot run inside a transaction, and the migrator wraps the run in one. This database is tiny -- use a plain CREATE INDEX."
               fail=1
             fi
-            if grep -inE '^[[:space:]]*INSERT[[:space:]]+INTO[[:space:]]+(branch\.)?users\b' "$f"; then
+            if printf '%s\n' "$sql" | grep -inE '^[[:space:]]*INSERT[[:space:]]+INTO[[:space:]]+(branch\.)?users\b'; then
               echo "::warning file=$f::inserting users in a migration puts rows in PRODUCTION. Seeded users with a NULL cognito_sub are claimable by POST /auth/register, so this can hand someone an account. Dev seed rows belong in apps/backend/db/seed.sql."
             fi
           done

apps/backend/db/migrations/20260812012951_add_expenditure_admin_notes.sql

@@ -0,0 +1,16 @@
+-- add_expenditure_admin_notes
+--
+-- Every pending migration runs inside a SINGLE transaction, with
+-- search_path = branch, public -- so table names can be unqualified, and
+-- CREATE INDEX CONCURRENTLY / VACUUM will not work here.
+--
+-- This migration is applied to PRODUCTION automatically when the PR merges,
+-- BEFORE the new lambda code is deployed. It must be safe for the code that is
+-- live right now: additive changes only. See apps/backend/db/README.md for the
+-- expand/contract rules that destructive changes need.
+--
+-- Forward-only: there is no rollback. Fix a mistake with a new migration, and
+-- never edit a migration that has been merged -- someone has already run it.
+-- Do not use IF NOT EXISTS: you want a failure, not silent drift.
+
+ALTER TABLE expenditures ADD COLUMN admin_notes TEXT;

infrastructure/aws/README.md

@@ -49,6 +49,7 @@ No modules.
 | [aws_iam_role_policy.ci_plan_state_lock](https://registry.terraform.io/providers/hashicorp/aws/6.14.1/docs/resources/iam_role_policy) | resource |
 | [aws_iam_role_policy.ci_preview](https://registry.terraform.io/providers/hashicorp/aws/6.14.1/docs/resources/iam_role_policy) | resource |
 | [aws_iam_role_policy.lambda_cognito_admin](https://registry.terraform.io/providers/hashicorp/aws/6.14.1/docs/resources/iam_role_policy) | resource |
+| [aws_iam_role_policy.lambda_s3_objects](https://registry.terraform.io/providers/hashicorp/aws/6.14.1/docs/resources/iam_role_policy) | resource |
 | [aws_iam_role_policy_attachment.ci_apply_admin](https://registry.terraform.io/providers/hashicorp/aws/6.14.1/docs/resources/iam_role_policy_attachment) | resource |
 | [aws_iam_role_policy_attachment.ci_plan_readonly](https://registry.terraform.io/providers/hashicorp/aws/6.14.1/docs/resources/iam_role_policy_attachment) | resource |
 | [aws_iam_role_policy_attachment.lambda_basic](https://registry.terraform.io/providers/hashicorp/aws/6.14.1/docs/resources/iam_role_policy_attachment) | resource |

infrastructure/aws/lambda.tf

@@ -46,6 +46,27 @@ resource "aws_iam_role_policy" "lambda_cognito_admin" {
   })
 }
 
+# The expenditures lambda presigns receipt uploads and downloads. A presigned
+# URL carries the signer's permissions, so the role needs both PutObject and
+# GetObject or the browser's PUT/GET fails AccessDenied.
+resource "aws_iam_role_policy" "lambda_s3_objects" {
+  name = "branch-lambda-s3-objects"
+  role = aws_iam_role.lambda_role.id
+
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Sid    = "LambdaReportsBucketObjects"
+      Effect = "Allow"
+      Action = [
+        "s3:PutObject",
+        "s3:GetObject",
+      ]
+      Resource = "${aws_s3_bucket.reports_bucket.arn}/*"
+    }]
+  })
+}
+
 # Get AWS account ID for unique bucket naming
 data "aws_caller_identity" "current" {}
 

shared/types/db-types.d.ts

@@ -30,6 +30,7 @@ export interface BranchDonors {
 }
 
 export interface BranchExpenditures {
+  admin_notes: string | null;
   amount: Numeric;
   category: string | null;
   created_at: Generated<Timestamp | null>;