Skip to content

fix(security): skip symlinks in GitExtractor - #14427

Merged
erichare merged 2 commits into
release-1.11.3from
fix/le-2138-gitextractor-symlinks
Aug 7, 2026
Merged

fix(security): skip symlinks in GitExtractor#14427
erichare merged 2 commits into
release-1.11.3from
fix/le-2138-gitextractor-symlinks

Conversation

@erichare

@erichare erichare commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

  • skip checked-out symlinks before every GitExtractor file stat or read
  • keep file statistics and returned content limited to regular repository files
  • add safe out-of-tree canary regressions for all three affected outputs

Validation

  • 9 passedsrc/backend/tests/unit/components/git/test_gitextractor_ssrf.py
  • Ruff format and lint passed for changed files

Internal tracking: LE-2138 / PVR0836215

Summary by CodeRabbit

  • Bug Fixes
    • Improved repository scanning to safely skip symbolic links during statistics, file-content extraction, and text-content extraction.
    • Prevented files linked from outside the repository from appearing in extracted content or repository metrics.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 422b09bd-6b27-4508-a81e-cf8b1512b8e4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

GitExtractor now skips symbolic links during statistics, full content extraction, and text extraction. Tests create a repository with an external symlink and verify that its path and target content are excluded.

Changes

Git symlink handling

Layer / File(s) Summary
Exclude symlinks from extraction paths
src/bundles/lfx-bundles/src/lfx_bundles/git/gitextractor.py
Statistics and content extraction skip symbolic links.
Validate external symlink handling
src/backend/tests/unit/components/git/test_gitextractor_ssrf.py
Tests use an external symlink fixture and verify that listings, text extraction, and statistics exclude it.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 9
✅ Passed checks (9 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the security fix to skip symlinks in GitExtractor.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Test Coverage For New Implementations ✅ Passed The PR updates the backend-named test_gitextractor_ssrf.py with three concrete symlink regressions covering statistics, full file content, and text content.
Test Quality And Coverage ✅ Passed Tests use pytest async patterns and exercise all three changed methods with an out-of-tree symlink, asserting file exclusion, canary non-disclosure, and corrected statistics.
Test File Naming And Structure ✅ Passed The backend test file uses pytest fixtures and async tests with descriptive names, covers all three symlink outputs, and checks retained regular-file data plus excluded symlink data.
Excessive Mock Usage Warning ✅ Passed Mocks patch external SSRF and clone dependencies; symlink tests use a real temporary filesystem. The changed tests do not excessively mock GitExtractor core behavior.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/le-2138-gitextractor-symlinks

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.

@github-actions github-actions Bot added the bug Something isn't working label Aug 5, 2026
@erichare
erichare marked this pull request as ready for review August 5, 2026 20:32
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

✅ Test Coverage Advisor

No source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉

Advisory check only — never blocks merge.

@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Aug 5, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@src/backend/tests/unit/components/git/test_gitextractor_ssrf.py`:
- Around line 80-99: Update the GitExtractor tests around
test_gitextractor_files_content_skips_symlinks,
test_gitextractor_text_content_skips_symlinks, and
test_gitextractor_statistics_skips_symlinks to use
ComponentTestBaseWithoutClient and the required component_class, default_kwargs,
and file_names_mapping fixtures. Instantiate and exercise GitExtractorComponent
through the harness while preserving the existing symlink-skipping assertions.
- Around line 95-99: Update test_gitextractor_statistics_skips_symlinks to also
assert that result[0].data["total_size_bytes"] reflects only the non-symlink
file, confirming symlink targets are excluded from size statistics.

In `@src/bundles/lfx-bundles/src/lfx_bundles/git/gitextractor.py`:
- Around line 91-97: Update the directory-counting logic in the os.walk loop to
exclude symlinked entries from dirs before incrementing directories, matching
the existing file_path.is_symlink() filtering for files. Preserve os.walk’s
non-following behavior and continue counting only non-symlink directories.
🪄 Autofix

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: 39c0e8e0-7d3f-4b95-aa45-c70a83c7569d

📥 Commits

Reviewing files that changed from the base of the PR and between 73629e2 and 594d42e.

📒 Files selected for processing (2)
  • src/backend/tests/unit/components/git/test_gitextractor_ssrf.py
  • src/bundles/lfx-bundles/src/lfx_bundles/git/gitextractor.py

Comment thread src/backend/tests/unit/components/git/test_gitextractor_ssrf.py Outdated
Comment thread src/backend/tests/unit/components/git/test_gitextractor_ssrf.py Outdated
Comment thread src/bundles/lfx-bundles/src/lfx_bundles/git/gitextractor.py
@erichare
erichare requested a review from Adam-Aghili August 5, 2026 21:03
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.63%. Comparing base (73629e2) to head (180167a).

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##           release-1.11.3   #14427      +/-   ##
==================================================
+ Coverage           61.40%   61.63%   +0.22%     
==================================================
  Files                2398     2398              
  Lines              238302   238302              
  Branches            35840    35840              
==================================================
+ Hits               146336   146880     +544     
+ Misses              90158    89614     -544     
  Partials             1808     1808              
Flag Coverage Δ
backend 68.95% <ø> (+1.28%) ⬆️
lfx 60.76% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 78 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Frontend Unit Test Coverage Report

Coverage Summary

Lines Statements Branches Functions
Coverage: 47%
47.4% (67712/142846) 70.44% (9580/13599) 45.85% (1558/3398)

Unit Test Results

Tests Skipped Failures Errors Time
5422 0 💤 0 ❌ 0 🔥 18m 6s ⏱️

@Adam-Aghili Adam-Aghili left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot added the lgtm This PR has been approved by a maintainer label Aug 6, 2026
@erichare
erichare merged commit 5fd265c into release-1.11.3 Aug 7, 2026
175 checks passed
@erichare
erichare deleted the fix/le-2138-gitextractor-symlinks branch August 7, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants