Skip to content

Better mount detection#289

Merged
wonderwhy-er merged 1 commit into
mainfrom
improve-mount-detection-for-docker
Dec 10, 2025
Merged

Better mount detection#289
wonderwhy-er merged 1 commit into
mainfrom
improve-mount-detection-for-docker

Conversation

@wonderwhy-er

@wonderwhy-er wonderwhy-er commented Dec 9, 2025

Copy link
Copy Markdown
Owner

CodeAnt-AI Description

Improve container host mount detection in Linux containers

What Changed

  • System mount points (/, /dev, /proc, /sys, /run, etc.) are excluded so only user-relevant mounts are considered
  • Host-mounted directories are now detected by known host filesystem types (e.g., 9p, virtiofs), by host-marked device paths, and by an expanded set of user-visible mount paths (/mnt, /workspace, /data, /home, /Users, /app, /project, /src, /code)
  • Read-only vs read-write mount status is preserved and mounts are reported with a clearer description

Impact

✅ Clearer workspace mount detection
✅ Fewer missed host-mounted directories
✅ Fewer false positives from system filesystems

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

Release Notes

  • Improvements
    • Enhanced container mount detection to better distinguish between system mounts and user-mounted directories.
    • Improved recognition of host-mounted resources within containers.
    • Expanded support for additional user mount path patterns to provide more accurate container filesystem classification.

✏️ Tip: You can customize this high-level summary in your review settings.

@codeant-ai

codeant-ai Bot commented Dec 9, 2025

Copy link
Copy Markdown
Contributor

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai

coderabbitai Bot commented Dec 9, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The change enhances filesystem mount detection in system utilities by introducing filesystem-type classification. It adds categorization for system mounts and host-mounted resources, improves skip logic for non-user directories, and refines user-mount detection to consider filesystem types, host markers, and path patterns.

Changes

Cohort / File(s) Summary
Mount detection enhancement
src/utils/system-info.ts
Introduces systemFsTypes and hostMountFsTypes concept sets for filesystem classification; tracks fsType from /proc/mounts; adds explicit skip logic for system mount points (/, /dev*, /sys*, /proc, /run, /sbin, /etc/*); reworks user-mount detection to evaluate host-mount filesystem types, /run/host_mark/ devices, and non-system filesystem types with user-like paths; expands user-like path prefixes (/Users/, /app/, /project/, /src/, /code/); applies combined condition (hostMountFs OR hostMarkDevice OR (nonSystemFs AND userLikePath)) for DockerMount classification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Multiple new filesystem type classification sets require verification of accuracy and completeness
  • Enhanced conditional logic with compound boolean expressions needs careful tracing to ensure all mount detection paths are covered correctly
  • Expanded path prefix list should be validated against intended use cases to prevent false positives/negatives

Possibly related PRs

  • Docker installation #199: Modifies the same file (src/utils/system-info.ts) with related mount discovery and Docker/host-mount detection logic, making it a direct predecessor or sibling implementation.

Poem

🐰 A rabbit hops through filesystems bright,
Classifying mounts from dawn to night,
System, host, and user paths so true,
Discovering where the docker mounts grew! 🏔️
With tmps and procs now sorted right,
The mount detection shines with light! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Better mount detection' directly relates to the main change, which introduces enhanced filesystem classification and improved logic for detecting user-mounted directories in container environments.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch improve-mount-detection-for-docker

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.

@codeant-ai codeant-ai Bot added the size:M This PR changes 30-99 lines, ignoring generated files label Dec 9, 2025
@codeant-ai

codeant-ai Bot commented Dec 9, 2025

Copy link
Copy Markdown
Contributor

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Mount parsing fragility
    The code naively splits lines from /proc/mounts with simple space splitting. /proc/mounts encodes spaces and other chars as octal escapes (e.g. '\040') and fields can be separated by multiple whitespace. This can cause incorrect device/mount/fsType/options extraction and lead to wrong mount detection.

@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: 1

🧹 Nitpick comments (1)
src/utils/system-info.ts (1)

200-209: Consider hoisting filesystem-type sets to module scope for reuse and clarity

Defining systemFsTypes and hostMountFsTypes inside the loop body means new Set instances are allocated on every discoverContainerMounts call. Functionally fine, but these are static classifications.

Consider lifting them to top-level const sets (or at least outside the /proc/mounts parsing block). That reduces allocations and centralizes the FS-type lists for future extension.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 367039a and d63fa11.

📒 Files selected for processing (1)
  • src/utils/system-info.ts (1 hunks)
🔇 Additional comments (1)
src/utils/system-info.ts (1)

218-233: Verify whether skipping all /run* and other system paths matches your intended coverage

The isSystemMountPoint filter excludes mounts under /dev*, /sys*, /proc*, /run*, /sbin, plus specific /etc/* files. This is a reasonable default for hiding low-level/system mounts, but it may also exclude any future host volumes that end up mounted directly under /run/....

If in your Docker/Podman setups host-facing mounts can ever live under /run (e.g., /run/user/... or similar helper mounts you might want to expose), you may want a more targeted rule (e.g., only skip specific well-known system subpaths) instead of all /run*.

Comment thread src/utils/system-info.ts
Comment on lines +234 to 252
// Detect user mounts by:
// 1. Known host-mount filesystem types (fakeowner, 9p, virtiofs)
// 2. Device from /run/host_mark/ (docker-mcp-gateway pattern)
// 3. Non-system filesystem type with user-like mount point
const isHostMountFs = hostMountFsTypes.has(fsType);
const isHostMarkDevice = device.startsWith('/run/host_mark/');
const isNonSystemFs = !systemFsTypes.has(fsType);
const isUserLikePath = mountPoint.startsWith('/mnt/') ||
mountPoint.startsWith('/workspace') ||
mountPoint.startsWith('/data/') ||
(mountPoint.startsWith('/home/') && !mountPoint.startsWith('/home/root'))) {

mountPoint.startsWith('/home/') ||
mountPoint.startsWith('/Users/') ||
mountPoint.startsWith('/app/') ||
mountPoint.startsWith('/project/') ||
mountPoint.startsWith('/src/') ||
mountPoint.startsWith('/code/');

if (isHostMountFs || isHostMarkDevice || (isNonSystemFs && isUserLikePath)) {
const isReadOnly = options.includes('ro');

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.

⚠️ Potential issue | 🟡 Minor

Tighten read-only detection and consider normalizing user-like path heuristics

The combined condition

if (isHostMountFs || isHostMarkDevice || (isNonSystemFs && isUserLikePath)) {

is a solid improvement and should catch many realistic host mounts.

One minor correctness issue:

const isReadOnly = options.includes('ro');

can misclassify mounts such as rw,errors=remount-ro as read-only because ro appears as a substring. It’s safer to parse mount options by token:

- const isReadOnly = options.includes('ro');
+ const mountOptions = options.split(',');
+ const isReadOnly = mountOptions.includes('ro');

This avoids false positives while preserving behavior for true ro mounts.

As an optional polish, you might also consider whether isUserLikePath should distinguish /workspace from /workspace-* (currently startsWith('/workspace') matches both) and whether any additional project-specific prefixes should be centralized/configurable.

🤖 Prompt for AI Agents
In src/utils/system-info.ts around lines 234 to 252, the read-only detection
uses options.includes('ro') which can falsely match substrings like
"errors=remount-ro"; instead split or tokenize the mount options by comma (and
trim each token) and check for an exact 'ro' token (e.g.,
options.split(',').map(t=>t.trim()).includes('ro')). Update the isReadOnly
assignment to use this tokenized check. Optionally, when updating nearby code,
consider tightening the isUserLikePath heuristics (e.g., match '/workspace'
exactly or use a configurable prefix list) but the immediate fix is to replace
the substring check with a comma-token exact-match check for 'ro'.

@codeant-ai

codeant-ai Bot commented Dec 9, 2025

Copy link
Copy Markdown
Contributor

CodeAnt AI finished reviewing your PR.

@wonderwhy-er wonderwhy-er merged commit d318d8b into main Dec 10, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant