feat: implement Docker permission fixer and diagnosis tool #521
Conversation
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as "Cortex CLI"
participant PM as "PermissionManager"
participant FS as "File System"
participant Compose as "docker-compose.yml"
participant Subproc as "subprocess (chown)"
User->>CLI: run `cortex docker permissions` [--yes/--execute?]
CLI->>PM: instantiate(base_path=cwd)
CLI->>PM: call check_compose_config()
PM->>Compose: read docker-compose.yml (if present)
Compose-->>PM: content (may lack user:)
PM-->>CLI: print guidance (if applicable)
CLI->>PM: call diagnose()
PM->>FS: walk & stat files (skip venv/.git/node_modules/...)
FS-->>PM: return mismatched paths list
alt no mismatches
PM-->>CLI: empty list
CLI-->>User: report no issues (exit 0)
else mismatches found
CLI->>User: prompt to apply fixes (skipped if --yes)
alt user confirms
CLI->>PM: call fix_permissions(execute flag)
PM->>Subproc: run `sudo chown UID:GID <paths>` (batched)
Subproc-->>PM: success / error
PM-->>CLI: result (true/false)
CLI-->>User: report result & exit (0/1)
else user declines
CLI-->>User: canceled (exit 2)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @cortex/cli.py:
- Around line 1882-1904: When handling args.fix_permissions, ensure the process
exits with a non-zero status on failure: call sys.exit(1) (or another non-zero
code) when manager.fix_permissions(issues) returns False; keep sys.exit(0) for
the success or when there are no issues. Locate the block using
args.fix_permissions, PermissionManager, manager.diagnose(), and
manager.fix_permissions to update the exit path so failures return non-zero to
callers.
In @cortex/permission_manager.py:
- Around line 71-72: The docstring in permission_manager.py incorrectly states
that an OSError is raised; update the docstring to reflect actual behavior:
remove the "Raises" section or replace it with a brief note that the function
catches subprocess and permission errors and returns False on failure, and keep
the return description accurate (e.g., "Returns: bool — True on success, False
on failure"). Ensure the description aligns with the implementation that handles
exceptions rather than propagating OSError.
In @tests/test_permission_manager.py:
- Around line 45-60: The test test_check_compose_config_suggests_fix calls
manager.check_compose_config() but never asserts output and wrongly relies on
capsys; instead patch the Rich console.print used inside check_compose_config
(e.g., with patch("module_under_test.console.print") or patch.object(console,
"print")) and assert it was called with the expected tip string; keep the
existing os.path.exists and builtins.open patches, call
manager.check_compose_config(), then assert the mocked
console.print.assert_called_once_with(...) (or check call args) to verify the
correct message was printed.
🧹 Nitpick comments (5)
tests/test_permission_manager.py (1)
62-79: Consider patching at the module where imports are used.The test works, but for more precise patching, target the module where the imports are used rather than the global modules. This ensures the test is resilient to import order changes.
🔎 Suggested improvement
-@patch("subprocess.run") -@patch("platform.system", return_value="Linux") +@patch("cortex.permission_manager.subprocess.run") +@patch("cortex.permission_manager.platform.system", return_value="Linux") def test_fix_permissions_executes_chown(mock_platform, mock_run, manager):cortex/cli.py (1)
1886-1886: Consider adding a status parameter for consistent output styling.For consistency with other
cx_printcalls in the codebase, consider explicitly specifying the status parameter.- cx_print("🔍 Scanning for Docker-related permission issues...") + cx_print("🔍 Scanning for Docker-related permission issues...", "info")cortex/permission_manager.py (3)
26-29: Substring matching may skip unintended directories.The check
"venv" in root or ".git" in rootuses substring matching, which could inadvertently skip directories like/path/to/myvenv_backup/or/path/to/.github/. Consider using path segment matching for more precise filtering.🔎 Proposed fix
for root, _, files in os.walk(self.base_path): # Skip virtual environment and git folders to save time - if "venv" in root or ".git" in root: + path_parts = root.split(os.sep) + if "venv" in path_parts or ".venv" in path_parts or ".git" in path_parts: continue
40-60: The substring check foruser:may produce false positives.The check
"user:" not in contentcould incorrectly determine that user mapping exists if the string appears elsewhere (e.g., in comments or keys likeauthorized_users:). For a hint feature this is acceptable, but consider using YAML parsing for accuracy if this becomes critical.
86-88: Consider adding a timeout and batching for large file lists.The
subprocess.runcall has no timeout, which could cause the CLI to hang ifsudoprompts for a password without an available TTY. Additionally, very large file lists could exceed OS argument length limits (typically ~128KB on Linux).🔎 Proposed fix with timeout
subprocess.run( - ["sudo", "chown", f"{uid}:{gid}"] + file_paths, check=True, capture_output=True + ["sudo", "chown", f"{uid}:{gid}"] + file_paths, + check=True, + capture_output=True, + timeout=60, )For handling very large file lists, consider using
xargs-style batching orsubprocess.runwithstdinpiped from a null-separated file list.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cortex/cli.pycortex/permission_manager.pytests/test_permission_manager.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/permission_manager.pytests/test_permission_manager.pycortex/cli.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_permission_manager.py
🧬 Code graph analysis (2)
tests/test_permission_manager.py (1)
cortex/permission_manager.py (3)
diagnose(19-38)check_compose_config(40-60)fix_permissions(62-91)
cortex/cli.py (2)
cortex/permission_manager.py (4)
fix_permissions(62-91)PermissionManager(8-91)check_compose_config(40-60)diagnose(19-38)cortex/branding.py (1)
cx_print(49-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Package
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
🔇 Additional comments (4)
tests/test_permission_manager.py (2)
9-13: LGTM!The fixture correctly uses
os.path.normpathfor cross-platform path handling.
16-42: LGTM!The test correctly mocks the filesystem traversal and stat calls, properly simulating root-owned vs. user-owned files with cross-platform path handling.
cortex/cli.py (1)
1627-1631: LGTM!The
--fix-permissionsflag is properly defined as a global argument with a clear help message.cortex/permission_manager.py (1)
8-17: LGTM!The class and constructor are well-documented with proper docstrings and type hints as required by the coding guidelines.
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @cortex/cli.py:
- Line 1892: Remove Rich markup tags like "[green]", "[yellow]", and "[red]"
from messages passed to cx_print and rely on cx_print's status/level argument to
apply formatting; locate calls such as cx_print("[green]✅ No root-owned files
detected in bind mounts![/green]", "success") and similar calls (e.g., the other
occurrences around the bind-mount checks) and change the message to plain text
like "✅ No root-owned files detected in bind mounts!" while keeping the same
status argument ("success", "warning", "error") so cx_print handles
coloring/formatting.
- Around line 1882-1907: Wrap the entire args.fix_permissions flow in a
try/except around the PermissionManager use: instantiate PermissionManager() and
call manager.check_compose_config(), manager.diagnose(), and
manager.fix_permissions(issues) inside a try block, and catch specific
exceptions like PermissionError, FileNotFoundError,
subprocess.CalledProcessError (and a general Exception fallback) to call
cx_print with a clear error message including the exception text and exit
non-zero; keep the existing success/warning prints and the interactive
console.input logic but ensure any raised exception is handled and reported
instead of crashing the CLI.
🧹 Nitpick comments (1)
cortex/cli.py (1)
1896-1907: Enhance user input validation for better UX.The current implementation only accepts lowercase "y". Consider accepting common affirmative responses like "yes", "Y", and handling keyboard interrupts gracefully.
🔎 Proposed enhancement
cx_print(f"⚠️ Found {len(issues)} files owned by root.", "warning") - confirm = console.input("[bold cyan]Fix these permissions now? (y/n): [/bold cyan]") - if confirm.lower() == "y": - if manager.fix_permissions(issues): - cx_print("✨ Permissions fixed successfully!", "success") - sys.exit(0) - else: - cx_print("❌ Failed to fix permissions. You may need sudo access.", "error") - sys.exit(1) - sys.exit(0) + try: + confirm = console.input("[bold cyan]Fix these permissions now? (y/n): [/bold cyan]") + if confirm.strip().lower() in ("y", "yes"): + if manager.fix_permissions(issues): + cx_print("✨ Permissions fixed successfully!", "success") sys.exit(0) + else: + cx_print("❌ Failed to fix permissions. You may need sudo access.", "error") + sys.exit(1) + else: + cx_print("Operation cancelled by user.", "info") + sys.exit(0) + except (EOFError, KeyboardInterrupt): + console.print() + cx_print("Operation cancelled by user.", "info") + sys.exit(0)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cortex/cli.pycortex/permission_manager.pytests/test_permission_manager.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_permission_manager.py
- cortex/permission_manager.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.py
🧬 Code graph analysis (1)
cortex/cli.py (2)
cortex/permission_manager.py (4)
fix_permissions(57-84)PermissionManager(8-84)check_compose_config(41-55)diagnose(19-39)cortex/branding.py (1)
cx_print(49-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Package
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
🔇 Additional comments (1)
cortex/cli.py (1)
1627-1631: LGTM: Clear flag definition.The
--fix-permissionsflag is well-defined with a descriptive help message. The implementation follows argparse conventions correctly.
|
@Anshgrover23 will you review my PR i have implemented the changes as mentioned in the Issue description and also addressed the mentioned suggestions by coderabbitai. |
Anshgrover23
changed the title
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@Kesavaraja67 Documentation is missing, and the fourth requirement from the issue is not yet implemented, generating correct docker-compose settings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cortex/cli.py (1)
1620-1638: Adddockercommand to the help table for discoverability.The new
docker permissionssubcommand is not listed inshow_rich_help(). Users runningcortexwithout arguments won't see this feature.🔎 Proposed fix
table.add_row("sandbox <cmd>", "Test packages in Docker sandbox") + table.add_row("docker permissions", "Fix Docker bind-mount permissions") table.add_row("doctor", "System health check")
🧹 Nitpick comments (1)
cortex/cli.py (1)
95-102: Subprocess exceptions are already caught inPermissionManager.fix_permissions.Looking at
permission_manager.py(lines 99-107), thefix_permissionsmethod catchessubprocess.CalledProcessError,subprocess.TimeoutExpired, andPermissionErrorinternally and returnsFalseinstead of re-raising. These exceptions will never propagate to this handler.This code is unreachable but harmless. Consider removing it for clarity, or refactor
PermissionManager.fix_permissionsto optionally re-raise if you want detailed error reporting at the CLI level.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cortex/cli.pycortex/permission_manager.pytests/test_permission_manager.py
🚧 Files skipped from review as they are similar to previous changes (2)
- cortex/permission_manager.py
- tests/test_permission_manager.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.py
🧠 Learnings (1)
📚 Learning: 2025-12-11T12:03:24.071Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T12:03:24.071Z
Learning: Applies to **/*install*.py : No silent sudo execution - require explicit user confirmation
Applied to files:
cortex/cli.py
🧬 Code graph analysis (1)
cortex/cli.py (2)
cortex/permission_manager.py (4)
PermissionManager(25-113)check_compose_config(74-89)diagnose(39-59)fix_permissions(91-113)cortex/branding.py (1)
cx_print(49-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
🔇 Additional comments (4)
cortex/cli.py (4)
4-4: LGTM!The
subprocessimport is correctly added to support catching subprocess-related exceptions in thedocker_permissionsmethod's error handling.
42-88: LGTM! Well-structured permission repair flow.The method correctly:
- Handles non-interactive mode via
--yesflag (addressing past feedback)- Gracefully handles stdin unavailability with
EOFError/KeyboardInterrupt- Uses appropriate exit codes (0 for success/cancel, 1 for failure)
- Prompts for user confirmation before running
sudo chown(per learning: "No silent sudo execution")
1702-1712: LGTM! Clean subcommand structure.The
docker permissionssubcommand with--yesflag properly addresses past reviewer feedback about using a subcommand pattern and supporting non-interactive execution.
1966-1971: LGTM!The command routing correctly handles the
docker permissionssubcommand and falls back to help for unknown docker actions.
Kesavaraja67
left a comment
Kesavaraja67
left a comment
There was a problem hiding this comment.
I have Addressed All your suggested changes.
|
@Anshgrover23 I have addressed all the changes requested by you and also i have implemented the |
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@Kesavaraja67 Docs still missing.
|
@Anshgrover23, I have added Google-style docstrings to all new methods. Are you referring to an update in the |
|
@Kesavaraja67 Yes. |
|
@Anshgrover23 I've updated all the docs (README, COMMANDS, and a new guide). |
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@Kesavaraja67 Rest looks good for now, just a nit comment added.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @cortex/permission_manager.py:
- Around line 74-89: The current check in check_compose_config uses a naive
string search for "user:" which can be fooled by comments or other occurrences;
change it to parse docker-compose.yml with a YAML parser (import yaml and use
yaml.safe_load) and then inspect the parsed mapping for a top-level "services"
dict and whether any service dict contains the "user" key (e.g.,
has_user_mapping = any("user" in svc for svc in services.values())); if no
service has a user entry, keep the console.print guidance; wrap parsing in the
existing try/except to silently fail on unreadable or invalid YAML, and if you
cannot add PyYAML as a dependency, document this limitation and leave the
existing behavior.
- Around line 91-139: The fix_permissions method currently passes the entire
mismatches list to a single subprocess.run chown call, risking "Argument list
too long" failures for large projects; modify fix_permissions to batch the chown
operations (or use an xargs/tempfile approach) so it never exceeds ARG_MAX:
after computing mismatches via diagnose(), break mismatches into sized chunks
(or write them to a temp file and call xargs -a / xargs -0) and run
subprocess.run(["sudo","chown", f"{self.host_uid}:{self.host_gid}"] + chunk,
...) for each chunk (or run xargs with the temp file), ensure proper cleanup of
any temp file and preserve existing error handling around
subprocess.CalledProcessError, subprocess.TimeoutExpired, and PermissionError so
the method returns False on failure and True on success.
- Around line 36-37: The class currently assigns host_uid/host_gid to 1000 on
Windows which is misleading; update __init__ to avoid hardcoded fallbacks by
either (A) detecting WSL and using real UIDs (e.g., check env WSL_DISTRO_NAME or
inspect /proc/version for "Microsoft" and then call os.getuid()/os.getgid() to
set host_uid/host_gid) or (B) explicitly block native Windows by raising a
RuntimeError in __init__ when platform.system() == "Windows" and document the
limitation in the class docstring; remove the hardcoded 1000 values and ensure
fix_permissions still handles the platform check consistently (refer to
host_uid, host_gid, __init__, and fix_permissions).
🧹 Nitpick comments (2)
cortex/permission_manager.py (2)
14-22: LGTM! Consider extending with additional common build directories.The EXCLUDED_DIRS constant is well-chosen and addresses the past review feedback. The use of a set ensures efficient lookups during traversal.
Optional: Additional directories to consider
If your projects commonly use these patterns, consider extending:
EXCLUDED_DIRS = { "venv", ".venv", ".git", "__pycache__", "node_modules", ".pytest_cache", + "dist", + "build", + ".tox", + ".mypy_cache", + "*.egg-info", }
61-72: Consider documenting indentation assumptions.The generated YAML snippet assumes 4-space indentation, which may not match all docker-compose.yml files. While this is reasonable for a suggestion/template, consider noting this assumption in the docstring or making it configurable.
Optional: Document or parameterize indentation
def generate_compose_settings(self) -> str: - """Generates the recommended user mapping for docker-compose.yml. + """Generates the recommended user mapping for docker-compose.yml. + + Note: The snippet uses 4-space indentation. Adjust as needed for your file. Returns: str: A formatted YAML snippet for the user directive. """
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cortex/cli.pycortex/permission_manager.pytests/test_permission_manager.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_permission_manager.py
- cortex/cli.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/permission_manager.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Package
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
🔇 Additional comments (1)
cortex/permission_manager.py (1)
39-59: LGTM! UID comparison now correctly checks against host user.The implementation correctly addresses the previous feedback by comparing against
self.host_uidrather than just checking for root-owned files. This will detect mismatches from any container-created UID, not just root.The in-place modification of
dirs[:]efficiently prunes excluded directories, and exception handling gracefully skips inaccessible paths.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cortex/cli.py (1)
4-4: Unused import: subprocess is not directly used in cli.py.The
subprocessmodule is imported but never called directly in this file. The actual subprocess usage is encapsulated withinPermissionManager. Consider removing this import unless there's a future use case.🔎 Proposed fix
import argparse import logging import os -import subprocess import sys import time
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cortex/cli.pycortex/permission_manager.pytests/test_permission_manager.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.pycortex/permission_manager.pytests/test_permission_manager.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_permission_manager.py
🧠 Learnings (1)
📚 Learning: 2025-12-11T12:03:24.071Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T12:03:24.071Z
Learning: Applies to **/*install*.py : No silent sudo execution - require explicit user confirmation
Applied to files:
cortex/cli.py
🧬 Code graph analysis (2)
cortex/cli.py (1)
cortex/permission_manager.py (3)
PermissionManager(30-173)check_compose_config(99-126)fix_permissions(128-173)
tests/test_permission_manager.py (2)
cortex/permission_manager.py (4)
PermissionManager(30-173)diagnose(65-85)check_compose_config(99-126)fix_permissions(128-173)cortex/sandbox/sandbox_executor.py (1)
success(74-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Package
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
🔇 Additional comments (14)
cortex/permission_manager.py (5)
14-27: Well-organized exclusion constant.The EXCLUDED_DIRS constant effectively groups common directories to skip, making it easy to extend without modifying the scan logic. This addresses the past review feedback.
39-63: Excellent Windows/WSL detection logic.The implementation correctly blocks native Windows while allowing WSL execution by checking for "microsoft" in the platform release string. This is a standard and reliable WSL detection pattern that addresses the past review concern about misleading Windows support.
65-85: Correct UID mismatch detection.The diagnose() method now compares against
self.host_uidrather than only checking for root ownership (UID 0), which properly catches files created by any mismatched container UID. This addresses the past review feedback about non-root container UIDs.
99-126: Proper YAML parsing eliminates false positives.Using
yaml.safe_load()and structural analysis of the services dict correctly identifies missing user mappings without being fooled by comments or string occurrences. The graceful fallback for missing PyYAML is appropriate.
128-173: Batching strategy prevents ARG_MAX failures.Processing files in chunks of 100 effectively prevents "Argument list too long" errors on large projects. The batch size is conservative and the error handling is comprehensive.
cortex/cli.py (4)
1586-1591: Improved documentation for help display function.The enhanced docstring clearly describes the function's purpose and its role in the CLI help system.
1619-1619: Help table correctly documents new docker permissions command.The new entry is appropriately placed and describes the feature succinctly.
1686-1700: Well-structured argparse configuration for Docker utilities.The docker command with its permissions subcommand follows argparse best practices. Both
--execute/-eand--yes/-yflags are properly defined, though the--yesflag needs to be integrated into the actual logic (see separate comment on lines 43-84).
1955-1959: Command routing correctly dispatches docker permissions requests.The routing logic properly checks for the docker command and its permissions action before invoking the handler.
tests/test_permission_manager.py (5)
10-21: Well-designed fixture with proper environment mocking.The fixture correctly patches OS-level functions to simulate a Linux environment, preventing NotImplementedError during test setup. The use of
os.path.normpathensures cross-platform compatibility.
24-52: Comprehensive Windows and WSL detection tests.Both
test_init_native_windows_raises_errorandtest_init_wsl_detectionthoroughly validate platform-specific behavior:
- Native Windows correctly raises NotImplementedError with an informative message
- WSL on Windows is properly detected via the "microsoft" release string and allows instantiation
These tests ensure the implementation handles different Windows environments correctly.
55-77: Accurate test of UID mismatch detection logic.The test properly validates that
diagnose()identifies files with mismatched UIDs (root-owned file flagged) while ignoring files owned by the host user. The mocking strategy cleanly simulates the filesystem state.
80-112: Thorough validation of docker-compose config analysis.Both test cases correctly verify the YAML parsing logic:
test_check_compose_config_with_valid_yaml: confirms no recommendation when user mapping existstest_check_compose_config_recommends_settings: verifies recommendation output when mapping is missingThe tests properly mock
console.printto validate output behavior without relying oncapsys, which wouldn't capture Rich console output.
115-133: Excellent verification of batching behavior.The test confirms that
fix_permissionscorrectly batches 250 files into 3 subprocess calls (100, 100, 50) and validates the structure of the chown command. This ensures ARG_MAX limits won't be exceeded on large projects.
…g for docker permissions
|
|
@Anshgrover23 i have added the |
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@Kesavaraja67 Nice Work, LGTM!
|
@Anshgrover23 Thanks!!! |
…ux#521) * feat: implement Docker permission fixer and diagnosis tool (Pain Point cortexlinux#17) * feat: implement permission fixer and sync with latest upstream * refactor: address CodeRabbit feedback on security, testing, and exit codes * refactor: address review feedback on safety logic, test patching, and documentation * feat: addressed maintainer feedback and fixed linting * trigger rerun * chore: add docker to help menu and remove redundant exception handling * docs: finalized all documentation for docker permission fixer * feat: implement real --execute flag and fix duplicate exceptions * refactor: implement batching, safe YAML parsing, and WSL detection per review * feat: implement interactive confirmation and production-grade batching for docker permissions
3 participants
Summary
Implements a PermissionManager to diagnose and fix host-user lockouts caused by Docker root-owned files. Added the --fix-permissions flag for automated recovery and a docker-compose.yml configuration tip.
Related Issue
Closes #449
Type of Change
AI Disclosure
Testing
NotImplementedErroron native Windows.docker-compose.ymlto prevent false positives.ruff check --fixandblackformatting; resolvedUP035native list type hint deprecations.--executeflag with interactive confirmation.Documentation
docs/DOCKER_PERMISSIONS.mdcovering technical logic and troubleshooting.docs/COMMANDS.mdwith usage examples and flags.README.mdto reflect the feature's completed status.Checklist
Demo Evidence
kesava@DESKTOP-N0H7M36_._mnt_f_Open-Source-bounties_cortex.2026-01-06.01-55-38.mp4
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.