Skip to content

Conversation

jackcooper20
Copy link
Collaborator

@jackcooper20 jackcooper20 commented Sep 18, 2025

PR Description:

  • Implemented AES-256-CBC encryption for sensitive credentials.
  • Migrated Twitter and Telegram credentials from .env to Redis, storing them in separate hashes (twitter-variables, telegram-variables).
  • Updated scripts to load, encrypt, and display credentials securely.
  • Removed fallback and plaintext .env usage for secrets; ENCRYPTION_KEY must now be provided as an environment variable.
  • Improved code safety and separation of credentials for better security and maintainability.

Summary by CodeRabbit

  • New Features

    • AES-256-GCM encryption for securely storing sensitive configuration.
    • Tool to migrate and encrypt environment variables into Redis, grouping service credentials.
    • CLI to list stored Twitter and Telegram credentials with optional decryption or masked display.
  • Chores

    • Standardized environment variable names for Twitter and Telegram credentials and added validation for required keys.
    • Added masking utility for safer display of secrets.

Copy link

coderabbitai bot commented Sep 18, 2025

Walkthrough

Adds AES-256-GCM encrypt/decrypt utilities, two CLI scripts to migrate and display encrypted env vars in Redis, a string-masking helper, and renames/validates several Twitter and Telegram environment variables plus small usage-tracking calls.

Changes

Cohort / File(s) Summary
Encryption utilities
src/lib/encryption.ts
New AES-256-GCM helpers. Reads ENCRYPTION_KEY (required, 64‑char hex), caches key Buffer. Exports encrypt(text: string): string producing v1:ivHex:tagHex:ciphertextHex, and decrypt(text: string): string that parses payload, verifies tag, and returns plaintext. Throws on invalid input or unsupported scheme.
Env → Redis migration script
src/utils/moveEnvToRedis.ts
New script that iterates process.env (skips ENCRYPTION_KEY), encrypts values with encrypt, groups known service keys into twitterAccount, telegramAccount, and otherVars, merges/appends into Redis JSON arrays twitter-accounts / telegram-accounts, and writes env-variables object. Uses REDIS_URL, ensures connection, logs summary, and exits.
Show Redis-stored vars CLI
src/utils/showEnvVariables.ts
New CLI that connects to Redis (REDIS_URL), reads twitter-accounts and telegram-accounts, JSON-parses values (parse-fallback), and prints numbered account entries. Supports --decrypt to call decrypt, otherwise masks values using mask. Closes Redis client when done.
String masking helper
src/lib/utils/string.ts
Adds exported mask(v: string): string — returns empty for falsy, ******** if length ≤ 8, else first4 + … + last4.
Twitter env rename & usage tracking
src/twitterApi.ts
Replaced Twitter env names with TWITTER_* (TWITTER_BEARER, TWITTER_CSRF_TOKEN, TWITTER_AUTH_TOKEN). Updated headers/cookies accordingly. After successful fetches, conditionally call trackApiKeyUsage with accountId = TWITTER_ID, platform: 'twitter' and log authenticated account.
Telegram env rename / validations
src/index.ts, src/fetchTelegramMessages.ts
Switched Telegram env var names to TELEGRAM_API_ID, TELEGRAM_API_HASH, TELEGRAM_TG_CHANNEL; added validation requiring TELEGRAM_API_HASH; updated missing-channel error message to reference TELEGRAM_TG_CHANNEL.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev
  participant Move as moveEnvToRedis.ts
  participant ENV as process.env
  participant ENC as src/lib/encryption.ts
  participant Redis as Redis

  Dev->>Move: run script
  Move->>ENV: read env vars (skip ENCRYPTION_KEY)
  loop per env var to persist
    Move->>ENC: encrypt(value)
    ENC-->>Move: encryptedPayload (v1:iv:tag:cipher)
    alt twitter keys
      Move->>Redis: GET `twitter-accounts`
      Redis-->>Move: existing JSON array?
      Move->>Redis: SET updated `twitter-accounts`
    else telegram keys
      Move->>Redis: GET `telegram-accounts`
      Move->>Redis: SET updated `telegram-accounts`
    else other keys
      Move->>Redis: GET `env-variables`
      Move->>Redis: SET updated `env-variables`
    end
  end
  Move-->>Dev: log summary
  Move->>Redis: disconnect
Loading
sequenceDiagram
  autonumber
  actor Dev
  participant Show as showEnvVariables.ts
  participant Redis as Redis
  participant ENC as src/lib/encryption.ts

  Dev->>Show: run script [--decrypt?]
  Show->>Redis: connect (REDIS_URL)
  Show->>Redis: GET `twitter-accounts`
  Redis-->>Show: JSON array or null
  Show->>Redis: GET `telegram-accounts`
  Redis-->>Show: JSON array or null
  loop each account entry
    alt --decrypt flag
      Show->>ENC: decrypt(encryptedValue)
      ENC-->>Show: plaintext
    else masked
      Show-->>Show: mask(encryptedValue)
    end
    Show-->>Dev: print account fields
  end
  Show->>Redis: disconnect
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • tasin2610

Poem

I’m a rabbit with a tiny key,
I hop and wrap secrets in GCM glee.
IVs and tags snug in a hex-string nest,
Off to Redis where encrypted treasures rest.
I nibble bugs and bounce — secure, at best. 🐇🔐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely and accurately summarizes the primary change: adding secure encryption and moving Twitter and Telegram credentials into Redis with tooling for storage and retrieval, which matches the main files changed (encryption module, migration script, and show CLI).
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/add-encryption-function-for-safety

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (6)
src/lib/encryption.ts (1)

6-12: Validate and cache the key; avoid per-call Buffer.from and unclear failures.

Early, explicit validation of hex and length improves error messages and avoids runtime throws in createCipheriv/createDecipheriv. Also prefer import * as crypto unless esModuleInterop is set.

If you keep the default import, confirm tsconfig has:

  • "esModuleInterop": true
  • "allowSyntheticDefaultImports": true
src/utils/showEnvVariables.ts (1)

4-5: Add try/finally and graceful shutdown; prefer quit() and REDIS_URL.

Avoid leaking the connection on errors and make target configurable.

-async function showEnvVariables() {
-    const redisClient = createClient({ url: 'redis://localhost:6379' });
-    await redisClient.connect();
+async function showEnvVariables() {
+    const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });
+    try {
+        await redisClient.connect();
         // ... existing logic ...
-    await redisClient.disconnect();
+    } finally {
+        if (redisClient.isOpen) await redisClient.quit();
+    }
 }

Also applies to: 23-26

src/utils/moveEnvToRedis.ts (4)

6-12: Make Redis URL configurable and ensure connection lifecycle is managed once.

Hardcoding localhost limits deployments; also ensure single connect/quit.

-const redisClient = createClient({ url: 'redis://localhost:6379' });
+const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });

28-37: Batch writes to Redis to cut round‑trips.

Use a pipeline/multi for HSET operations.

-    for (const [key, value] of Object.entries(envVars)) {
-        const encrypted = encrypt(value);
-        if (twitterKeys.includes(key)) {
-            await redisClient.hSet('twitter-variables', key, encrypted);
-        } else if (telegramKeys.includes(key)) {
-            await redisClient.hSet('telegram-variables', key, encrypted);
-        } else {
-            await redisClient.hSet('env-variables', key, encrypted);
-        }
-    }
+    const pipe = redisClient.multi();
+    for (const [key, value] of Object.entries(envVars)) {
+        const encrypted = encrypt(value);
+        if (twitterKeys.includes(key)) {
+            pipe.hSet('twitter-variables', key, encrypted);
+        } else if (telegramKeys.includes(key)) {
+            pipe.hSet('telegram-variables', key, encrypted);
+        } else {
+            pipe.hSet('env-variables', key, encrypted);
+        }
+    }
+    await pipe.exec();

41-41: Avoid process.exit; close Redis cleanly and propagate errors.

process.exit(0) can mask failures and skip cleanup. Prefer try/catch/finally with quit().

-moveEnvToRedis().then(() => process.exit(0));
+(async () => {
+  try {
+    await moveEnvToRedis();
+  } catch (err) {
+    console.error('Failed to move env to Redis:', err);
+    process.exitCode = 1;
+  } finally {
+    if (redisClient.isOpen) await redisClient.quit();
+  }
+})();

24-27: Key classification: confirm coverage and future‑proof.

Hard‑coded lists can drift. Consider prefix‑based grouping (e.g., TWITTER_, TELEGRAM_) or a config map.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ff434d and 9070e63.

📒 Files selected for processing (3)
  • src/lib/encryption.ts (1 hunks)
  • src/utils/moveEnvToRedis.ts (1 hunks)
  • src/utils/showEnvVariables.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/utils/moveEnvToRedis.ts (1)
src/lib/encryption.ts (1)
  • encrypt (14-20)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (6)
src/lib/encryption.ts (3)

27-39: Harden payload parsing: validate IV/tag lengths and hex.

Catch format issues early to avoid confusing crypto errors.

Apply this diff within these lines:

 export function decrypt(text: string): string {
     const parts = text.split(':');
     if (parts.length !== 4) throw new Error('Invalid payload format');
     const [scheme, ivHex, tagHex, dataHex] = parts;
     if (scheme !== SCHEME) throw new Error(`Unsupported scheme: ${scheme}`);
+    if (ivHex.length !== IV_LENGTH * 2) throw new Error('Invalid IV length');
+    const TAG_LENGTH = 16; // bytes
+    if (tagHex.length !== TAG_LENGTH * 2) throw new Error('Invalid auth tag length');
+    if (dataHex.length % 2 !== 0) throw new Error('Ciphertext must be hex');
     const iv = Buffer.from(ivHex, 'hex');
     const tag = Buffer.from(tagHex, 'hex');
     const data = Buffer.from(dataHex, 'hex');
-    const decipher = crypto.createDecipheriv(ALGORITHM, KEY, iv);
+    const decipher = crypto.createDecipheriv(ALGORITHM, getKey(), iv);
     decipher.setAuthTag(tag);
     const plaintext = Buffer.concat([decipher.update(data), decipher.final()]);
     return plaintext.toString('utf8');
 }

18-24: Optional: support AAD/context binding.

Consider an overload that accepts AAD (e.g., record type or key name) and uses setAAD() to bind ciphertexts to context.


1-1: Nit: prefer node:crypto import path.

Modern Node style and slightly clearer intent.

Apply:

-import * as crypto from 'crypto';
+import * as crypto from 'node:crypto';
src/utils/showEnvVariables.ts (3)

12-16: Defensive decryption and stable ordering.

  • Catch per-entry decrypt errors to avoid aborting the whole listing.
  • Sort keys for deterministic output.

Apply this diff within these ranges:

-    for (const [key, value] of Object.entries(twitterVars)) {
-        const shown = decryptFlag ? decrypt(value) : mask(value);
+    for (const key of Object.keys(twitterVars).sort()) {
+        const value = (twitterVars as Record<string,string>)[key];
+        const shown = decryptFn ? safeDecrypt(decryptFn, value) : mask(value);
         console.log(`${i}. ${key} - ${shown}`);
         i++;
     }
@@
-    for (const [key, value] of Object.entries(telegramVars)) {
-        const shown = decryptFlag ? decrypt(value) : mask(value);
+    for (const key of Object.keys(telegramVars).sort()) {
+        const value = (telegramVars as Record<string,string>)[key];
+        const shown = decryptFn ? safeDecrypt(decryptFn, value) : mask(value);
         console.log(`${i}. ${key} - ${shown}`);
         i++;
     }

Add this helper outside the selected lines:

function safeDecrypt(decryptFn: (v: string) => string, v: string): string {
  try { return decryptFn(v); } catch { return '<decrypt-error>'; }
}

Also applies to: 22-26


7-7: Always quit Redis in finally.

Prevents leaked connections on error.

Apply:

-    await redisClient.connect();
+    await redisClient.connect();
+    try {
       // Show Twitter variables
       // …
-    await redisClient.quit();
+    } finally {
+        await redisClient.quit();
+    }

Also applies to: 27-27


5-5: REDIS_URL fallback or explicit validation.

Passing an undefined url is brittle across redis client versions. The local default improves DX.

Apply:

-    const redisClient = createClient({ url: process.env.REDIS_URL});
+    const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9070e63 and e089521.

📒 Files selected for processing (3)
  • src/lib/encryption.ts (1 hunks)
  • src/utils/moveEnvToRedis.ts (1 hunks)
  • src/utils/showEnvVariables.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/utils/moveEnvToRedis.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/utils/showEnvVariables.ts (1)
src/lib/encryption.ts (1)
  • decrypt (27-39)
🔇 Additional comments (2)
src/lib/encryption.ts (1)

3-5: Good move to AEAD (GCM) with a 96‑bit IV.

This addresses malleability and adds integrity. Nice.

src/utils/showEnvVariables.ts (1)

32-35: Masking behavior LGTM.

Clear defaults; short values fully masked.

…nd Telegram accounts, encrypt values, and update display logic
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
src/utils/showEnvVariables.ts (2)

2-2: Gate decrypt import behind the --decrypt flag to avoid requiring ENCRYPTION_KEY on masked runs.

Static import triggers key validation at load time and breaks masked-only usage. Dynamically import only when --decrypt is present.

-import { decrypt } from '../lib/encryption';
+// Decrypt is imported dynamically only when --decrypt is used.

And inside the function:

 async function showEnvVariables() {
-    const redisClient = createClient({ url: process.env.REDIS_URL });
-    const decryptFlag = process.argv.includes('--decrypt');
+    const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });
+    const decryptFlag = process.argv.includes('--decrypt');
+    let decryptFn: ((v: string) => string) | undefined;
+    if (decryptFlag) {
+        const mod = await import('../lib/encryption');
+        decryptFn = mod.decrypt;
+    }

And where used:

-                const shown = decryptFlag ? decrypt(v as string) : mask(v as string);
+                const shown = decryptFlag && decryptFn ? safeDecrypt(decryptFn, v as string) : mask(String(v));

Repeat the same change for the Telegram section.


5-7: Provide sane REDIS_URL fallback or fail fast with a clear message.

Either default to localhost for dev, or throw if missing. Below adds a dev-safe default.

-    const redisClient = createClient({ url: process.env.REDIS_URL });
+    const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });
🧹 Nitpick comments (8)
src/utils/showEnvVariables.ts (4)

11-24: Don’t crash on decrypting non-encrypted values; add per-value safe decrypt.

JSON-parse error placeholders (e.g., { error: 'Failed to parse' }) will throw when --decrypt is used. Guard with try/catch per value.

-            Object.entries(acc).forEach(([k, v]) => {
-                const shown = decryptFlag ? decrypt(v as string) : mask(v as string);
-                console.log(`  ${k}: ${shown}`);
-            });
+            Object.entries(acc).forEach(([k, v]) => {
+                const shown = decryptFlag && decryptFn ? safeDecrypt(decryptFn, String(v)) : mask(String(v));
+                console.log(`  ${k}: ${shown}`);
+            });

Add helper near mask():

+function safeDecrypt(decryptFn: (v: string) => string, v: string): string {
+    try { return decryptFn(v); } catch { return '<not-encrypted>'; }
+}

9-27: Schema compatibility: also support legacy Redis hashes (twitter-variables/telegram-variables).

PR description mentions “hashes: twitter-variables and telegram-variables,” but this code reads string JSON under *-accounts. Add a fallback to read hashes to avoid breakage.

-    const twitterRaw = await redisClient.get('twitter-accounts');
+    const twitterRaw = await redisClient.get('twitter-accounts');
     console.log('Twitter Accounts:');
-    if (twitterRaw) {
+    if (twitterRaw) {
       ...
-    } else {
-        console.log('  (none)');
-    }
+    } else {
+        // Fallback: legacy hash
+        const twitterHash = await redisClient.hGetAll('twitter-variables');
+        if (twitterHash && Object.keys(twitterHash).length) {
+            console.log('Account 1:');
+            for (const [k, v] of Object.entries(twitterHash)) {
+                const shown = decryptFlag && decryptFn ? safeDecrypt(decryptFn, String(v)) : mask(String(v));
+                console.log(`  ${k}: ${shown}`);
+            }
+        } else {
+            console.log('  (none)');
+        }
+    }
@@
-    const telegramRaw = await redisClient.get('telegram-accounts');
+    const telegramRaw = await redisClient.get('telegram-accounts');
     console.log('\nTelegram Accounts:');
-    if (telegramRaw) {
+    if (telegramRaw) {
       ...
-    } else {
-        console.log('  (none)');
-    }
+    } else {
+        // Fallback: legacy hash
+        const telegramHash = await redisClient.hGetAll('telegram-variables');
+        if (telegramHash && Object.keys(telegramHash).length) {
+            console.log('Account 1:');
+            for (const [k, v] of Object.entries(telegramHash)) {
+                const shown = decryptFlag && decryptFn ? safeDecrypt(decryptFn, String(v)) : mask(String(v));
+                console.log(`  ${k}: ${shown}`);
+            }
+        } else {
+            console.log('  (none)');
+        }
+    }

Also applies to: 30-48


54-57: Mask plaintext more conservatively to reduce leakage.

Masking first/last 4 chars of ciphertext/plaintext leaks structure. Consider returning a fixed placeholder.

-function mask(v: string): string {
-    if (!v) return '';
-    return v.length <= 8 ? '********' : `${v.slice(0, 4)}…${v.slice(-4)}`;
-}
+function mask(_v: string): string {
+    return '<masked>';
+}

52-52: Optional safety: require an explicit confirmation when decrypting.

To avoid accidental secret leaks to terminals/log collectors, require --decrypt --yes or CONFIRM_DECRYPT=1.

src/utils/moveEnvToRedis.ts (4)

6-12: Graceful shutdown and error handling; avoid process.exit(0) and always quit() Redis.

Unconditional exit may skip socket cleanup. Wrap in try/catch/finally and quit the client.

-const redisClient = createClient({ url: process.env.REDIS_URL });
+const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });
@@
-moveEnvToRedis().then(() => process.exit(0));
+(async () => {
+  try {
+    await moveEnvToRedis();
+  } catch (err) {
+    console.error(err);
+    process.exitCode = 1;
+  } finally {
+    if (redisClient.isOpen) await redisClient.quit();
+  }
+})();

Also applies to: 78-79


15-29: .env parsing: handle quoted values and whitespace.

Values like FOO="bar baz" will retain quotes. Strip surrounding quotes for common .env patterns.

-        const value = line.slice(idx + 1);
+        let value = line.slice(idx + 1).trim();
+        if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
+            value = value.slice(1, -1);
+        }

50-71: Appending accounts risks duplicates; consider idempotent upsert.

Running the migrator multiple times will grow arrays. Prefer merging or replacing by a stable key (e.g., account name) or clear-then-set.


72-75: Parity gap with viewer: env-variables aren’t displayed by showEnvVariables.ts.

Either add a section there to show non-service envs or document that they’re intentionally hidden.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e089521 and 746e489.

📒 Files selected for processing (2)
  • src/utils/moveEnvToRedis.ts (1 hunks)
  • src/utils/showEnvVariables.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/utils/showEnvVariables.ts (1)
src/lib/encryption.ts (1)
  • decrypt (27-39)
src/utils/moveEnvToRedis.ts (1)
src/lib/encryption.ts (1)
  • encrypt (18-24)
🔇 Additional comments (4)
src/utils/showEnvVariables.ts (1)

1-3: Mode mismatch: PR text says AES‑256‑CBC; codebase uses AES‑256‑GCM.

Ensure docs/PR title match actual mode to avoid operational confusion and wrong expectations for payload format (scheme:iv:tag:data implies GCM).

src/utils/moveEnvToRedis.ts (3)

31-34: Verify key taxonomy for services.

Confirm these keys align with your real envs; Twitter commonly uses consumer/access tokens; if you rely on cookies (AUTH_TOKEN/CSRF_TOKEN), document that explicitly.


50-60: Storage schema naming diverges from PR description (hash vs JSON string keys).

PR text says hashes twitter-variables/telegram-variables, but this writes string JSON under *-accounts. Align naming or write both for compatibility.

Suggested minimal compatibility:

-        await redisClient.set('twitter-accounts', JSON.stringify(twitterArr));
+        await redisClient.set('twitter-accounts', JSON.stringify(twitterArr));
+        // Optional: also expose as a hash for legacy readers (flattened)
+        // await redisClient.hSet('twitter-variables', twitterAccount);

Repeat similarly for Telegram.


1-4: Mode mismatch note (AES‑256‑GCM vs AES‑256‑CBC).

Your encryption lib uses GCM (auth tag present). Update PR description and any runbooks to GCM for clarity.

… and dynamically import decrypt function for environment variable display
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/utils/showEnvVariables.ts (1)

5-5: Add default REDIS_URL fallback to avoid passing undefined to createClient.

Regression from earlier suggestion; passing an explicit undefined url can break node-redis defaults.

Apply this diff:

-    const redisClient = createClient({ url: process.env.REDIS_URL});
+    const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });
🧹 Nitpick comments (5)
src/utils/showEnvVariables.ts (5)

18-22: Don’t flow parse failures into the render/decrypt path; log and skip.

Placing a sentinel object gets decrypted/logged later and can crash or mislead.

Apply this diff:

-        } catch (e) {
-            twitterAccounts = [{ error: 'Failed to parse' }];
-        }
+        } catch (e) {
+            console.error('Failed to parse twitter-accounts JSON:', e);
+            twitterAccounts = [];
+        }
@@
-        } catch (e) {
-            telegramAccounts = [{ error: 'Failed to parse' }];
-        }
+        } catch (e) {
+            console.error('Failed to parse telegram-accounts JSON:', e);
+            telegramAccounts = [];
+        }

Also applies to: 39-43


12-12: Ensure Redis connection is always closed with try/finally.

If decryption or logging throws, the client may remain open.

Apply this diff:

-    await redisClient.connect();
+    await redisClient.connect();
+    try {
@@
-    await redisClient.quit();
+    } finally {
+        await redisClient.quit();
+    }

Also applies to: 54-55


13-33: Reduce duplication by extracting a small “print section” helper.

Two nearly identical blocks for Twitter/Telegram increase drift risk.

I can provide a quick refactor if you want to proceed.

Also applies to: 34-53


57-57: Gate CLI execution to avoid running on import.

Keeps the function importable/testable.

Apply this diff (CommonJS-safe):

-showEnvVariables();
+if (typeof require !== 'undefined' && typeof module !== 'undefined' && require.main === module) {
+    void showEnvVariables();
+}

59-62: Mask helper is fine; consider minimal hardening.

If you later pass non-strings here, coerce first.

Apply this diff:

-function mask(v: string): string {
-    if (!v) return '';
-    return v.length <= 8 ? '********' : `${v.slice(0, 4)}…${v.slice(-4)}`;
-}
+function mask(v: unknown): string {
+    const s = typeof v === 'string' ? v : String(v ?? '');
+    if (!s) return '';
+    return s.length <= 8 ? '********' : `${s.slice(0, 4)}…${s.slice(-4)}`;
+}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 746e489 and 4adaa3e.

📒 Files selected for processing (2)
  • src/lib/encryption.ts (1 hunks)
  • src/utils/showEnvVariables.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/encryption.ts
🔇 Additional comments (2)
src/utils/showEnvVariables.ts (2)

8-11: Good: decryption is dynamically imported only when requested.

Prevents key validation on masked runs and avoids hard dependency on ENCRYPTION_KEY.


14-15: Incorrect — repository uses JSON-string keys 'twitter-accounts' / 'telegram-accounts', not Redis hashes.

src/utils/moveEnvToRedis.ts writes 'twitter-accounts' and 'telegram-accounts' with redisClient.set(JSON.stringify(...)); src/utils/showEnvVariables.ts reads them with redisClient.get; src/utils/redisUtils.ts uses hGetAll for other keys but not these. No 'twitter-variables' or hash-based usage found.

Likely an incorrect or invalid review comment.

…sistency and add string masking utility for secure display
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/twitterApi.ts (1)

156-161: Inconsistent env name: TWITTER_ID vs TWITTER_ACCOUNT_ID

trackApiKeyUsage uses TWITTER_ID, but main() only falls back to TWITTER_ACCOUNT_ID and then throws with a message that ignores TWITTER_ID. This will fail when only TWITTER_ID is set.

-    const accountId = viewer?.userId ?? process.env.TWITTER_ACCOUNT_ID;
-    if (!accountId) throw new Error('Missing TWITTER_ACCOUNT_ID and Viewer lookup failed.');
+    const accountId = viewer?.userId ?? process.env.TWITTER_ID ?? process.env.TWITTER_ACCOUNT_ID;
+    if (!accountId) throw new Error('Missing TWITTER_ID (or TWITTER_ACCOUNT_ID) and Viewer lookup failed.');

Optionally standardize everywhere on TWITTER_ID and deprecate TWITTER_ACCOUNT_ID.

Also applies to: 173-175

♻️ Duplicate comments (1)
src/utils/showEnvVariables.ts (1)

34-35: Handle non-strings and decrypt errors to avoid CLI crashes.

Current code will throw on malformed ciphertext or non-string values.

-                        const shown = decryptFlag && decryptFn ? decryptFn(v as string) : mask(v as string);
+                        const shown =
+                            typeof v === 'string'
+                                ? (decryptFlag && decryptFn ? safeDecrypt(decryptFn, v) : mask(v))
+                                : JSON.stringify(v);

Add helper (module-scope function):

+function safeDecrypt(decrypt: (v: string) => string, v: string): string {
+    try { return decrypt(v); } catch { return '<decrypt-error>'; }
+}
🧹 Nitpick comments (13)
src/fetchTelegramMessages.ts (3)

22-28: Align error messages to TELEGRAM_TG_CHANNEL.

Keep env-var names consistent to avoid confusion.

-      throw new Error(`TG_CHANNEL "${channel}" is a private/forbidden channel; cannot fetch history.`);
+      throw new Error(`TELEGRAM_TG_CHANNEL "${channel}" is a private/forbidden channel; cannot fetch history.`);
...
-      throw new Error(`TG_CHANNEL "${channel}" is not a channel-type peer.`);
+      throw new Error(`TELEGRAM_TG_CHANNEL "${channel}" is not a channel-type peer.`);
...
-    throw new Error(`Failed to resolve TG_CHANNEL "${channel}": ${e instanceof Error ? e.message : e}`);
+    throw new Error(`Failed to resolve TELEGRAM_TG_CHANNEL "${channel}": ${e instanceof Error ? e.message : e}`);

13-13: Don’t gate usage tracking on TELEGRAM_API_ID presence.

Tracking depends on accountId; the apiId check suppresses valid telemetry.

-  const apiId = process.env.TELEGRAM_API_ID;
+  const apiId = process.env.TELEGRAM_API_ID; // not required for usage tracking

...
-  if (apiId) {
-    let accountId: string;
+  {
+    let accountId: string;
     try {
       const me = await client.getMe();
       if (me && me.id) {
         accountId = String(me.id);
       } else {
         throw new Error('Unable to determine Telegram account ID');
       }
     } catch (e) {
       throw new Error('Unable to determine Telegram account ID');
     }
     await trackApiKeyUsage({ accountId, platform: 'telegram' });
-  }
+  }

Also applies to: 56-69


49-49: Avoid logging full message content by default.

Guard under a debug flag to reduce noisy/sensitive logs.

-      console.log(formatted);
+      if (process.env.DEBUG?.includes('telegram')) console.log(formatted);
src/index.ts (3)

12-17: Fix env-var names in errors (TELEGRAM_*).

Current messages reference legacy names.

-if (!Number.isFinite(apiId)) {
-  throw new Error("API_ID environment variable is missing or not a valid number.");
-}
+if (!Number.isFinite(apiId)) {
+  throw new Error("TELEGRAM_API_ID environment variable is missing or not a valid number.");
+}
...
-if (!apiHash) {
-  throw new Error("API_HASH environment variable is not set.");
-}
+if (!apiHash) {
+  throw new Error("TELEGRAM_API_HASH environment variable is not set.");
+}

42-46: Remove non-null assertions; validate TELEGRAM_TG_CHANNEL once and reuse.

Prevents runtime undefined from slipping through and improves clarity.

-  // Run once at startup
-  try {
-    await fetchTelegramMessages(client, process.env.TELEGRAM_TG_CHANNEL!);
+  // Resolve channel once
+  const channel = process.env.TELEGRAM_TG_CHANNEL;
+  if (!channel) {
+    throw new Error('TELEGRAM_TG_CHANNEL environment variable is not set.');
+  }
+
+  // Run once at startup
+  try {
+    await fetchTelegramMessages(client, channel);
...
-      await fetchTelegramMessages(client, process.env.TELEGRAM_TG_CHANNEL!);
+      await fetchTelegramMessages(client, channel);

Also applies to: 62-63


58-67: Add a simple no-overlap guard to the cron task.

Avoid concurrent runs if a fetch exceeds 5 minutes.

-  // Schedule to run every 5 minutes (no overlap guard)
-  cron.schedule('*/5 * * * *', async () => {
+  // Schedule to run every 5 minutes (with no-overlap guard)
+  let isFetching = false;
+  cron.schedule('*/5 * * * *', async () => {
+    if (isFetching) {
+      console.log('Previous Telegram fetch still running; skipping this tick.');
+      return;
+    }
+    isFetching = true;
     console.log('Refetching Telegram messages...');
     try {
-      await fetchTelegramMessages(client, process.env.TELEGRAM_TG_CHANNEL!);
+      await fetchTelegramMessages(client, channel);
       // No duplicate print of Telegram API usage
     } catch (err) {
       console.error('Scheduled Telegram fetch failed:', err);
+    } finally {
+      isFetching = false;
     }
   });
src/utils/showEnvVariables.ts (1)

6-6: Provide a sane default for REDIS_URL.

Improves local/dev ergonomics.

-    const redisClient = createClient({ url: process.env.REDIS_URL });
+    const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });
src/utils/moveEnvToRedis.ts (3)

3-5: Remove unused imports.

-import fs from 'fs';
-import path from 'path';

6-6: Provide a sane default for REDIS_URL.

-const redisClient = createClient({ url: process.env.REDIS_URL });
+const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });

69-69: Graceful shutdown; avoid process.exit(0).

Ensure Redis connection is closed even on errors.

-moveEnvToRedis().then(() => process.exit(0));
+(async () => {
+  try {
+    await moveEnvToRedis();
+  } finally {
+    try { await redisClient.quit(); } catch {}
+  }
+})().catch((err) => {
+  console.error(err);
+  process.exit(1);
+});
src/twitterApi.ts (3)

1-6: Load dotenv before env access and drop unused constant

You read an env var before calling dotenv and keep an unused constant. Either remove dotenv entirely (preferred given the Redis migration) or at least move config up and delete the dead variable.

-import dotenv from 'dotenv';
-import cron from 'node-cron';
-import { getApiKeyUsage } from './utils/redisUtils';
-import { trackApiKeyUsage } from './utils/redisUtils';
-const AUTH_TOKEN = process.env.TWITTER_AUTH_TOKEN;
-dotenv.config();
+import dotenv from 'dotenv';
+dotenv.config(); // ensure .env is loaded before accessing process.env
+import cron from 'node-cron';
+import { getApiKeyUsage } from './utils/redisUtils';
+import { trackApiKeyUsage } from './utils/redisUtils';

If .env is no longer used anywhere, remove both the import and the config call instead.


12-15: Guard fetchViewerAccount with a token presence check

Without tokens this call will 401; returning early is cleaner than making a doomed request.

 async function fetchViewerAccount(): Promise<{ screenName: string; userId: string } | null> {
   const url = "https://x.com/i/api/graphql/jMaTSZ5dqXctUg5f97R6xw/Viewer";
 
+  const required = ['TWITTER_BEARER', 'TWITTER_CSRF_TOKEN', 'TWITTER_AUTH_TOKEN'] as const;
+  const missing = required.filter(k => !process.env[k]);
+  if (missing.length) {
+    console.warn(`fetchViewerAccount: Missing required tokens: ${missing.join(', ')}`);
+    return null;
+  }
+
   const headers = {
     "authorization": `Bearer ${process.env.TWITTER_BEARER}`,
     "x-csrf-token": process.env.TWITTER_CSRF_TOKEN as string,
     "cookie": `auth_token=${process.env.TWITTER_AUTH_TOKEN}; ct0=${process.env.TWITTER_CSRF_TOKEN}`,
   };

191-199: Prevent overlapping cron runs

A slow fetch can overlap with the next tick. Add a simple in-flight guard.

+let isFetching = false;
 // Schedule to run every 5 minutes
 cron.schedule('*/5 * * * *', async () => {
-  console.log('Refetching Twitter timeline...');
-  try {
-    const timeline = await fetchHomeTimeline();
-    console.log('Fetched timeline:', timeline);
-  } catch (err) {
-    console.error('Scheduled Twitter timeline fetch failed:', err);
-  }
+  if (isFetching) {
+    console.log('Previous fetch still running, skipping this tick.');
+    return;
+  }
+  isFetching = true;
+  console.log('Refetching Twitter timeline...');
+  try {
+    const timeline = await fetchHomeTimeline();
+    console.log('Fetched timeline:', timeline);
+  } catch (err) {
+    console.error('Scheduled Twitter timeline fetch failed:', err);
+  } finally {
+    isFetching = false;
+  }
 });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4adaa3e and 0d37d8c.

📒 Files selected for processing (6)
  • src/fetchTelegramMessages.ts (1 hunks)
  • src/index.ts (3 hunks)
  • src/lib/utils/string.ts (1 hunks)
  • src/twitterApi.ts (3 hunks)
  • src/utils/moveEnvToRedis.ts (1 hunks)
  • src/utils/showEnvVariables.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/utils/showEnvVariables.ts (1)
src/lib/utils/string.ts (1)
  • mask (7-10)
src/index.ts (2)
src/fetchTelegramMessages.ts (1)
  • fetchTelegramMessages (6-72)
src/utils/redisUtils.ts (1)
  • getApiKeyUsage (60-88)
src/twitterApi.ts (1)
src/utils/redisUtils.ts (1)
  • getApiKeyUsage (60-88)
src/utils/moveEnvToRedis.ts (1)
src/lib/encryption.ts (1)
  • encrypt (20-26)
🔇 Additional comments (4)
src/lib/utils/string.ts (1)

7-10: LGTM — simple, deterministic masking.

src/utils/showEnvVariables.ts (1)

18-20: Key names vs. PR description: accounts arrays vs. hashes.

Code reads keys "twitter-accounts"/"telegram-accounts" (JSON arrays). PR text says "twitter-variables"/"telegram-variables" (Redis hashes). Please confirm intended storage format and align both scripts accordingly (or support both for backward compatibility).

src/utils/moveEnvToRedis.ts (2)

41-65: Align storage format with PR goal (hashes), keep arrays for compatibility.

Write to Redis hashes "twitter-variables"/"telegram-variables" in addition to the existing JSON arrays.

     if (Object.keys(twitterAccount).length) {
         let twitterArr: any[] = [];
         const existing = await redisClient.get('twitter-accounts');
         if (existing) {
             try {
                 twitterArr = JSON.parse(existing);
             } catch { }
         }
         twitterArr.push(twitterAccount);
         await redisClient.set('twitter-accounts', JSON.stringify(twitterArr));
+        // Also store as a hash for direct access
+        await redisClient.hSet('twitter-variables', twitterAccount);
     }
     if (Object.keys(telegramAccount).length) {
         let telegramArr: any[] = [];
         const existing = await redisClient.get('telegram-accounts');
         if (existing) {
             try {
                 telegramArr = JSON.parse(existing);
             } catch { }
         }
         telegramArr.push(telegramAccount);
         await redisClient.set('telegram-accounts', JSON.stringify(telegramArr));
+        // Also store as a hash for direct access
+        await redisClient.hSet('telegram-variables', telegramAccount);
     }
     if (Object.keys(otherVars).length) {
-        await redisClient.set('env-variables', JSON.stringify(otherVars));
+        // Prefer a hash for misc variables too
+        await redisClient.hSet('env-variables', otherVars);
     }

1-1: AES mode mismatch with PR text (CBC vs GCM).

PR description mentions AES-256-CBC; encryption.ts (used here) implements AES-256-GCM. Confirm intended mode and update docs/code accordingly.

… with direct import for improved clarity and performance
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/utils/showEnvVariables.ts (1)

3-3: Load decrypt dynamically to avoid requiring ENCRYPTION_KEY when masking only.

Static import forces key validation even without --decrypt. Use dynamic import inside the flag path.

-import { decrypt } from '../lib/encryption';
+// decrypt is loaded dynamically only when --decrypt is used.
-    let decryptFn: ((v: string) => string) | null = null;
-    if (decryptFlag) {
-        decryptFn = decrypt;
-    }
+    let decryptFn: ((v: string) => string) | null = null;
+    if (decryptFlag) {
+        const mod = await import('../lib/encryption');
+        decryptFn = mod.decrypt;
+    }

Also applies to: 9-12

🧹 Nitpick comments (5)
src/utils/showEnvVariables.ts (5)

7-7: Provide a sane default for REDIS_URL.

Prevent connection errors when env isn’t set.

-    const redisClient = createClient({ url: process.env.REDIS_URL });
+    const redisClient = createClient({ url: process.env.REDIS_URL || 'redis://localhost:6379' });

13-16: Always quit Redis via finally.

Ensure the client closes on errors.

-    await redisClient.connect();
-    await showAccounts(redisClient, decryptFlag, decryptFn);
-    await redisClient.quit();
+    await redisClient.connect();
+    try {
+        await showAccounts(redisClient, decryptFlag, decryptFn);
+    } finally {
+        await redisClient.quit();
+    }

53-56: Handle top-level errors for a clean CLI exit.

Surface failures and exit non‑zero.

-showEnvVariables();
+showEnvVariables().catch((err) => {
+    console.error('[showEnvVariables] Error:', err?.message || err);
+    process.exit(1);
+});

26-29: Optional: make Redis keys configurable for env/tenant flexibility.

Allow overrides via env (e.g., TWITTER_REDIS_KEY/TWITTER_REDIS_HASH) to avoid hardcoding.


18-20: Type nit: narrow AccountRecord or disambiguate on error key.

A discriminated union (e.g., { kind: 'error'; message: string }) avoids accidental decryption attempts on error payloads.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d37d8c and 6de3c32.

📒 Files selected for processing (1)
  • src/utils/showEnvVariables.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/utils/showEnvVariables.ts (2)
src/lib/encryption.ts (1)
  • decrypt (29-41)
src/lib/utils/string.ts (1)
  • mask (7-10)
🔇 Additional comments (2)
src/utils/showEnvVariables.ts (2)

8-9: Good: explicit --decrypt flag with masking by default.

This aligns with least-privilege logging.


1-56: Verify repository-wide Redis key names and command usage.

  • Automated search returned no hits; manually confirm producers/consumers use the same keys for Twitter/Telegram (e.g., "twitter-accounts" vs "twitter-variables", "telegram-accounts" vs "telegram-variables").
  • Confirm whether values are stored as strings (get/set) or hashes (hGetAll/hSet) and align this file with the canonical usage.

@tasin2610 tasin2610 self-requested a review September 18, 2025 14:22
@tasin2610 tasin2610 merged commit d30bae7 into main Sep 18, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants