feat: support permissions in DriveInstance create/update (ENG-2761) by devin-ai-integration[bot] · Pull Request #177 · blaxel-ai/sdk-python

devin-ai-integration · 2026-06-24T20:45:47Z

Summary

Extends the DriveInstance / SyncDriveInstance wrapper to support per-drive ACL permissions, building on the auto-generated DrivePermission types from PR #176.

Changes:

DriveCreateConfiguration.__init__ gains permissions: list | None = None; from_dict extracts it.
DriveInstance.create / SyncDriveInstance.create pass permissions into DriveSpec for both DriveCreateConfiguration and dict inputs, using is not None check (not or UNSET) to preserve explicit permissions=[].

_update_drive_by_name / _update_drive_by_name_sync include permissions in both the new_spec construction (from config/dict) and the merge logic:

permissions=new_spec.permissions
    if new_spec and new_spec.permissions and not isinstance(new_spec.permissions, Unset)
    else (current_drive.spec.permissions if current_drive.spec else UNSET),

Both instance classes expose a permissions property (drive.spec.permissions).
16 unit tests cover create (dict + config + absent + empty-list), update (config + dict + preserve existing), and property access.

Link to Devin session: https://app.devin.ai/sessions/730361d1c61c4cd884e912562ebefdfe
Requested by: @drappier-charles

…2761) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Replace manual test with proper pytest integration test in tests/integration/core/sandbox/test_drive_acl.py following the existing TestDriveOperations pattern. Covers all 9 ACL scenarios: open-access, label-match, label-mismatch, read-only, OR logic, AND logic, path scoping, update permissions, and wildcard permission. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

- Add permissions field to DriveCreateConfiguration - Pass permissions through create() for both async and sync variants - Include permissions in update merge logic for _update_drive_by_name and _update_drive_by_name_sync (config, dict, and Drive inputs) - Add permissions property to DriveInstance and SyncDriveInstance - Import DrivePermission and Unset in drive.py - Add 15 unit tests covering create, update, and property access Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

devin-ai-integration · 2026-06-24T20:45:51Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR that start with 'DevinAI' or '@devin'.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment, CI, and merge conflict monitoring

mendral-app · 2026-06-24T20:46:42Z

✅ Linked to Linear issue ENG-2761 — status already In Progress

Assignee: Charles Drappier
PR linked: ✅ Issue will auto-close when this PR merges

Note

Posted by Linear Issue Enforcer · Tag @mendral-app with feedback.

mendral-app · 2026-06-24T20:47:48Z

🔀 Interaction Flow: Drive Permissions Support

sequenceDiagram
    participant User as User Code
    participant Config as DriveCreateConfiguration
    participant Instance as DriveInstance / SyncDriveInstance
    participant Spec as DriveSpec
    participant Client as API Client (Generated)
    participant API as Backend API

    %% Create Flow
    Note over User,API: Create Drive with Permissions
    User->>Config: new(name, permissions=[DrivePermission(...)])
    Config->>Config: from_dict() extracts permissions
    User->>Instance: create(config)
    Instance->>Spec: DriveSpec(permissions=config.permissions or UNSET)
    Instance->>Client: create_drive(body=Drive(spec=Spec))
    Client->>API: POST /drives {spec: {permissions: [...]}}
    API-->>Client: Drive response
    Client-->>Instance: Drive model
    Instance-->>User: DriveInstance (with .permissions property)

    %% Update Flow
    Note over User,API: Update Drive Permissions
    User->>Instance: update(drive_name, new_config)
    Instance->>Client: get_drive(drive_name)
    Client-->>Instance: current Drive state
    Instance->>Instance: Merge: new permissions if set,<br/>else preserve existing
    Instance->>Spec: DriveSpec(permissions=merged)
    Instance->>Client: update_drive(body=Drive(spec=Spec))
    Client->>API: PUT /drives/{name} {spec: {permissions: [...]}}
    API-->>Client: Updated Drive
    Client-->>Instance: Drive model
    Instance-->>User: DriveInstance

Summary

This PR adds per-drive ACL permission support through the SDK wrapper layer:

Layer	Role
DriveCreateConfiguration	Accepts `permissions: list[DrivePermission]` from user
DriveInstance / SyncDriveInstance	Passes permissions into `DriveSpec`, exposes `.permissions` property, handles merge logic on update
DriveSpec (generated)	New `permissions` field with `Unset`-aware serialization
DrivePermission (generated)	Model with `labels`, `mode`, and `path` fields
API Client	Serializes/deserializes permissions to/from HTTP JSON

Key design decisions:

permissions=None → UNSET (omitted from request)
permissions=[] → serialized as empty list (clears permissions)
On update: preserves existing permissions unless explicitly overridden
Permissions use OR logic (first match wins); labels within a permission use AND logic (all must match)

Note

Posted by PR Sequence Diagram · Tag @mendral-app with feedback.

…rve empty list An explicit permissions=[] should be sent as an empty list (clear permissions), not converted to UNSET (omit field). Fixes all 4 occurrences in create/update paths for both async and sync variants. Adds test for empty-list case. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

mendral-app

Needs attention — 3 issues across 2 files

The or UNSET bug in drive.py create paths is fixed (commit 60a138c), but the same empty-list falsy bug remains in two other places: the update merge conditions at lines 607 and 685 use new_spec.permissions as a truthiness check, which treats [] the same as UNSET. The drive_spec.py from_dict issue from the previous review also remains unfixed.

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.

<assessment>
The `or UNSET` bug in `drive.py` create paths is fixed (commit 60a138c), but the same empty-list falsy bug remains in two other places: the update merge conditions at lines 607 and 685 use `new_spec.permissions` as a truthiness check, which treats `[]` the same as UNSET. The `drive_spec.py` `from_dict` issue from the previous review also remains unfixed.
</assessment>

<file name="src/blaxel/core/drive/drive.py">
<issue location="src/blaxel/core/drive/drive.py:606">
`new_spec.permissions` is falsy when `[]`, so explicitly passing `permissions=[]` to clear permissions falls through to the else branch, preserving old permissions instead. Same fix pattern as commit 60a138c should apply here.
</issue>
<issue location="src/blaxel/core/drive/drive.py:684">
Same empty-list falsy bug in the sync variant of update merge logic.
</issue>
</file>

<file name="src/blaxel/core/client/models/drive_spec.py">
<issue location="src/blaxel/core/client/models/drive_spec.py:77">
(Unaddressed from previous review) `permissions` is initialized to `[]` unconditionally; when `_permissions` is UNSET (key absent from dict), `from_dict` produces `permissions=[]` instead of preserving UNSET, changing serialization on round-trips. This is auto-generated code so it may need to be fixed upstream in the generator.
</issue>
</file>

_{Tag @mendral-app with feedback or questions. View session}

mendral-app · 2026-06-24T20:50:36Z

+        permissions=new_spec.permissions
+        if new_spec and new_spec.permissions and not isinstance(new_spec.permissions, Unset)
+        else (current_drive.spec.permissions if current_drive.spec else UNSET),


bug (P2): new_spec.permissions is falsy when [], so explicitly passing permissions=[] to clear permissions falls through to the else branch, preserving old permissions instead. Same fix pattern as commit 60a138c should apply here.

Suggested change

Suggested change

permissions=new_spec.permissions

if new_spec and new_spec.permissions and not isinstance(new_spec.permissions, Unset)

else (current_drive.spec.permissions if current_drive.spec else UNSET),

permissions=new_spec.permissions

if new_spec and not isinstance(new_spec.permissions, Unset)

else (current_drive.spec.permissions if current_drive.spec else UNSET),

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/blaxel/core/drive/drive.py, line 606: <issue> `new_spec.permissions` is falsy when `[]`, so explicitly passing `permissions=[]` to clear permissions falls through to the else branch, preserving old permissions instead. Same fix pattern as commit 60a138c should apply here. </issue>

Valid — the and new_spec.permissions truthiness check here also treats [] as falsy. Will fix by removing that condition and keeping only not isinstance(new_spec.permissions, Unset).

mendral-app · 2026-06-24T20:50:36Z

+        permissions=new_spec.permissions
+        if new_spec and new_spec.permissions and not isinstance(new_spec.permissions, Unset)
+        else (current_drive.spec.permissions if current_drive.spec else UNSET),


bug (P2): Same empty-list falsy bug in the sync variant of update merge logic.

Suggested change

Suggested change

permissions=new_spec.permissions

if new_spec and new_spec.permissions and not isinstance(new_spec.permissions, Unset)

else (current_drive.spec.permissions if current_drive.spec else UNSET),

permissions=new_spec.permissions

if new_spec and not isinstance(new_spec.permissions, Unset)

else (current_drive.spec.permissions if current_drive.spec else UNSET),

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/blaxel/core/drive/drive.py, line 684: <issue> Same empty-list falsy bug in the sync variant of update merge logic. </issue>

Same fix needed here as the async variant above.

mendral-app · 2026-06-24T20:50:36Z

+        permissions = []
+        _permissions = d.pop("permissions", UNSET)
+        for permissions_item_data in _permissions or []:
+            permissions_item = DrivePermission.from_dict(permissions_item_data)
+
+            permissions.append(permissions_item)


bug (P3), medium confidence: (Unaddressed from previous review) permissions is initialized to [] unconditionally; when _permissions is UNSET (key absent from dict), from_dict produces permissions=[] instead of preserving UNSET, changing serialization on round-trips. This is auto-generated code so it may need to be fixed upstream in the generator.

Suggested change

Suggested change

permissions = []

_permissions = d.pop("permissions", UNSET)

for permissions_item_data in _permissions or []:

permissions_item = DrivePermission.from_dict(permissions_item_data)

permissions.append(permissions_item)

_permissions = d.pop("permissions", UNSET)

permissions: Union[Unset, list["DrivePermission"]] = UNSET

if not isinstance(_permissions, Unset):

for permissions_item_data in _permissions or []:

if isinstance(permissions, Unset):

permissions = []

permissions_item = DrivePermission.from_dict(permissions_item_data)

permissions.append(permissions_item)

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/blaxel/core/client/models/drive_spec.py, line 77: <issue> (Unaddressed from previous review) `permissions` is initialized to `[]` unconditionally; when `_permissions` is UNSET (key absent from dict), `from_dict` produces `permissions=[]` instead of preserving UNSET, changing serialization on round-trips. This is auto-generated code so it may need to be fixed upstream in the generator. </issue>

Acknowledged — this is auto-generated code (client/models/) which per repo convention must not be edited manually. Should be addressed upstream in the code generator.

mendral-app · 2026-06-24T20:51:13Z

🧪 Testing Guide

What this PR addresses

Extends the DriveInstance / SyncDriveInstance wrapper classes to support per-drive ACL (Access Control List) permissions. This builds on auto-generated DrivePermission types from PR #176 and allows users to specify who can access a drive (and with what mode) when creating or updating drives through the SDK.

Key additions:

DriveCreateConfiguration gains a permissions parameter
create() and update methods pass permissions through to DriveSpec
Updates preserve existing permissions if none are specified in the new config
New permissions property on drive instances for read access

Steps to exercise the new behavior

Unit tests — run the new test suite that covers the wrapper logic:
```
pytest tests/core/test_drive.py -v
```
This validates:
- DriveCreateConfiguration with/without permissions
- from_dict() deserialization of permissions
- Create with dict config + permissions
- Create with DriveCreateConfiguration object + permissions
- Create without permissions (sends UNSET, not None)
- Create with empty permissions list [] (sends empty list, distinct from UNSET)
- .permissions property accessor on both async and sync instances
Integration tests (requires sandbox environment):
```
pytest tests/integration/core/sandbox/test_drive_acl.py -v
```
This validates ACL enforcement end-to-end:
- Open access (no permissions → all sandboxes allowed)
- Label matching (AND within a permission, OR across permissions)
- Read-only mode enforcement
- Path-scoped permissions
- Updating permissions on an existing drive
- Wildcard (empty labels match all workloads)
Existing test suite — ensure no regressions in drive functionality:
```
pytest tests/ -v --ignore=tests/integration
```

What to verify (expected behavior)

✅ Creating a drive without permissions behaves exactly as before (backward compatible — UNSET is sent, not None or [])
✅ Creating a drive with a permissions list passes those permissions into the DriveSpec correctly
✅ Creating a drive with an empty list [] sends the empty list (meaning open access), distinguishable from omitting permissions entirely
✅ Updating a drive without specifying permissions preserves the existing permissions on the drive
✅ Updating a drive with new permissions replaces existing permissions
✅ The .permissions property on DriveInstance / SyncDriveInstance returns the current permissions from drive.spec.permissions
✅ DriveCreateConfiguration.from_dict() correctly deserializes a permissions key from dict input
✅ Auto-generated DrivePermission model serializes/deserializes correctly (mode, labels, path fields)

Note

Posted by PR Testing Guide · Tag @mendral-app with feedback.

drappier-charles and others added 5 commits June 24, 2026 20:06

feat: regenerate controlplane client with DrivePermission types (ENG-…

d6653fc

…2761) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

fix: restore with_headers merge behavior after regeneration

41f7106

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

test: add manual drive ACL test for per-drive permissions (ENG-2761)

af3182c

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

drappier-charles self-assigned this Jun 24, 2026

drappier-charles self-requested a review June 24, 2026 20:45

This comment was marked as outdated.

Sign in to view

mendral-app Bot reviewed Jun 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: support permissions in DriveInstance create/update (ENG-2761)#177

feat: support permissions in DriveInstance create/update (ENG-2761)#177
devin-ai-integration[bot] wants to merge 6 commits into
mainfrom
cdrappier/devin/drive-permissions-wrapper

devin-ai-integration Bot commented Jun 24, 2026 •

edited

Loading

Uh oh!

devin-ai-integration Bot commented Jun 24, 2026

Uh oh!

mendral-app Bot commented Jun 24, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

mendral-app Bot commented Jun 24, 2026

Uh oh!

mendral-app Bot left a comment

Uh oh!

mendral-app Bot Jun 24, 2026

Uh oh!

devin-ai-integration Bot Jun 24, 2026

Uh oh!

mendral-app Bot Jun 24, 2026

Uh oh!

devin-ai-integration Bot Jun 24, 2026

Uh oh!

mendral-app Bot Jun 24, 2026

Uh oh!

devin-ai-integration Bot Jun 24, 2026

Uh oh!

mendral-app Bot commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

devin-ai-integration Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

devin-ai-integration Bot commented Jun 24, 2026

🤖 Devin AI Engineer

Uh oh!

mendral-app Bot commented Jun 24, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

mendral-app Bot commented Jun 24, 2026

🔀 Interaction Flow: Drive Permissions Support

Summary

Uh oh!

mendral-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mendral-app Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

mendral-app Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

mendral-app Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

mendral-app Bot commented Jun 24, 2026

🧪 Testing Guide

What this PR addresses

Steps to exercise the new behavior

What to verify (expected behavior)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

devin-ai-integration Bot commented Jun 24, 2026 •

edited

Loading