Skip to content

feat(delete): add --dry-run flag and safety guardrails#442

Open
ishaanxgupta wants to merge 1 commit intourunc-dev:mainfrom
hemang1404:feat-delete-dry-run-safety-423
Open

feat(delete): add --dry-run flag and safety guardrails#442
ishaanxgupta wants to merge 1 commit intourunc-dev:mainfrom
hemang1404:feat-delete-dry-run-safety-423

Conversation

@ishaanxgupta
Copy link
Contributor

Description

This PR adds security hardening to the urunc delete command to prevent accidental deletion of host system paths and provides a dry-run mode for safe preview of operations.

1. Dry-Run Functionality

  • Added --dry-run (-n) flag to preview what would be deleted without actually deleting
  • Implemented DryRunDelete() method that shows all paths that would be removed
  • Useful for debugging and verifying delete operations

2. Path Traversal Protection

  • validateContainerID(): Prevents path traversal attacks via containerID
    • Blocks .. sequences
    • Blocks absolute paths
    • Blocks path separators
  • Example: urunc delete ../../etc is now rejected

3. Path Safety Validation

  • validateDeletionPath(): Ensures target paths stay within expected boundaries
    • Validates paths are within rootDir
    • Uses filepath.Rel() to detect escapes
  • validatePathSafety(): Additional safety layer in unikontainers package
    • Checks against critical system path denylist
    • Prevents deletion of /, /usr, /etc, /lib, etc.

Related issues

Fixes #423

How was this tested?

  • Code review for logic correctness
  • Verified no compilation errors
  • Validated against existing code patterns
  • Checked error handling paths
  • Ensured backward compatibility
  • Confirmed validation functions work correctly

LLM usage

GitHub Copilot assisted with code implementation

Checklist

  • I have read the contribution guide.
  • The linter passes locally (make lint).
  • The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
  • If LLMs were used: I have read the llm policy.

collaborated with @hemang1404

- Add --dry-run (-n) flag to preview deletions without executing
- Implement containerID validation to prevent path traversal attacks
- Add path safety validation before all delete operations
- Create validateDeletionPath() to ensure paths stay within rootDir
- Add critical system path denylist (/, /usr, /etc, etc.)
- Implement DryRunDelete() method for safe preview
- Validate bundleDir, rootfsDir, and BaseDir before deletion

Security improvements:
- Block path traversal sequences (..) in containerID
- Prevent deletion of absolute paths outside expected roots
- Refuse to delete critical system directories
- Defense-in-depth with multiple validation layers

Fixes: urunc-dev#423

Signed-off-by: hemang1404 <[email protected]>
Co-authored-by: ishaanxgupta <[email protected]>
Signed-off-by: hemang1404 <[email protected]>
@netlify
Copy link

netlify bot commented Feb 4, 2026

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit 14347ba
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6982c9c9e6a5630008cb9834

@cmainas
Copy link
Contributor

cmainas commented Feb 4, 2026

Please do not open PRs for issues that have not be identified as such yet.

@cmainas cmainas added do-not-merge invalid This doesn't seem right duplicate This issue or pull request already exists and removed do-not-merge labels Feb 4, 2026
@cmainas
Copy link
Contributor

cmainas commented Feb 4, 2026

Duplicate with #436

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(delete): add --dry-run and safety guardrails to prevent deleting host paths

3 participants