Repository navigation
fix(apple): Added methods to add and update Sentry build phase script - #1029
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1029 +/- ##
==========================================
+ Coverage 28.09% 28.61% +0.51%
==========================================
Files 128 128
Lines 14452 14539 +87
Branches 904 932 +28
==========================================
+ Hits 4061 4160 +99
+ Misses 10374 10362 -12
Partials 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…isting-upload-symbols-script
| inputPaths: string[] = [], | ||
| ): string { | ||
| const buildPhaseUuid = this.project.generateUuid(); | ||
| const escapedScript = script.replace(/"/g, '\\"'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding
This autofix suggestion was applied.
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 year ago
To fix the issue, we need to ensure that backslashes in the script parameter are properly escaped before the string is used. This can be achieved by first replacing all backslashes (\) with double backslashes (\\) and then escaping double quotes ("). This ensures that both backslashes and double quotes are correctly handled.
The best way to implement this fix is to use two replace calls in sequence:
- Replace all backslashes with double backslashes.
- Replace all double quotes with escaped double quotes.
Alternatively, a single regular expression can be used to handle both cases, but the two-step approach is simpler and easier to understand.
| @@ -528,3 +528,3 @@ | ||
| const buildPhaseUuid = this.project.generateUuid(); | ||
| const escapedScript = script.replace(/"/g, '\\"'); | ||
| const escapedScript = script.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); | ||
|
|
| return; | ||
| } | ||
|
|
||
| const escapedScript = script.replace(/"/g, '\\"'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding
This autofix suggestion was applied.
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 year ago
To fix the problem, we should ensure that both backslashes and double quotes are escaped in the script string before embedding it in the shell script. This can be done by first replacing all backslashes with double backslashes, and then replacing all double quotes with escaped double quotes, using regular expressions with the global flag. The fix should be applied to line 581 in the updateScriptBuildPhase method, making it consistent with the escaping logic used in the addScriptBuildPhase method on line 529. No new imports or dependencies are required.
| @@ -580,3 +580,3 @@ | ||
|
|
||
| const escapedScript = script.replace(/"/g, '\\"'); | ||
| const escapedScript = script.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); | ||
| buildPhase.shellScript = `"${escapedScript}"`; |
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
Converting back to draft because tests are failing |
philippsuess
left a comment
There was a problem hiding this comment.
LGTM, the PR description mentions how to test this change. Is it worth the effort to add these manual test to the test suite?
|
Looking at the change in code coverage from Codecov, I'll add additional tests to keep coverage up. We should implement the command as an integration test case in #888. |
|
You can still merge the PR, if you want to and follow up with the tests. Up to you @philprime. |
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Added methods to add and update Sentry build phase script ([#1029](https://github.com/getsentry/sentry-wizard/pull/1029))If none of the above apply, you can opt out of this check by adding |
This PR should be merged after #1028 is merged.
This PR fixes #987 by updating existing Sentry build phase scripts instead of re-creating them.
It can be tested by using the following command from the project root:
gitto view the changes infixtures/test-applications/apple/project-with-synchronized-folders/Project.xcodeproj/project.pbxprojand it will have added a new script calledUpload Debug Symbols to Sentry.Closes Fix Xcode build script injection to remove orphaned build scripts #987