Review and fix open source issues - #167
Review and fix open source issues#167Richard Ogundele (richardogundele) wants to merge 3 commits into
Conversation
- Fix table of contents numbering (6 → 5) in README.md - Fix typo "Leaning" → "Learning" in README.md:73 - Remove excessive blank lines in README.md - Fix "straight forward" → "straightforward" in .docs/python_training.md - Remove duplicate nest_asyncio entry in .ci/environment.yml - Add missing Node.js/TypeScript entries to .gitignore
Manya Sharma (ManyaS-Git)
left a comment
There was a problem hiding this comment.
This PR contains the same critical security problem I flagged on #175 (and which is also present in #142) — it must not be merged in this state.
1. (CRITICAL — security) Third-party production site with live credentials committed to the repo
ai200-architectures/TrainDistributedDeepModel is a valid submodule (gitlink d037c56…) declared in .gitmodules. This PR deletes that gitlink and replaces it with a ~100 KB base64-encoded blob that decodes to a dumped HTML page of the production SaaS site alpha.duality.dev. Decoded, it contains live secrets: a GITHUB_CLIENT_SECRET/GITHUB_CLIENT_ID, NETLIFY_PUBLIC_NETLIFY_CLIENT_SECRET/_ID, a POSTHOG_KEY/POSTHOG_HOST, a SENTRY_DSN, session/access tokens, internal ENV/routing state, and a personal user record (email, invite code, plan data).
Committing this to a public repo exposes third-party production credentials. Please:
- Remove the blob and restore the
TrainDistributedDeepModelsubmodule pointer. - Treat all values in that blob as compromised — don't paste them in the discussion.
- Notify the operator of
alpha.duality.devso they can rotate every exposed credential (GitHub/Netlify OAuth secrets, PostHog/Sentry keys, session tokens). - Run secret scanning (e.g.,
gitleaks) on the branch before any further changes.
2. Duplicate of #175 and #142
This PR, #142, and #175 carry essentially the same change set (portal app, LICENSE edit, ML-For-Beginners gitlink, contribution_plan.md/contribution_tracker.md, the leaked blob, CI whitespace cleanup). Please pick one PR to develop and close the other two, to avoid the security review being applied to only one of three open merge paths.
3. Scope (50 files) and unrelated LICENSE edit
A single PR again mixes a full React/Vite app, README/.gitignore/environment.yml/CI edits, docs, images, LICENSE, submodules, and personal planning docs (contribution_plan.md references issues in other repos and doesn't belong here). The LICENSE change (de-indenting the first line) should be reverted — license files shouldn't be touched casually.
4. Submodule integrity
.gitmodulesstill declaresai200-architectures/TrainDistributedDeepModel, but the gitlink is gone.ML-For-Beginnersis added as a gitlink (f925c9a…) but is not registered in.gitmodules, sogit submodule update --initwon't include it.
5. The portal app won't build as written
src/data/resources.ts uses icon: ActivityIcon (seismic-dl entry) but ActivityIcon is not imported — a compile error under the strict-mode tsconfig this PR enables. Also icon?: any undercuts strict typing; no lockfile is committed.
6. The legit fixes in this PR are good — but they belong in a small, separate PR
The .ci/environment.yml duplicate-dependency removal, the README TOC numbering fix (6. → 5.), and the "Deep Leaning" → "Deep Learning" typo fix are correct and welcome. Split those out as their own PR and they can be merged immediately; everything else needs rework per the above.
Recommendation: do not merge. Restore the submodule and remove the leaked blob, revert LICENSE, register (or drop) ML-For-Beginners, split the app and the small fixes into separate PRs, and consolidate the three duplicate PRs. Happy to re-review a cleaned-up version.
I've successfully reviewed the open source repository and fixed 6 issues across 4 files. All changes have been committed and pushed to the branch claude/review-open-source-fixes-v5sPw.
Issues Fixed:
README.md:26 - Fixed table of contents numbering (changed item "6" to "5" for Contributing section)
README.md:73 - Fixed typo "Train Distributed Deep Leaning Model" → "Train Distributed Deep Learning Model"
README.md:11-13 - Removed excessive blank lines (reduced from 3 to 1 blank line between sections)
.docs/python_training.md:7 - Fixed "straight forward" → "straightforward"
.ci/environment.yml:24-26 - Removed duplicate nest_asyncio>=0.9.10 entry
.gitignore - Added missing Node.js/TypeScript entries:
node_modules/
npm/yarn debug logs
TypeScript build info
Vite cache and build directories
Local environment files (.env.local, .env.*.local)
OS-specific files (.DS_Store, Thumbs.db)