Skip to content

Handle access denied file upload error #6139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2025

Conversation

szluzero
Copy link
Contributor

WHY are these changes introduced?

Closes https://github.com/Shopify/workflows-operations/issues/3398

Adds error handling for file_uploader to handle ACCESS_DENIED errors for stagedUploadCreate. What the error looks like in prod:

accessdnied

WHAT is this pull request doing?

Instead of the above, we'll now show this:
Screenshot 2025-07-21 at 3 46 49 PM

This PR makes it so that adding bulk_data permissions from admin will allow access of stagedUploadCreate for sqlite files for staff members of a shop that aren't shop owners.

How to test your changes?

Get added to an EA shop but not as owner, make sure you don't have bulk_data_import or bulk_data_export permissions. Then try running an import e.g. pnpm shopify store copy --from-file=test.sqlite --to-store=rusty-development-dev-3

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@szluzero szluzero self-assigned this Jul 21, 2025
Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
77.94% (-0.01% 🔻)
13011/16694
🟡 Branches
72.09% (+0.03% 🔼)
6343/8799
🟡 Functions
78.09% (-0.01% 🔻)
3372/4318
🟡 Lines
78.35% (-0% 🔻)
12321/15726
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / Dev.tsx
90.59% (-2.35% 🔻)
75% (-1.79% 🔻)
86.36% (-4.55% 🔻)
92.5% (-1.25% 🔻)
🟡
... / admin.ts
75% (-5.43% 🔻)
45.16% (+0.72% 🔼)
90.91%
76.47% (-5.75% 🔻)
🟢
... / file-uploader.ts
89.29% (-0.51% 🔻)
71.88% (-3.13% 🔻)
100%
88.89% (-0.69% 🔻)

Test suite run success

3047 tests passing in 1314 suites.

Report generated by 🧪jest coverage report action from f2fed6f

@szluzero szluzero marked this pull request as ready for review July 21, 2025 20:34
@szluzero szluzero requested review from a team as code owners July 21, 2025 20:34
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

Copy link
Member

@alexcwatt alexcwatt left a comment

Choose a reason for hiding this comment

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

Makes sense to me

@szluzero szluzero added this pull request to the merge queue Jul 23, 2025
Merged via the queue into main with commit bf2fc00 Jul 23, 2025
52 of 53 checks passed
@szluzero szluzero deleted the handle_access_denied_file_upload branch July 23, 2025 18:34
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.

3 participants