Fix 477 fine grained pat attachments #478
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alez#477) Fine-grained personal access tokens cannot download attachments from private repositories directly due to a GitHub platform limitation. This adds a workaround for image attachments (/assets/ URLs) using GitHub's Markdown API to convert URLs to JWT-signed URLs that can be downloaded without authentication. Changes: - Add get_jwt_signed_url_via_markdown_api() function - Detect fine-grained token + private repo + /assets/ URL upfront - Use JWT workaround for those cases, mark success with jwt_workaround flag - Skip download with skipped_at when workaround fails - Add startup warning when using --attachments with fine-grained tokens - Document limitation in README (file attachments still fail) - Add 6 unit tests for JWT workaround logic
Uses the real parse_args() function to get CLI defaults, so when new arguments are added they're automatically available to all tests. Changes: - Add tests/conftest.py with create_args fixture - Update 8 test files to use shared fixture - Remove duplicate _create_mock_args methods - Remove redundant @pytest.fixture mock_args definitions This eliminates the need to update multiple test files when adding new CLI arguments.
Contributor
|
@Iamrodos Thanks for the fix. Sorry I only saw it after it was merged, GitHub didn't send me a notification for the PR. I just tested it and ran into two issues:
|
Contributor
Author
|
Well that does complicate things a bit. I like your ideas on detecting the token type, I will do some research in the codebase. Yes, if you can find more details about 404s that you are not expecting that would be great. Rather than a new issue, should #477 be reopened? I am not familiar with the convention in such circumstances. That would be my preference. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #477
First commit.
Fine-grained personal access tokens cannot download attachments from private repositories due to a GitHub limitation. Add a workaround for image attachments using GitHub's Markdown API to convert URLs to JWT-signed URLs.
Second commit.
Refactor test fixtures to use shared create_args helper (uses real parse_args() so new CLI args are automatically available to all tests). This makes the tests more robust to CLI changes. Changing the CLI should no longer require modifying existing tests.