Skip to content

[Detail Bug] ReadWritePermissions conversion from SDK allows invalid (false,false) stateΒ #123

@detail-app

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_879aaf8b-133f-4633-8219-d272912761b0

Summary

  • Context: The ReadWritePermissions type is used to represent read/write permissions for operation groups in access token scopes. It can be created from string parsing or by converting from the SDK type.
  • Bug: The From<sdk::types::ReadWritePermissions> implementation can create a ReadWritePermissions with both read=false and write=false, but this state is impossible to create through string parsing and violates the parsing invariant.
  • Actual vs. expected: The FromStr implementation rejects creating instances with both fields false (returns MissingPermission error), but the From<sdk::types::ReadWritePermissions> allows it, creating an inconsistent state.
  • Impact: If the SDK returns a ReadWritePermissions with both fields false, converting it to the CLI type creates an object that cannot be round-tripped back to a string representation, violating the expected invariant and potentially causing confusion or errors in serialization/display logic.

Code with bug

impl From<sdk::types::ReadWritePermissions> for ReadWritePermissions {
    fn from(permissions: sdk::types::ReadWritePermissions) -> Self {
        ReadWritePermissions {
            read: permissions.read,
            write: permissions.write,  // <-- BUG πŸ”΄ Can create (false, false) here
        }
    }
}

Codebase inconsistency

The parser enforces an invariant that at least one permission must be set:

impl FromStr for ReadWritePermissions {
    type Err = OpGroupsParseError;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let mut read = false;
        let mut write = false;
        for c in s.chars() {
            match c {
                'r' => read = true,
                'w' => write = true,
                _ => return Err(OpGroupsParseError::InvalidPermissionChar(c)),
            }
        }
        if !read && !write {
            return Err(OpGroupsParseError::MissingPermission);
        }
        Ok(ReadWritePermissions { read, write })
    }
}

The conversion from the SDK type does not enforce this invariant and can produce (read=false, write=false), creating a state that cannot be represented by the parser.

Recommended fix

Add validation to the SDK-to-CLI conversion to enforce the same invariant as FromStr (defaulting to a safe, representable state when both are false):

impl From<sdk::types::ReadWritePermissions> for ReadWritePermissions {
    fn from(permissions: sdk::types::ReadWritePermissions) -> Self {
        let read = permissions.read;
        let write = permissions.write;

        if !read && !write {
            ReadWritePermissions { read: true, write: false } // <-- FIX 🟒 Enforce invariant; default to read-only
        } else {
            ReadWritePermissions { read, write }
        }
    }
}

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions