Migration logistic-sentry#147
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
simantak-dabhade
left a comment
There was a problem hiding this comment.
🚨 Security: same leaked key as #132 — merge blocked
Second occurrence of the same TinyFish API key in as many PRs. logistics-sentry/.env.local was committed in a95557e (2026-04-09) with the identical key string we already flagged on PR #132. Treat the key as compromised (if it wasn't already) — both fork branches expose it publicly.
This is now a pattern, not a one-off. Your local workflow is creating .env.local with the real key and letting it get staged because neither the recipe nor the repo has a .gitignore covering it. Before any further PRs, please confirm with @simantak-dabhade that the key was rotated (he has the earlier screenshot request from #132), and adjust your local flow.
Security steps before we can continue review
- Confirm rotation — verify with Simantak out-of-band that the key is dead (the one shared across #132 and #147).
- Delete
logistics-sentry/.env.localfrom this branch. - Add
logistics-sentry/.gitignore(and the repo-wide one is coming separately) covering.env,.env.local,.env.*.local,node_modules/,.next/. - Force-push to rewrite history so the key isn't in the branch log (
git reset --soft <parent>+ recommit without.env.local, thengit push --force-with-lease). - Fix your local dev flow so future PRs don't keep doing this. A one-line check before pushing:
git diff --cached --name-only | grep -E '\.env($|\.)'catches this. Or set up a personal pre-commit hook.
Also noted (not blocking — flagged for when security lifts)
Since I'm here, other things I saw while checking:
env.exampleshould be.env.example. Currently namedenv.example(no leading dot). Playbook requires the dotfile form. Rename..env.examplehas no get-your-key comment — justTINYFISH_API_KEY=your_tinyfish_api_key_here. Match the format used in #129 tutor-finder / #134 wing-command.- PR body is empty — describe what was migrated.
.jsfiles instead of.ts— unusual for the cookbook but it's how the recipe was originally; not a blocker.
Full migration review of the code deferred until the security block lifts. README exists, SDK import is in place (src/lib/logistics/agent.js), Supabase not present — code side will probably review fast once the secret is handled.
|
Status update: @simantak-dabhade has confirmed the key was rotated (same one was on #132). The exposed Still need to complete the cleanup before full review can continue:
And please adjust your local flow so this doesn't happen a third time — (Scanned the rest of your open PRs — #129, #131, #134–#140, #144 are all clean. Only #132 and this one carried the leaked key.) Once the file is gone from history and the |
Full migration review (in addition to the security block above)Now that the key is rotated, I went through the rest of the PR. Good news: the code-side migration is actually really clean. Build passes — second PR in the batch where that's true (after #136 viet-bike-scout). ✅ What's working well
🔴 Additional blockers beyond the security one
🟡 Nits (not blocking)
SummaryOrder of operations from here:
Once those land I'll run the local test pass and approve. |
No description provided.