From ae02fd44732cead88115918612836a90f9e3b864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Zori=C4=87?= Date: Tue, 12 May 2026 11:43:53 +0200 Subject: [PATCH 1/2] chore: add SOURCE/TARGET_ACCOUNT_ID to all .env.example files The cross-account S3 access check reads accountId from config; without these vars populated the check is skipped silently. Co-Authored-By: Claude Sonnet 4.6 --- package.json | 2 +- projects/v5-to-v6/.env.example | 2 + src/commands/run/handler.ts | 15 ------ src/domain/pipeline/abstractions/Processor.ts | 8 --- src/domain/transform/filters.ts | 4 ++ src/features/S3Processor/S3Processor.ts | 49 +++++++++++++------ src/presets/v5-to-v6-ddb.ts | 11 +++++ templates/internal-project/.env.example | 2 + templates/projects/example/.env.example | 2 + 9 files changed, 55 insertions(+), 40 deletions(-) diff --git a/package.json b/package.json index 13175a7a..4bcb40bc 100644 --- a/package.json +++ b/package.json @@ -13,8 +13,8 @@ "transfer": "tsx src/cli.ts", "ts-check": "yarn tsc --noEmit", "test": "vitest run", - "test:coverage": "vitest run --coverage --coverage.include=\"src/**/**\"", "test:watch": "vitest", + "test:coverage": "vitest run --coverage --coverage.include=\"src/**/**\"", "format": "oxfmt", "format:fix": "oxfmt", "format:check": "oxfmt --check", diff --git a/projects/v5-to-v6/.env.example b/projects/v5-to-v6/.env.example index 0a711b73..8ef187e4 100644 --- a/projects/v5-to-v6/.env.example +++ b/projects/v5-to-v6/.env.example @@ -17,6 +17,7 @@ SOURCE_REGION=us-east-1 # SOURCE_AWS_SESSION_TOKEN= SOURCE_DDB_TABLE=webiny-v5-table SOURCE_S3_BUCKET=webiny-v5-files +SOURCE_ACCOUNT_ID= SOURCE_AUDIT_LOGS_TABLE= SOURCE_OS_TABLE=webiny-v5-es-table @@ -28,6 +29,7 @@ TARGET_REGION=us-east-1 # TARGET_AWS_SESSION_TOKEN= TARGET_DDB_TABLE=webiny-v6-table TARGET_S3_BUCKET=webiny-v6-files +TARGET_ACCOUNT_ID= TARGET_AUDIT_LOGS_TABLE=webiny-v6-audit-logs TARGET_OS_TABLE=webiny-v6-es-table TARGET_OS_ENDPOINT=https://search-my-domain.us-east-1.es.amazonaws.com diff --git a/src/commands/run/handler.ts b/src/commands/run/handler.ts index 44ac22aa..c0a28366 100644 --- a/src/commands/run/handler.ts +++ b/src/commands/run/handler.ts @@ -2,7 +2,6 @@ import { fileURLToPath } from "node:url"; import { join } from "node:path"; import { readdir, readFile } from "node:fs/promises"; import { execa } from "execa"; -import { confirm } from "@inquirer/prompts"; import { bootstrap } from "~/bootstrap.ts"; import { formatError } from "~/base/index.ts"; import type { RunStats } from "~/features/PipelineRunner/abstractions/PipelineRunner.ts"; @@ -123,20 +122,6 @@ export async function handler( throw new AccessCheckError(blocked.length); } - const guardWarnings = ( - await Promise.all(runner.getProcessors().map(p => p.getGuardWarning?.() ?? null)) - ).filter((w): w is string => w !== null); - - if (guardWarnings.length > 0) { - for (const warning of guardWarnings) { - logger.warn(warning); - } - const proceed = await confirm({ message: "Proceed with transfer?" }); - if (!proceed) { - process.exit(0); - } - } - const beforeHook = container.resolve(BeforeTransferHook); logger.info("Running before-transfer hooks..."); await beforeHook.execute(); diff --git a/src/domain/pipeline/abstractions/Processor.ts b/src/domain/pipeline/abstractions/Processor.ts index b0e78eac..8b7f4977 100644 --- a/src/domain/pipeline/abstractions/Processor.ts +++ b/src/domain/pipeline/abstractions/Processor.ts @@ -50,14 +50,6 @@ interface IProcessor< */ checkAccess(): Promise; - /** - * Pre-transfer guard check. Called in the orchestrator before any segment - * workers are spawned. Return a human-readable warning string when the - * processor detects a condition that requires user confirmation (e.g. - * cross-account S3 copy), or null to proceed silently. - */ - getGuardWarning?(): Promise; - /** * Drain the processor's commands from the bag and write to target. The * act of calling commands.get(key) marks that key as "claimed" — the diff --git a/src/domain/transform/filters.ts b/src/domain/transform/filters.ts index 2df11b19..be149188 100644 --- a/src/domain/transform/filters.ts +++ b/src/domain/transform/filters.ts @@ -127,3 +127,7 @@ export const isFormBuilderRecord = (record: BaseRecord): boolean => { } return type.startsWith("fb.form.") || type.startsWith("fb.formSubmission"); }; + +export const isAdminUser = (record: BaseRecord): boolean => { + return record.PK.includes("#SECURITY#USER#") && record.GSI1_PK === "securityRole#full-access"; +}; diff --git a/src/features/S3Processor/S3Processor.ts b/src/features/S3Processor/S3Processor.ts index 97f51fca..ce848fee 100644 --- a/src/features/S3Processor/S3Processor.ts +++ b/src/features/S3Processor/S3Processor.ts @@ -43,21 +43,13 @@ class S3ProcessorImpl implements Processor.Interface< // No onEnd — S3 has no sensible per-record default. Transformers call // ctx.copyFile(...) explicitly when they want to emit a copy. - public async getGuardWarning(): Promise { + public async checkAccess(): Promise { const sourceAccount = this.config.source.accountId || null; const targetAccount = this.config.target.accountId || null; - if (sourceAccount === null || targetAccount === null || sourceAccount === targetAccount) { - return null; - } - return ( - `S3 file copy is cross-account: source account ${sourceAccount} → target account ${targetAccount}.\n` + - `CopyObject runs with target credentials — the source bucket "${this.config.source.s3.bucket}"\n` + - `must have a bucket policy granting account ${targetAccount} s3:GetObject access.` - ); - } + const isCrossAccount = + sourceAccount !== null && targetAccount !== null && sourceAccount !== targetAccount; - public async checkAccess(): Promise { - const [sourceEntry, targetEntry] = await Promise.all([ + const checks: Promise[] = [ this.headBucket( this.config.source.credentials, this.config.source.region, @@ -70,17 +62,42 @@ class S3ProcessorImpl implements Processor.Interface< this.config.target.s3.bucket, "target" ) - ]); - return [sourceEntry, targetEntry]; + ]; + + if (isCrossAccount) { + checks.push( + this.headBucketWithLabel( + this.config.target.credentials, + this.config.source.region, + this.config.source.s3.bucket, + `S3 cross-account read (target credentials → source bucket: ${this.config.source.s3.bucket})` + ) + ); + } + + return Promise.all(checks); } - private async headBucket( + private headBucket( credentials: MigrationConfig.Interface["source"]["credentials"], region: string, bucket: string, side: string ): Promise { - const label = `S3 ${side} bucket: ${bucket}`; + return this.headBucketWithLabel( + credentials, + region, + bucket, + `S3 ${side} bucket: ${bucket}` + ); + } + + private async headBucketWithLabel( + credentials: MigrationConfig.Interface["source"]["credentials"], + region: string, + bucket: string, + label: string + ): Promise { const client = new S3({ region, credentials: credentials as never }); try { await client.headBucket({ Bucket: bucket }); diff --git a/src/presets/v5-to-v6-ddb.ts b/src/presets/v5-to-v6-ddb.ts index d6473fbe..f4a401bc 100644 --- a/src/presets/v5-to-v6-ddb.ts +++ b/src/presets/v5-to-v6-ddb.ts @@ -8,6 +8,7 @@ import { createFilter } from "~/domain/pipeline/Filter.ts"; import { byType, isAcoSearchRecord, + isAdminUser, isAuditLogEntry, isBackgroundTask, isBuiltInSecurityRole, @@ -287,6 +288,15 @@ export default createTransferPreset({ .blackhole() .build(); + const adminUsers = factory + .create({ + name: "AdminUsers", + scanner: DdbScanner, + processors: [DdbProcessor] + }) + .filter(createFilter(isAdminUser)) + .build(); + // ======================================================================== // Register pipelines with runner // IMPORTANT: Order matters due to first-match-wins behavior @@ -305,6 +315,7 @@ export default createTransferPreset({ .register(cmsModels) .register(folderPermissions) .register(cmsEntries) + .register(adminUsers) .register(formBuilderRecords); } }); diff --git a/templates/internal-project/.env.example b/templates/internal-project/.env.example index fd847727..40496648 100644 --- a/templates/internal-project/.env.example +++ b/templates/internal-project/.env.example @@ -20,6 +20,7 @@ SOURCE_REGION={{SOURCE_REGION}} SOURCE_DDB_TABLE={{SOURCE_DDB_TABLE}} SOURCE_S3_BUCKET={{SOURCE_S3_BUCKET}} +SOURCE_ACCOUNT_ID={{SOURCE_ACCOUNT_ID}} SOURCE_AUDIT_LOGS_TABLE={{SOURCE_AUDIT_LOGS_TABLE}} SOURCE_OS_TABLE={{SOURCE_OS_TABLE}} @@ -34,6 +35,7 @@ TARGET_REGION={{TARGET_REGION}} TARGET_DDB_TABLE={{TARGET_DDB_TABLE}} TARGET_S3_BUCKET={{TARGET_S3_BUCKET}} +TARGET_ACCOUNT_ID={{TARGET_ACCOUNT_ID}} TARGET_AUDIT_LOGS_TABLE={{TARGET_AUDIT_LOGS_TABLE}} TARGET_OS_TABLE={{TARGET_OS_TABLE}} TARGET_OS_ENDPOINT={{TARGET_OS_ENDPOINT}} diff --git a/templates/projects/example/.env.example b/templates/projects/example/.env.example index 3e17406d..a3697983 100644 --- a/templates/projects/example/.env.example +++ b/templates/projects/example/.env.example @@ -22,6 +22,7 @@ SOURCE_REGION={{SOURCE_REGION}} SOURCE_DDB_TABLE={{SOURCE_DDB_TABLE}} SOURCE_S3_BUCKET={{SOURCE_S3_BUCKET}} +SOURCE_ACCOUNT_ID={{SOURCE_ACCOUNT_ID}} SOURCE_OS_TABLE={{SOURCE_OS_TABLE}} # --- Target environment ------------------------------------------------ @@ -35,6 +36,7 @@ TARGET_REGION={{TARGET_REGION}} TARGET_DDB_TABLE={{TARGET_DDB_TABLE}} TARGET_S3_BUCKET={{TARGET_S3_BUCKET}} +TARGET_ACCOUNT_ID={{TARGET_ACCOUNT_ID}} TARGET_OS_TABLE={{TARGET_OS_TABLE}} TARGET_OS_ENDPOINT={{TARGET_OS_ENDPOINT}} TARGET_OS_INDEX_PREFIX={{TARGET_OS_INDEX_PREFIX}} From 609fec60188e3be4fa5e6dcbb035f512f720ed32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Zori=C4=87?= Date: Tue, 12 May 2026 11:45:27 +0200 Subject: [PATCH 2/2] fix: print actionable hint when cross-account S3 access check fails AccessCheck.Entry gains an optional hint field. When a DENIED or MISSING entry carries a hint it is printed on the next line so the user knows exactly what bucket policy to add (target account needs s3:GetObject on source bucket because CopyObject runs with target credentials). Co-Authored-By: Claude Sonnet 4.6 --- src/commands/run/handler.ts | 6 ++++++ src/domain/pipeline/abstractions/Processor.ts | 1 + src/features/S3Processor/S3Processor.ts | 11 +++++++---- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/commands/run/handler.ts b/src/commands/run/handler.ts index c0a28366..1f5b6b31 100644 --- a/src/commands/run/handler.ts +++ b/src/commands/run/handler.ts @@ -109,8 +109,14 @@ export async function handler( logger.info(` ok ${entry.label}`); } else if (entry.status === "denied") { logger.error(` DENIED ${entry.label}`); + if (entry.hint) { + logger.error(` ${entry.hint}`); + } } else if (entry.status === "missing") { logger.error(` MISSING ${entry.label}`); + if (entry.hint) { + logger.error(` ${entry.hint}`); + } } else { logger.warn(` unknown ${entry.label}`); } diff --git a/src/domain/pipeline/abstractions/Processor.ts b/src/domain/pipeline/abstractions/Processor.ts index 8b7f4977..1f580d46 100644 --- a/src/domain/pipeline/abstractions/Processor.ts +++ b/src/domain/pipeline/abstractions/Processor.ts @@ -76,6 +76,7 @@ export namespace AccessCheck { export interface Entry { label: string; status: Status; + hint?: string; } export type Report = Entry[]; diff --git a/src/features/S3Processor/S3Processor.ts b/src/features/S3Processor/S3Processor.ts index ce848fee..11480715 100644 --- a/src/features/S3Processor/S3Processor.ts +++ b/src/features/S3Processor/S3Processor.ts @@ -70,7 +70,9 @@ class S3ProcessorImpl implements Processor.Interface< this.config.target.credentials, this.config.source.region, this.config.source.s3.bucket, - `S3 cross-account read (target credentials → source bucket: ${this.config.source.s3.bucket})` + `S3 cross-account read (target credentials → source bucket: ${this.config.source.s3.bucket})`, + `S3 CopyObject runs with target credentials. Add a bucket policy on ` + + `"${this.config.source.s3.bucket}" granting s3:GetObject to account ${targetAccount}.` ) ); } @@ -96,7 +98,8 @@ class S3ProcessorImpl implements Processor.Interface< credentials: MigrationConfig.Interface["source"]["credentials"], region: string, bucket: string, - label: string + label: string, + hint?: string ): Promise { const client = new S3({ region, credentials: credentials as never }); try { @@ -104,12 +107,12 @@ class S3ProcessorImpl implements Processor.Interface< return { label, status: "ok" }; } catch (error) { if (isAccessDeniedError(error)) { - return { label, status: "denied" }; + return { label, status: "denied", hint }; } const errName = (error as AwsErrorLike).name ?? (error as AwsErrorLike).code; const httpStatus = (error as AwsErrorLike).$metadata?.httpStatusCode; if (errName === "NoSuchBucket" || httpStatus === 404) { - return { label, status: "missing" }; + return { label, status: "missing", hint }; } return { label, status: "unknown" }; } finally {