Skip to content

Add early channel permission check#649

Merged
AlexVelezLl merged 1 commit intomainfrom
claude/test-fix-related-issues-01FwwYDNaQ7wwQY7MNKJsyur
Feb 3, 2026
Merged

Add early channel permission check#649
AlexVelezLl merged 1 commit intomainfrom
claude/test-fix-related-issues-01FwwYDNaQ7wwQY7MNKJsyur

Conversation

@rtibbles
Copy link
Copy Markdown
Member

@rtibbles rtibbles commented Dec 17, 2025

Summary

  • Calls add_channel() before DOWNLOAD_FILES stage to fail fast if unauthorized
  • Caches root_id and channel_id to avoid duplicate API calls

References

Fixes #95
Fixes #434

Reviewer guidance

Probably the best way to test this would be to edit the tests/test_chef_integration.py to use a static channel domain and channel source id.
Then run with one Studio user token, then run it again with another (non-admin) Studio user token.
When I did this, it gave me a properly semantic error message before downloading any files telling me I didn't have permission to edit the channel

🤖 This was created by Claude Code. @rtibbles then reviewed the generated output, and did iterative rounds of updates before making it ready for review 🤖

This commit addresses two related issues:
- Issue #95: Unhelpful error messages when user lacks channel edit permissions
- Issue #434: Files downloaded/uploaded before checking permissions

Changes:
- Added early permission check in commands.py after channel construction
- Calls add_channel() before DOWNLOAD_FILES stage to fail fast if unauthorized
- Caches root_id and channel_id to avoid duplicate API calls
- Prevents wasted bandwidth and processing time

Benefits:
- Users get immediate feedback on authorization issues
- No wasted resources downloading/uploading files when unauthorized
- No Studio API changes required - uses existing endpoints
- Backward compatible with existing chef scripts
@rtibbles rtibbles force-pushed the claude/test-fix-related-issues-01FwwYDNaQ7wwQY7MNKJsyur branch from 0bfc64a to 90b72d9 Compare December 17, 2025 01:21
Copy link
Copy Markdown
Member

@AlexVelezLl AlexVelezLl left a comment

Choose a reason for hiding this comment

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

Thanks, Richard. Code changes make sense, and I have smoke-tested this with tokens with and without permissions, and can confirm that this now fails early for non-authorized users with a readable auth error message.

Comment on lines +274 to +278
# Use cached root_id and channel_id if already set (from early permission check)
if self.root_id is not None and self.channel_id is not None:
root, channel_id = self.root_id, self.channel_id
else:
root, channel_id = self.add_channel()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just out of curiosity. Would there be any state when we reach this point, but the add_channel hasn't been called yet?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I would really hope not - but maybe if someone subclassed the tree manager somewhere and overrode the place we do the initial check?

@AlexVelezLl AlexVelezLl merged commit 72fbf04 into main Feb 3, 2026
23 checks passed
@rtibbles rtibbles deleted the claude/test-fix-related-issues-01FwwYDNaQ7wwQY7MNKJsyur branch February 3, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check access to channel before downloading and starting upload Provide more meaningful message when user is not authorized to edit channel

3 participants