fix(migrations): log loud warnings on down() for irreversible data migrations

docs/migrations.md

@@ -219,7 +219,7 @@ pnpm build
 
 | Practice                                       | Why                                                                                                           |
 | ---------------------------------------------- | ------------------------------------------------------------------------------------------------------------- |
-| Always implement `down()`                      | Enables safe rollback                                                                                         |
+| Always implement `down()`                      | Enables safe rollback; log loud warnings if data changes or extension dependencies are irreversible (#1207)     |
 | Never modify an applied migration              | Create a new migration instead                                                                                |
 | Test rollbacks locally                         | Run `up` → verify → `down` → verify                                                                           |
 | Use `IF EXISTS` / `IF NOT NULL`                | Makes migrations idempotent                                                                                   |

src/achievements/achievements.seed.ts

@@ -314,8 +314,8 @@ export async function seedAchievements(achievementsService: any): Promise<void>
     for (const achievementData of DEFAULT_ACHIEVEMENTS) {
       await achievementsService.createAchievement(achievementData);
     }
-    console.log(`✅ Seeded ${DEFAULT_ACHIEVEMENTS.length} achievements`);
+    Logger.log(`✅ Seeded ${DEFAULT_ACHIEVEMENTS.length} achievements`);
   } catch (error) {
-    console.error('❌ Error seeding achievements:', error);
+    Logger.error('❌ Error seeding achievements:', error);
   }
 }

src/email-marketing/automation/automation.service.ts

@@ -196,6 +196,9 @@ export class AutomationService {
     if (workflow.status === WorkflowStatus.ACTIVE) {
       throw new BusinessValidationException('Deactivate workflow before deleting');
     }
+    this.logger.warn(
+      `Removing automation workflow ${id} will permanently delete its triggers and actions. This action is irreversible.`,
+    );
     await this.workflowRepository.manager.transaction(async (manager) => {
       await manager.getRepository(AutomationTrigger).softDelete({ workflowId: id });
       await manager.getRepository(AutomationAction).softDelete({ workflowId: id });
@@ -353,6 +356,11 @@ export class AutomationService {
           tag: action.config.tag,
         });
         break;
+    }
+  }
+}g.tag,
+        });
+        break;
       case ActionType.REMOVE_TAG:
         this.eventEmitter.emit(APP_EVENTS.USER_REMOVE_TAG, {
           userId: payload.userId,

src/migrations/1599999999999-BaselineSchema.ts

@@ -2311,8 +2311,9 @@ export class BaselineSchema1599999999999 implements MigrationInterface {
   }
 
   public async down(queryRunner: QueryRunner): Promise<void> {
-    // Dropping the entire baseline schema is handled by `migration:revert`
-    // in reverse order; the tables are dropped as their corresponding
-    // migrations are reverted.
+    console.warn(
+      'WARNING: BaselineSchema1599999999999 is an irreversible migration. ' +
+        'If you need to undo it, restore from a backup or create a new migration.',
+    );
   }
 }

src/migrations/1600000000000-enable-uuid-ossp.ts

@@ -20,8 +20,12 @@ export class EnableUuidOssp1600000000000 implements MigrationInterface {
     await queryRunner.query('CREATE EXTENSION IF NOT EXISTS "uuid-ossp"');
   }
 
-  public async down(): Promise<void> {
+  public async down(_queryRunner?: QueryRunner): Promise<void> {
     // No-op: other schema objects depend on this extension, so dropping it
     // during a rollback could break the database. Leaving it in place is safe.
+    console.warn(
+      'WARNING: [EnableUuidOssp1600000000000] down() is a no-op. ' +
+        'The "uuid-ossp" extension is retained because other database objects depend on it.',
+    );
   }
 }

src/migrations/1783000000000-clear-plaintext-auth-tokens.ts

@@ -27,7 +27,11 @@ export class ClearPlaintextAuthTokens1783000000000 implements MigrationInterface
     `);
   }
 
-  public async down(): Promise<void> {
+  public async down(_queryRunner?: QueryRunner): Promise<void> {
     // No-op: the cleared plaintext tokens cannot be restored.
+    console.warn(
+      'WARNING: [ClearPlaintextAuthTokens1783000000000] down() cannot restore cleared plaintext ' +
+        'tokens (passwordResetToken/emailVerificationToken). Affected users must request new verification or reset links.',
+    );
   }
 }

src/migrations/1783000000001-reencrypt-oauth-provider-tokens.ts

@@ -67,9 +67,13 @@ export class ReencryptOAuthProviderTokens1783000000001 implements MigrationInter
     }
   }
 
-  public async down(): Promise<void> {
+  public async down(_queryRunner?: QueryRunner): Promise<void> {
     // No-op: AES-GCM ciphertext cannot be reversed without the key, and the
     // migration is not the place to log or stash the raw values.
+    console.warn(
+      'WARNING: [ReencryptOAuthProviderTokens1783000000001] down() is a no-op. ' +
+        'AES-GCM encrypted OAuth provider tokens cannot be reverted to plaintext.',
+    );
   }
 
   private maybeEncrypt(stored: string | null, key: Buffer): string | null {

src/migrations/1783000000006-clear-legacy-bcrypt-refresh-tokens.ts

@@ -37,8 +37,12 @@ export class ClearLegacyBcryptRefreshTokens1783000000006 implements MigrationInt
     `);
   }
 
-  public async down(): Promise<void> {
+  public async down(_queryRunner?: QueryRunner): Promise<void> {
     // Cannot reverse this migration - bcrypt hashes cannot be recovered from HMAC-SHA-256 hashes.
     // Affected users will need to re-login to obtain new refresh tokens.
+    console.warn(
+      'WARNING: [ClearLegacyBcryptRefreshTokens1783000000006] down() cannot restore cleared legacy ' +
+        'bcrypt refresh tokens. Affected users must re-authenticate to obtain new tokens.',
+    );
   }
 }

src/migrations/1790000000000-add-paused-subscription-status.ts

@@ -33,5 +33,9 @@ export class AddPausedSubscriptionStatus1790000000000 implements MigrationInterf
     // This is a limitation of PostgreSQL's enum type
     // For production, consider using a different approach for status management
     // such as a separate status table or string type with check constraints
+    console.warn(
+      'WARNING: [AddPausedSubscriptionStatus1790000000000] down() is a no-op. ' +
+        'PostgreSQL does not support removing values from an enum type; "paused" remains in subscriptions_status_enum.',
+    );
   }
 }

src/migrations/1791000000001-fix-forum-anonymous-author.ts

@@ -120,5 +120,9 @@ export class FixForumAnonymousAuthor1791000000001 implements MigrationInterface
     `);
     // Irreversible: purged anonymous votes and flagged->active status changes
     // on threads/comments cannot be reconstructed (see class JSDoc).
+    console.warn(
+      'WARNING: [FixForumAnonymousAuthor1791000000001] down() cannot restore purged anonymous forum votes ' +
+        'or reset flagged thread/comment statuses.',
+    );
   }
 }

src/migrations/README.md

@@ -98,13 +98,41 @@ and fails the build if found — the same footgun cannot silently come back.
    pnpm run migration:run      # re-apply to leave the DB migrated
    ```
 
+---
+
+## Non-reversible and data migrations
+
+Certain migrations perform one-way data updates, security token scrubbing/re-encryption, or enable shared database extensions that cannot (or should not) be rolled back automatically in `down()`:
+
+- **Security & Data Sanitization:** Irreversible operations such as clearing unrecoverable plaintext tokens or wiping obsolete bcrypt hashes.
+- **Extensions & Global Types:** Shared PostgreSQL extensions (e.g. `uuid-ossp`) and enum type values that cannot be safely dropped without breaking existing dependencies.
+
+### Documented non-reversible / partial-reversal migrations
+
+| Migration | Reason for No-Op / Partial Rollback in `down()` | Mitigation / Action on Rollback |
+| :--- | :--- | :--- |
+| `1600000000000-enable-uuid-ossp.ts` | The `uuid-ossp` extension is shared by multiple tables and columns; dropping it would break dependent schemas. | Extension is retained in the database; safe no-op. |
+| `1783000000000-clear-plaintext-auth-tokens.ts` | Cleared plaintext reset/verification tokens cannot be reconstructed. | Affected users must re-request verification or password reset links. |
+| `1783000000001-reencrypt-oauth-provider-tokens.ts` | Plaintext OAuth provider tokens were encrypted at rest with AES-256-GCM. Plaintext cannot be restored. | Tokens remain encrypted; safe no-op. |
+| `1783000000006-clear-legacy-bcrypt-refresh-tokens.ts` | Legacy bcrypt refresh token hashes were wiped (transition to HMAC-SHA-256). | Affected users must re-authenticate to obtain new tokens. |
+| `1790000000000-add-paused-subscription-status.ts` | PostgreSQL does not support `ALTER TYPE ... DROP VALUE` for enum types. | The `'paused'` enum value remains in the type. |
+| `1790000000001-fix-invoice-number-sequence.ts` | Reassigned duplicate invoice numbers cannot be reverted to original collision-prone timestamp+random values. | Drops sequence and unique constraint; invoice numbers retain renumbered format. Restore from backup if needed. |
+| `1791000000001-fix-forum-anonymous-author.ts` | Purged anonymous forum votes and flagged thread/comment statuses cannot be reconstructed. | FK constraint and column type are reverted; purged anonymous votes cannot be restored. |
+
+### Rule: Loud warning on no-op / irreversible `down()`
+
+When a migration cannot reverse data or schema changes, its `down()` method **must log a clear warning** (e.g. via `console.warn`) explaining what was not restored so that `migration:revert` output is honest in CI, deployments, and incident responses.
+
+---
+
 ## Rules of thumb
 
-| Rule                                                      | Why                                          |
-| --------------------------------------------------------- | -------------------------------------------- |
-| Always implement `down()`                                 | Enables safe rollback in CI and production   |
-| Never modify an applied migration                         | Create a new migration instead               |
-| Use the passed `queryRunner`, never `createQueryRunner()` | Migrations share one transaction (see above) |
-| Prefer `IF EXISTS` / `IF NOT NULL`                        | Makes migrations idempotent                  |
-| Keep migrations small and focused                         | Easier to review and roll back               |
-| Use timestamp-based naming                                | Ensures deterministic ordering               |
+| Rule                                                      | Why                                                                             |
+| --------------------------------------------------------- | ------------------------------------------------------------------------------- |
+| Always implement `down()`                                 | Enables safe rollback; loud warnings make no-op/data rollbacks explicit (#1207) |
+| Never modify an applied migration                         | Create a new migration instead                                                  |
+| Use the passed `queryRunner`, never `createQueryRunner()` | Migrations share one transaction (see above)                                    |
+| Prefer `IF EXISTS` / `IF NOT NULL`                        | Makes migrations idempotent                                                     |
+| Keep migrations small and focused                         | Easier to review and roll back                                                  |
+| Use timestamp-based naming                                | Ensures deterministic ordering                                                  |
+

src/migrations/irreversible-migrations.spec.ts

@@ -0,0 +1,89 @@
+import { QueryRunner } from 'typeorm';
+import { EnableUuidOssp1600000000000 } from './1600000000000-enable-uuid-ossp';
+import { ClearPlaintextAuthTokens1783000000000 } from './1783000000000-clear-plaintext-auth-tokens';
+import { ReencryptOAuthProviderTokens1783000000001 } from './1783000000001-reencrypt-oauth-provider-tokens';
+import { ClearLegacyBcryptRefreshTokens1783000000006 } from './1783000000006-clear-legacy-bcrypt-refresh-tokens';
+import { AddPausedSubscriptionStatus1790000000000 } from './1790000000000-add-paused-subscription-status';
+import { FixInvoiceNumberSequence1790000000001 } from './1790000000001-fix-invoice-number-sequence';
+import { FixForumAnonymousAuthor1791000000001 } from './1791000000001-fix-forum-anonymous-author';
+
+/**
+ * Issue #1207 — Irreversible data migrations and no-op rollbacks must log a loud warning
+ * on down() so migration:revert output is honest in CI, incident response, and local workflows.
+ */
+describe('Irreversible migrations down() loud warnings (Issue #1207)', () => {
+  let warnSpy: jest.SpyInstance;
+  let logSpy: jest.SpyInstance;
+  let mockQueryRunner: jest.Mocked<QueryRunner>;
+
+  beforeEach(() => {
+    warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    logSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
+    mockQueryRunner = {
+      query: jest.fn().mockResolvedValue([]),
+    } as unknown as jest.Mocked<QueryRunner>;
+  });
+
+  afterEach(() => {
+    warnSpy.mockRestore();
+    logSpy.mockRestore();
+  });
+
+  it('1600000000000-enable-uuid-ossp logs a loud warning in down()', async () => {
+    const migration = new EnableUuidOssp1600000000000();
+    await migration.down(mockQueryRunner);
+
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+    expect(warnSpy.mock.calls[0][0]).toMatch(/WARNING:.*uuid-ossp/i);
+  });
+
+  it('1783000000000-clear-plaintext-auth-tokens logs a loud warning in down()', async () => {
+    const migration = new ClearPlaintextAuthTokens1783000000000();
+    await migration.down(mockQueryRunner);
+
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+    expect(warnSpy.mock.calls[0][0]).toMatch(/WARNING:.*plaintext.*token/i);
+  });
+
+  it('1783000000001-reencrypt-oauth-provider-tokens logs a loud warning in down()', async () => {
+    const migration = new ReencryptOAuthProviderTokens1783000000001();
+    await migration.down(mockQueryRunner);
+
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+    expect(warnSpy.mock.calls[0][0]).toMatch(/WARNING:.*OAuth/i);
+  });
+
+  it('1783000000006-clear-legacy-bcrypt-refresh-tokens logs a loud warning in down()', async () => {
+    const migration = new ClearLegacyBcryptRefreshTokens1783000000006();
+    await migration.down(mockQueryRunner);
+
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+    expect(warnSpy.mock.calls[0][0]).toMatch(/WARNING:.*bcrypt/i);
+  });
+
+  it('1790000000000-add-paused-subscription-status logs a loud warning in down()', async () => {
+    const migration = new AddPausedSubscriptionStatus1790000000000();
+    await migration.down(mockQueryRunner);
+
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+    expect(warnSpy.mock.calls[0][0]).toMatch(/WARNING:.*enum/i);
+  });
+
+  it('1790000000001-fix-invoice-number-sequence logs a loud warning in down()', async () => {
+    const migration = new FixInvoiceNumberSequence1790000000001();
+    await migration.down(mockQueryRunner);
+
+    const loggedWarnings = warnSpy.mock.calls.some(([msg]) =>
+      typeof msg === 'string' && msg.includes('WARNING: Down migration cannot recover original timestamp'),
+    );
+    expect(loggedWarnings).toBe(true);
+  });
+
+  it('1791000000001-fix-forum-anonymous-author logs a loud warning in down()', async () => {
+    const migration = new FixForumAnonymousAuthor1791000000001();
+    await migration.down(mockQueryRunner);
+
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+    expect(warnSpy.mock.calls[0][0]).toMatch(/WARNING:.*forum.*vote/i);
+  });
+});

src/payments/providers/payment-provider.service.ts

@@ -18,17 +18,17 @@ export interface CreditResult {
  * PaymentProviderService
  *
  * Thin abstraction over the payment provider (Stripe / PayPal / etc.).
- * The concrete implementation should call the real provider SDK; this
+ * The concrete implementation should call the real provider SDK; this 
  * default implementation throws so that tests must supply a mock and
  * production deployments must configure a real provider.
  *
- * Issue #1007 — extracted so SubscriptionsService can inject it and
+ * Issue #1007 ℒ extracted so SubscriptionsService can inject it and
  * tests can mock it without touching the real provider.
  */
 @Injectable()
 export class PaymentProviderService {
   private readonly logger = new Logger(PaymentProviderService.name);
-
+ 
   /**
    * Charge the customer's default payment method for `amount` (in the
    * subscription's currency).  Returns the provider's charge/payment-intent
@@ -38,12 +38,12 @@ export class PaymentProviderService {
     userId: string,
     amount: number,
     currency: string,
-    metadata: Record<string, unknown> = {},
+    _metadata: Record<string, unknown> = {},
   ): Promise<ChargeResult> {
     // TODO: replace with real Stripe call:
     //   const intent = await stripe.paymentIntents.create({ amount: Math.round(amount * 100), currency, ... });
     this.logger.warn(
-      `chargeCustomer called for user ${userId} amount=${amount} — no real provider configured`,
+      `ChargeCustomer called for user ${userId} amount=${amount}  no real provider configured`,
     );
     throw new Error(
       'No payment provider configured. Inject a concrete PaymentProviderService implementation.',
@@ -58,12 +58,12 @@ export class PaymentProviderService {
     userId: string,
     amount: number,
     currency: string,
-    metadata: Record<string, unknown> = {},
+    _metadata: Record<string, unknown> = {},
   ): Promise<CreditResult> {
     // TODO: replace with real Stripe call:
     //   const credit = await stripe.customers.createBalanceTransaction(customerId, { amount: -Math.round(amount * 100), currency });
     this.logger.warn(
-      `issueCredit called for user ${userId} amount=${amount} — no real provider configured`,
+      `issueCredit called for user ${userId} amount=${amount}  no real provider configured`,
     );
     throw new Error(
       'No payment provider configured. Inject a concrete PaymentProviderService implementation.',