sec: 390 - Add validation for potential XSS vuln #2797

ajay-sentry · 2024-04-22T21:27:07Z

Description

This ticket came from the Sentry pen test where it was found this function exposed a potential vulnerability for XSS where a user could end up going to a malicious URL via lack of validation on the "provider" variable coming into the endpoint.

This is definitely true, as we never validated to see if the provider entered was a valid one prior to appending it to our request URL to be called by the fetch API.

IMO, the simplest and most straight forward approach for this issue was to add validation using a pre-existing "isProvider" helper function that will check to see if the provider passed in is equivalent to the provider given by the Api.graphQL() function. If it is a provider, we append it, else we use the base URL (with no additional vars passed in).

Seemingly, this also allowed us to remove another condition added ~6 months back here: #2321, which added an additional variable passed from the TermsOfService component called "supportsServerless" which would "chunk down" the provider URL if supportsServerless was true AND there was no provider. I will need to double check with @RulaKhaled though here.

Also added a bunch of UT's for this new validation case.

Screenshots

Link to Sample Entry

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

codecov-qa · 2024-04-22T21:30:10Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.46%. Comparing base (2274a57) to head (0d96470).

✅ All tests successful. No failed tests found ☺️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2797   +/-   ##
=======================================
  Coverage   98.46%   98.46%           
=======================================
  Files         867      867           
  Lines       12673    12673           
  Branches     3335     3393   +58     
=======================================
  Hits        12478    12478           
  Misses        191      191           
  Partials        4        4

Files	Coverage Δ
src/shared/api/api.js	`98.00% <100.00%> (ø)`
src/shared/api/helpers.ts	`100.00% <100.00%> (ø)`

Components	Coverage Δ
Assets	`55.55% <ø> (ø)`
Layouts	`97.25% <ø> (ø)`
Pages	`99.26% <ø> (ø)`
Services	`99.56% <ø> (ø)`
Shared	`99.84% <100.00%> (ø)`
UI	`94.40% <ø> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2274a57...0d96470. Read the comment docs.

codecov-public-qa · 2024-04-22T21:30:13Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.46%. Comparing base (2274a57) to head (0d96470).

✅ All tests successful. No failed tests found ☺️

@@           Coverage Diff           @@
##             main    #2797   +/-   ##
=======================================
  Coverage   98.46%   98.46%           
=======================================
  Files         867      867           
  Lines       12673    12673           
  Branches     3376     3393   +17     
=======================================
  Hits        12478    12478           
  Misses        191      191           
  Partials        4        4

Files	Coverage Δ
src/shared/api/api.js	`98.00% <100.00%> (ø)`
src/shared/api/helpers.ts	`100.00% <100.00%> (ø)`

Components	Coverage Δ
Assets	`55.55% <ø> (ø)`
Layouts	`97.25% <ø> (ø)`
Pages	`99.26% <ø> (ø)`
Services	`99.56% <ø> (ø)`
Shared	`99.84% <100.00%> (ø)`
UI	`94.40% <ø> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2274a57...0d96470. Read the comment docs.

codecov-notifications · 2024-04-22T21:30:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found ☺️

@@           Coverage Diff           @@
##             main    #2797   +/-   ##
=======================================
  Coverage        ?   98.45%           
=======================================
  Files           ?      873           
  Lines           ?    12632           
  Branches        ?     3357           
=======================================
  Hits            ?    12437           
  Misses          ?      191           
  Partials        ?        4

Files	Coverage Δ
...rc/pages/TermsOfService/hooks/useTermsOfService.ts	`100.00% <ø> (ø)`
src/shared/api/api.js	`97.91% <100.00%> (ø)`
src/shared/api/helpers.ts	`100.00% <100.00%> (ø)`

Components	Coverage Δ
Assets	`55.55% <ø> (?)`
Layouts	`97.25% <ø> (?)`
Pages	`99.28% <ø> (?)`
Services	`99.56% <ø> (?)`
Shared	`99.84% <100.00%> (?)`
UI	`94.18% <ø> (?)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a662e79...d2b9e86. Read the comment docs.

codecov · 2024-04-22T21:30:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.46%. Comparing base (2274a57) to head (0d96470).

✅ All tests successful. No failed tests found ☺️

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #2797   +/-   ##
=====================================
  Coverage   98.46   98.46           
=====================================
  Files        867     867           
  Lines      12673   12673           
  Branches    3393    3393           
=====================================
  Hits       12478   12478           
  Misses       191     191           
  Partials       4       4

Files	Coverage Δ
src/shared/api/api.js	`98.00% <100.00%> (ø)`
src/shared/api/helpers.ts	`100.00% <100.00%> (ø)`

Components	Coverage Δ
Assets	`55.55% <ø> (ø)`
Layouts	`97.25% <ø> (ø)`
Pages	`99.26% <ø> (ø)`
Services	`99.56% <ø> (ø)`
Shared	`99.84% <100.00%> (ø)`
UI	`94.40% <ø> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2274a57...0d96470. Read the comment docs.

codecov-staging · 2024-04-22T21:32:39Z

Bundle Report

Changes will increase total bundle size by 13 bytes ⬆️

Bundle name	Size	Change
gazebo-staging-array-push	6.55MB	13 bytes ⬆️

codecov · 2024-04-22T21:32:52Z

Bundle Report

Changes will increase total bundle size by 13 bytes ⬆️

Bundle name	Size	Change
gazebo-production-array-push	6.55MB	13 bytes ⬆️

codecov-releaser · 2024-04-22T21:37:19Z

✅ Deploy preview for gazebo ready!

Previews expire after 1 month automatically.

Commit	Created	Cloud	Enterprise
`00d6ca9`	Mon, 22 Apr 2024 21:37:19 GMT	Expired	Expired
`d2b9e86`	Mon, 29 Apr 2024 16:57:39 GMT	Expired	Expired
`0d96470`	Tue, 30 Apr 2024 19:07:23 GMT	Cloud	Enterprise

RulaKhaled · 2024-04-30T08:56:27Z

src/shared/api/helpers.ts

@@ -13,7 +13,7 @@ export interface NetworkErrorObject {
  dev: `${string} - ${number} ${string}`
 }

-const AllProvidersArray = [
+export const AllProvidersArray = [


should we take care of capital letters as well? food for thought

RulaKhaled

looks good, thanks!

* add tests, and validation for provider * add back supportServiceless param

* feat: Add activation banner for trial eligible owners * pull out interface + spec stuff * Refactor CircleCI repo onboarding into one file (#2806) * Refactor Other CI repo onboarding into one file (#2807) * Update repo onboarding title position and page alignment (#2818) * sec: 390 - Add validation for potential XSS vuln (#2797) * add tests, and validation for provider * add back supportServiceless param * ref: 1548 Part 1: Convert all Header files to TS (#2821) * ref all header files to TS * remove prop types and rebase * fix: Remove repository from GUT settings page header (#2823) Small tweak removing `repository` from the GUT settings page. * Install radix-ui react radio group (#2825) * Update repo onboarding steps with new Card component (#2819) GH codecov/engineering-team#1665 * Update tests * Update to correct import orders * Update tests --------- Co-authored-by: Spencer Murray <[email protected]> Co-authored-by: ajay-sentry <[email protected]> Co-authored-by: nicholas-codecov <[email protected]>

* add tests, and validation for provider * add back supportServiceless param

* feat: Add activation banner for trial eligible owners * pull out interface + spec stuff * Update to reflect paid plan activation banner * Refactor CircleCI repo onboarding into one file (#2806) * Refactor Other CI repo onboarding into one file (#2807) * Update repo onboarding title position and page alignment (#2818) * sec: 390 - Add validation for potential XSS vuln (#2797) * add tests, and validation for provider * add back supportServiceless param * ref: 1548 Part 1: Convert all Header files to TS (#2821) * ref all header files to TS * remove prop types and rebase * fix: Remove repository from GUT settings page header (#2823) Small tweak removing `repository` from the GUT settings page. * Install radix-ui react radio group (#2825) * Update repo onboarding steps with new Card component (#2819) GH codecov/engineering-team#1665 * feat: Add hasSeatsLeft to plan query * Update to reflect SeatsLimitReachedBanner * feat: paid plan activation banner * update with from FreePlanSeatsLimitBanner * value duplicate * feat: paid plan seats limit banner * just one more small tweak * update name to FreePlanSeatsLimitBanner * remove queryclient call * update tests * fix padding * Update to have a const for plan value --------- Co-authored-by: Spencer Murray <[email protected]> Co-authored-by: ajay-sentry <[email protected]> Co-authored-by: nicholas-codecov <[email protected]>

* feat: Add activation banner for trial eligible owners * pull out interface + spec stuff * Update to reflect paid plan activation banner * Refactor CircleCI repo onboarding into one file (#2806) * Refactor Other CI repo onboarding into one file (#2807) * Update repo onboarding title position and page alignment (#2818) * sec: 390 - Add validation for potential XSS vuln (#2797) * add tests, and validation for provider * add back supportServiceless param * ref: 1548 Part 1: Convert all Header files to TS (#2821) * ref all header files to TS * remove prop types and rebase * fix: Remove repository from GUT settings page header (#2823) Small tweak removing `repository` from the GUT settings page. * Install radix-ui react radio group (#2825) * Update repo onboarding steps with new Card component (#2819) GH codecov/engineering-team#1665 * feat: Add hasSeatsLeft to plan query * Update to reflect SeatsLimitReachedBanner * feat: paid plan activation banner * update with from FreePlanSeatsLimitBanner * value duplicate * feat: Activation required banner * clean up previous commit * match design * fix style * Resolve conflicts --------- Co-authored-by: Spencer Murray <[email protected]> Co-authored-by: ajay-sentry <[email protected]> Co-authored-by: nicholas-codecov <[email protected]>

* feat: Add activation banner for trial eligible owners * pull out interface + spec stuff * Update to reflect paid plan activation banner * Refactor CircleCI repo onboarding into one file (#2806) * Refactor Other CI repo onboarding into one file (#2807) * Update repo onboarding title position and page alignment (#2818) * sec: 390 - Add validation for potential XSS vuln (#2797) * add tests, and validation for provider * add back supportServiceless param * ref: 1548 Part 1: Convert all Header files to TS (#2821) * ref all header files to TS * remove prop types and rebase * fix: Remove repository from GUT settings page header (#2823) Small tweak removing `repository` from the GUT settings page. * Install radix-ui react radio group (#2825) * Update repo onboarding steps with new Card component (#2819) GH codecov/engineering-team#1665 * feat: Add hasSeatsLeft to plan query * Update to reflect SeatsLimitReachedBanner * feat: paid plan activation banner * update with from FreePlanSeatsLimitBanner * value duplicate * feat: paid plan seats limit banner * just one more small tweak * update name to FreePlanSeatsLimitBanner * remove queryclient call * update tests * fix padding * Update to have a const for plan value --------- Co-authored-by: Spencer Murray <[email protected]> Co-authored-by: ajay-sentry <[email protected]> Co-authored-by: nicholas-codecov <[email protected]>

* feat: Add activation banner for trial eligible owners * pull out interface + spec stuff * Update to reflect paid plan activation banner * Refactor CircleCI repo onboarding into one file (#2806) * Refactor Other CI repo onboarding into one file (#2807) * Update repo onboarding title position and page alignment (#2818) * sec: 390 - Add validation for potential XSS vuln (#2797) * add tests, and validation for provider * add back supportServiceless param * ref: 1548 Part 1: Convert all Header files to TS (#2821) * ref all header files to TS * remove prop types and rebase * fix: Remove repository from GUT settings page header (#2823) Small tweak removing `repository` from the GUT settings page. * Install radix-ui react radio group (#2825) * Update repo onboarding steps with new Card component (#2819) GH codecov/engineering-team#1665 * feat: Add hasSeatsLeft to plan query * Update to reflect SeatsLimitReachedBanner * feat: paid plan activation banner * update with from FreePlanSeatsLimitBanner * value duplicate * feat: Activation required banner * clean up previous commit * match design * fix style * Resolve conflicts --------- Co-authored-by: Spencer Murray <[email protected]> Co-authored-by: ajay-sentry <[email protected]> Co-authored-by: nicholas-codecov <[email protected]>

* feat: Add free plan seats alert * feat: Create alerts for paid plan * fix: Remove repositoryDeprecated from path contents hooks (#2845) * update usePrefetchBranchDirEntry * update useRepoBranchContents * update index file * update query off of repositoryDeprecated * update tests * small change to have commit signed * update constants to ts * small tweaks to the query keys * Update useBranchCoverageMeasurements to use repository instead of repositoryDeprecated (#2843) * feat: Paid/Free plan seats limit banner (#2830) * feat: Add activation banner for trial eligible owners * pull out interface + spec stuff * Update to reflect paid plan activation banner * Refactor CircleCI repo onboarding into one file (#2806) * Refactor Other CI repo onboarding into one file (#2807) * Update repo onboarding title position and page alignment (#2818) * sec: 390 - Add validation for potential XSS vuln (#2797) * add tests, and validation for provider * add back supportServiceless param * ref: 1548 Part 1: Convert all Header files to TS (#2821) * ref all header files to TS * remove prop types and rebase * fix: Remove repository from GUT settings page header (#2823) Small tweak removing `repository` from the GUT settings page. * Install radix-ui react radio group (#2825) * Update repo onboarding steps with new Card component (#2819) GH codecov/engineering-team#1665 * feat: Add hasSeatsLeft to plan query * Update to reflect SeatsLimitReachedBanner * feat: paid plan activation banner * update with from FreePlanSeatsLimitBanner * value duplicate * feat: paid plan seats limit banner * just one more small tweak * update name to FreePlanSeatsLimitBanner * remove queryclient call * update tests * fix padding * Update to have a const for plan value --------- Co-authored-by: Spencer Murray <[email protected]> Co-authored-by: ajay-sentry <[email protected]> Co-authored-by: nicholas-codecov <[email protected]> * feat: Paid plans activation required banner (#2832) * feat: Add activation banner for trial eligible owners * pull out interface + spec stuff * Update to reflect paid plan activation banner * Refactor CircleCI repo onboarding into one file (#2806) * Refactor Other CI repo onboarding into one file (#2807) * Update repo onboarding title position and page alignment (#2818) * sec: 390 - Add validation for potential XSS vuln (#2797) * add tests, and validation for provider * add back supportServiceless param * ref: 1548 Part 1: Convert all Header files to TS (#2821) * ref all header files to TS * remove prop types and rebase * fix: Remove repository from GUT settings page header (#2823) Small tweak removing `repository` from the GUT settings page. * Install radix-ui react radio group (#2825) * Update repo onboarding steps with new Card component (#2819) GH codecov/engineering-team#1665 * feat: Add hasSeatsLeft to plan query * Update to reflect SeatsLimitReachedBanner * feat: paid plan activation banner * update with from FreePlanSeatsLimitBanner * value duplicate * feat: Activation required banner * clean up previous commit * match design * fix style * Resolve conflicts --------- Co-authored-by: Spencer Murray <[email protected]> Co-authored-by: ajay-sentry <[email protected]> Co-authored-by: nicholas-codecov <[email protected]> * Unlink PR author on pulls table (#2846) * ref: Use Repository instead of RepositoryDeprecated in useCommitYaml (#2844) * Convert UseCommitYaml to Repository from RepositoryDeprecated * Update query key * ref: Convert useCommitErrors to TS and remove repositoryDeprecated (#2847) * init conversion and remove deprecated, testing * fix tests for useCommitErrors * add additional spec * add dev stuff * update test coverage * feat: Route to plan if user session + to param == plan (#2837) * add plan route logic if to param set as plan * help w/ test * remove some unneeded mocks * remove to * fix spec mocks * fix: Show banner for private repos only (#2853) * fix: Footer Codecov icon displaying incorrect color (#2858) * fill color update * update hex val * Update useRepoSettings to move to new repository type (#2851) * Update usereposettings * Update tests * update test * Update more tests * Update more tests * more test updates * Update type * Update * Update query * Refactor * dep: Update to Sentry RC-1 (#2849) * pin @sentry/react to the latest release candidated * update lockfile * fix: Flaky tests around file explorer tables (#2856) * Fix coverage tab file list table * Fix coverage tab code tree table tests * Fix pull indirect changed files table * Fix commit file explorer table * Fix missed instance of getByText * ref: Convert SessionsTable to tanstack table (#2842) * Convert Access.jsx -> tsx * Fix type issue with TokensTable * Remove unnecessary import * Convert SessionsTable to ts * Add tests * Fix tests * Fix all testLocation: any * Use msw for mutation mock * Improve flatmap callback for readability * Better type hint for flat map return * Fix flake * Fix imports * Fix indirect coverage loss * style: Use Card component in bundle onboarding (#2861) * Fix copy pasta border/padding * Page width change * Vite onboarding overhaul * Webpack onboarding overhaul * Rollup onboarding overhaul * Fix tests * chore: Update codecov.yml so that PR comment isn't sent until 9 uploads (#2865) * Update to 8 uploads before comment to avoid many comments * Oopsie it's actually the ceiling * ref: Convert useComparisonForCommitAndParent to TS and remove repositoryDeprecated (#2857) * fix tests, confirm it works * export type and use instead * feat: Add radio button navigation to repo onboarding (#2839) * Use RadioTileGroup for navigation on the repo onboarding page * Pull getInitalProvider out of component * Fix tests * Remove unnecessary div * fix: A potential fix for the flaky test (#2854) * ref: Clean up pull file explorer tests (#2852) * Clean up pull file explorer tests * Fix tests * Revert "Fix tests" This reverts commit f5aeccd. * components over time display name instead of id (#2868) * style: Fix radio tile group indicator shifting on certain screen sizes (#2869) * Fix responsiveness issue on RadioTileGroup * Add longer description to story * Left align the label text * ref: Remove repositoryDeprecated from usePrefetchSingleFileComp (#2870) * update usePrefetchSingleFileComp to TS * update NameColumn tests * move fragment schema next to gql fragment string * Capitlize view in free alert * oops conflict stuff --------- Co-authored-by: nicholas-codecov <[email protected]> Co-authored-by: Spencer Murray <[email protected]> Co-authored-by: ajay-sentry <[email protected]> Co-authored-by: Rohit Vinnakota <[email protected]> Co-authored-by: JerrySentry <[email protected]>

add tests, and validation for provider

00d6ca9

ajay-sentry mentioned this pull request Apr 25, 2024

fix: use session if exists when doing initial redirect for user #2810

Merged

ajay-sentry added 2 commits April 29, 2024 09:44

Merge branch 'main' into Ajay/390-clientside-path-traversal

f3ba5e0

add back supportServiceless param

d2b9e86

RulaKhaled reviewed Apr 30, 2024

View reviewed changes

RulaKhaled approved these changes Apr 30, 2024

View reviewed changes

Merge branch 'main' into Ajay/390-clientside-path-traversal

0d96470

ajay-sentry merged commit 940744c into main Apr 30, 2024
51 checks passed

ajay-sentry deleted the Ajay/390-clientside-path-traversal branch April 30, 2024 20:11

RulaKhaled pushed a commit that referenced this pull request May 2, 2024

sec: 390 - Add validation for potential XSS vuln (#2797)

f19ba93

* add tests, and validation for provider * add back supportServiceless param

RulaKhaled pushed a commit that referenced this pull request May 2, 2024

sec: 390 - Add validation for potential XSS vuln (#2797)

325b5c6

* add tests, and validation for provider * add back supportServiceless param

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sec: 390 - Add validation for potential XSS vuln #2797

sec: 390 - Add validation for potential XSS vuln #2797

ajay-sentry commented Apr 22, 2024

codecov-qa bot commented Apr 22, 2024 •

edited

Loading

codecov-public-qa bot commented Apr 22, 2024 •

edited

Loading

codecov-notifications bot commented Apr 22, 2024 •

edited

Loading

codecov bot commented Apr 22, 2024 •

edited

Loading

codecov-staging bot commented Apr 22, 2024 •

edited

Loading

codecov bot commented Apr 22, 2024 •

edited

Loading

codecov-releaser commented Apr 22, 2024 •

edited

Loading

RulaKhaled Apr 30, 2024

RulaKhaled left a comment

sec: 390 - Add validation for potential XSS vuln #2797

sec: 390 - Add validation for potential XSS vuln #2797

Conversation

ajay-sentry commented Apr 22, 2024

Description

Screenshots

Link to Sample Entry

Legal Boilerplate

codecov-qa bot commented Apr 22, 2024 • edited Loading

Codecov Report

codecov-public-qa bot commented Apr 22, 2024 • edited Loading

Codecov Report

codecov-notifications bot commented Apr 22, 2024 • edited Loading

Codecov Report

codecov bot commented Apr 22, 2024 • edited Loading

Codecov Report

codecov-staging bot commented Apr 22, 2024 • edited Loading

Bundle Report

codecov bot commented Apr 22, 2024 • edited Loading

Bundle Report

codecov-releaser commented Apr 22, 2024 • edited Loading

✅ Deploy preview for gazebo ready!

RulaKhaled Apr 30, 2024

Choose a reason for hiding this comment

RulaKhaled left a comment

Choose a reason for hiding this comment

codecov-qa bot commented Apr 22, 2024 •

edited

Loading

codecov-public-qa bot commented Apr 22, 2024 •

edited

Loading

codecov-notifications bot commented Apr 22, 2024 •

edited

Loading

codecov bot commented Apr 22, 2024 •

edited

Loading

codecov-staging bot commented Apr 22, 2024 •

edited

Loading

codecov bot commented Apr 22, 2024 •

edited

Loading

codecov-releaser commented Apr 22, 2024 •

edited

Loading