Skip to content

fix(iscsi): block device allowlist confinement#432

Open
ambient-code[bot] wants to merge 3 commits intomainfrom
fix/352-iscsi-block-device-confinement
Open

fix(iscsi): block device allowlist confinement#432
ambient-code[bot] wants to merge 3 commits intomainfrom
fix/352-iscsi-block-device-confinement

Conversation

@ambient-code
Copy link
Copy Markdown
Contributor

@ambient-code ambient-code bot commented Apr 8, 2026

Summary

  • Add block_device_allowlist configuration parameter to ISCSI driver that must be explicitly populated to allow block device exposure
  • Validate block device paths against the allowlist using os.path.realpath() to resolve symlinks before checking
  • Reject all block device requests when the allowlist is empty (secure by default)
  • Add @validate_call decorators to all 10 @export methods for pydantic input type validation
  • Add unit tests covering allowlist enforcement, symlink resolution, and path confinement

Closes #352

Test plan

  • 9 unit tests pass covering block device allowlist (empty, not-in-list, accepted, relative path, symlink resolution, symlink-not-in-allowlist) and file path confinement (relative, absolute, traversal)
  • Lint passes (make lint-fix)
  • CI checks pass

🤖 Generated with Claude Code

@claude
fix(iscsi): add block device allowlist and validate_call to prevent a…
7f6a64b 
…rbitrary device exposure

Block device paths passed via add_lun(is_block=True) were accepted without
confinement. Add a block_device_allowlist config param that must be explicitly
populated; resolved paths are checked against it. Also add @validate_call to
all @export methods for input type validation.

Closes #352

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ambient-code ambient-code bot mentioned this pull request Apr 8, 2026
Open
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 8, 2026 •
edited
Loading

✅ Deploy Preview for jumpstarter-docs ready!

Name Link
🔨 Latest commit 9232932
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/69d6b441a35a720008b898fc
😎 Deploy Preview https://deploy-preview-432--jumpstarter-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@claude
fix(iscsi): skip tests on non-Linux platforms (macOS CI fix)
cd19c84 
The iSCSI driver depends on rtslib_fb which requires libudev, a
Linux-only library. Add a module-level pytest.skip to prevent import
errors on macOS CI runners.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ambient-code
Copy link
Copy Markdown
Contributor Author

ambient-code bot commented Apr 8, 2026

CI Fix: macOS test failures

Root cause: The new driver_test.py imports jumpstarter_driver_iscsi.driver at module level, which transitively imports rtslib_fb -> pyudev -> libudev. Since libudev is a Linux-only library, this causes an ImportError on macOS CI runners during test collection.

Fix applied (commit cd19c840): Added a module-level pytest.skip() guard before the driver import:

if sys.platform != "linux":
    pytest.skip("iSCSI driver requires Linux (libudev)", allow_module_level=True)

This ensures the test module is cleanly skipped on non-Linux platforms rather than failing with an import error.

Other CI observations:

  • pytest-matrix (ubuntu-24.04, 3.13) passed successfully
  • The pytest gate job failed only because the macOS matrix jobs failed/were cancelled
  • The uv.lock diff in the PR appears to match main now (no effective diff)
  • No unaddressed review comments on the PR

mangelajo
mangelajo reviewed Apr 8, 2026
View reviewed changes
python/packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py
host: str = field(default="")
port: int = 3260
remove_created_on_close: bool = False # Keep disk images persistent by default
block_device_allowlist: List[str] = field(default_factory=list)
Copy link
Copy Markdown
Member

@mangelajo mangelajo Apr 8, 2026

Choose a reason for hiding this comment

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

I guess this parameter would need to be documented in the README.md of the driver.

Copy link
Copy Markdown
Contributor Author

@ambient-code ambient-code bot Apr 8, 2026

Choose a reason for hiding this comment

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

Good catch! I've added block_device_allowlist to both the config parameters table and the YAML example in the driver README (commit 9232932).

@claude
docs(iscsi): document block_device_allowlist parameter in README
9232932 
Add the new block_device_allowlist config parameter to the README
config table and YAML example, as requested in PR review.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mangelajo mangelajo mangelajo left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

iSCSI block device exposure - no confinement when is_block=True

1 participant

@mangelajo