chore(security): ignore key material so an identity file cannot be committed - #2171
Conversation
…mmitted data/hub/identity.json reached PR #2043 with private keys in plaintext. It never landed on dev, but the data/ ignore rules are a per-file allowlist so nothing would have stopped a repeat. Adds pattern-based ignores for key material. Verified no tracked file becomes ignored.
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesCredential protection
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
PR Summary by QodoHarden .gitignore to prevent committing identity/private key material
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitignore:
- Line 156: Correct the date in the key material and secrets comment to July 27,
2026, keeping the rest of the comment unchanged.
- Line 168: Update the credential ignore rules in .gitignore to cover temporary
credential filenames, including data/mesh_credentials.json.tmp, by adding a
data-scoped suffix-aware pattern such as data/**/*credentials.json*. Ensure the
rule protects both final and temporary files produced by the mesh_credentials.py
atomic writer.
- Around line 163-168: Scope the credential-related ignore patterns in the
.gitignore block to the data/ directory, including *.key, *_private.*,
*_private_key*, identity.json, *.p8, and *credentials.json. Preserve their
existing matching behavior within data/ while preventing unrelated source files
or test fixtures elsewhere from being ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ef22d8fe-3785-40ff-9947-5935b7186449
📒 Files selected for processing (1)
.gitignore
| *_private_key* | ||
| identity.json | ||
| *.p8 | ||
| *credentials.json |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Ignore credential temp files as well.
The atomic writer creates data/mesh_credentials.json.tmp; *credentials.json does not match that filename. If the process stops before os.replace, the temporary file can retain plaintext tokens and remain committable. Add a suffix-aware, data-scoped rule such as data/**/*credentials.json*.
The same temporary-file behavior is defined in tinyagentos/taosnet/mesh_credentials.py.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitignore at line 168, Update the credential ignore rules in .gitignore to
cover temporary credential filenames, including data/mesh_credentials.json.tmp,
by adding a data-scoped suffix-aware pattern such as data/**/*credentials.json*.
Ensure the rule protects both final and temporary files produced by the
mesh_credentials.py atomic writer.
| # future one cannot. Verified against every tracked file: none become ignored. | ||
| data/hub/ | ||
| *.key | ||
| *_private.* |
There was a problem hiding this comment.
WARNING: *_private.* is overly broad — it ignores any file matching *_private.*, which includes legitimate files like test_private.py (test modules for a private module) or foo_private_data.json. This could cause legitimate files to be silently ignored, leading to data loss-by-invisibility.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| *.key | ||
| *_private.* | ||
| *_private_key* | ||
| identity.json |
There was a problem hiding this comment.
WARNING: identity.json is redundant with the existing data/hub/ rule (line 96) and overly broad — it ignores identity.json repo-wide. If a legitimate identity.json is needed outside data/hub/, it will be silently ignored.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| *_private_key* | ||
| identity.json | ||
| *.p8 | ||
| *credentials.json |
There was a problem hiding this comment.
SUGGESTION: *credentials.json is very broad and would ignore any file ending in credentials.json, including legitimate test fixtures or sample configurations like test_credentials.json. Consider narrowing to *_credentials.json if the intent is only to catch files with the _credentials naming convention.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous Review Summaries (3 snapshots, latest commit 97bd98e)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 97bd98e)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit 4a0dc78)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit ae0c6a4)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (1 file)
Reviewed by step-3.7-flash · Input: 82.9K · Output: 14.9K · Cached: 515.8K |
Code Review by Qodo
1.
|
CodeRabbit was right: the comment said 2026-07-28 but the change is dated 2026-07-27. The date came from a UTC-rolled timestamp rather than the actual date. A comment that exists to preserve WHY a rule was added should not carry a wrong WHEN.
qodo was right. dev already ignores data/hub/ (line 96, added in #2042); my second copy was redundant and the two rationale comments would drift apart. The premise of my original comment was also wrong: I ran git check-ignore against a stale local branch rather than origin/dev, so I reported a gap dev had already closed. The other six patterns ARE genuinely new, and that is what this PR is now scoped to.
kilo was right that *_private.* is overly broad: it matches api_private.py, utils_private.ts and similar plausible source names, which would be silently untracked. Narrowed to the key-material extensions (.pem/.key/.json). Verified again against every tracked file, and that a _private.py is no longer caught. A pattern that silently ignores working source is worse than the hole it closes.
|
Verdict on all four review findings, verified rather than assumed. Two were right and I acted on both, one is wrong and I proved it, one is deliberate. qodo, duplicate The reason I added it is worth recording, because it is the more useful lesson: I ran kilo, Narrowed to kilo,
kilo, |
#2173) PR #2171 closed the identity/key-material shapes but a plain creds.json still slipped through: *credentials.json does not match creds.json. @taOS-website-dev found this running the same audit on its own repo, which is the argument for having every repo run the check rather than trusting one pass. Verified on origin/dev before committing: creds.json, my_creds.json and data/creds.json were all NOT ignored, and 0 of 3100 tracked files become ignored by the new pattern.
Follows the leak found during the #2043 deep review:
data/hub/identity.jsonwas committed withsigning_privateandencryption_privatein plaintext. It was removed before merge and the repo squash-merges, so it never reached dev, but nothing stopped it going in.The existing
data/rules are a per-file allowlist (data/config.yaml,data/agents.json, and so on), so any NEW data file slips straight through. That is what happened. These replacements are pattern-based, so a future identity file cannot.Verified before committing that no currently tracked file becomes ignored by the new patterns (checked all of
git ls-files, zero hits).tinyagentos/taosnet/mesh_credentials.pyand its test are source files and stay tracked.taOS-website-dev found and closed the same gap in its repo (PR #121). This is the core half.
Summary by CodeRabbit