-
Notifications
You must be signed in to change notification settings - Fork 16
Closed as not planned
Closed as not planned
Copy link
Labels
Description
Detail Bug Report
Summary
- Context: The
ReadWritePermissionstype 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 aReadWritePermissionswith bothread=falseandwrite=false, but this state is impossible to create through string parsing and violates the parsing invariant. - Actual vs. expected: The
FromStrimplementation rejects creating instances with both fields false (returnsMissingPermissionerror), but theFrom<sdk::types::ReadWritePermissions>allows it, creating an inconsistent state. - Impact: If the SDK returns a
ReadWritePermissionswith 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 }
}
}
}Reactions are currently unavailable