News' "add this RSS feed" functionality doesn't honor the HTTPS upgrade setting

[fmarier]

Steps To Reproduce:

1. Enable Brave News
2. In Brave settings (brave://settings/shields), set "Upgrade connections to HTTPS" to "Strict"
3. Open WireShark. Enable capturing. Set the display filter in WireShark to http
4. Visit https://fmarier.github.io/brave-testing/news-add-rss.html
5. Click the feed icon in the URL bar
6. Go back to WireShark. You can see a plaintext request sent to 172.105.6.87

Actual

Expected

The request should be upgraded to HTTPS and no HTTP request should be visible in WireShark.

Originally reported at https://hackerone.com/reports/2502007

[fmarier]

The other thing that this suggests is that we are likely not running these URLs through our privacy filters (e.g. debouncer, query string filter).

[fmarier]

To confirm, I updated the test page to add an fbclid parameter to the URL and it doesn't get stripped out:

[bsclifton]

cc: @LorenzoMinto

[diracdeltas]

@bsclifton is anyone able to take this issue?

[diracdeltas]

@boocmp do you think you could take this? assuming @LorenzoMinto / @fallaciousreasoning have not started on it.

[fallaciousreasoning]

@fmarier I suspect this is probably a broader problem with our ApiRequestHelper class so this probably affects everything that uses it 😨

[boocmp]

ApiRequestHelper uses the SharedUploaderFactory which bypasses any Brave code for requests passing through it, I always thought it is by design.

[fallaciousreasoning]

cc @petemill not really sure on the best approach here if its by design that the APIRequestHelper isn't going through Brave's request handling code?

[fmarier]

I think this should be a News-specific fix (for these custom RSS feeds). We should not change any other internal requests.

[diracdeltas]

Reopening as the fix for this was incomplete, but feel free to close it again and just open a follow-up if that's easier: https://bravesoftware.slack.com/archives/C6R461GF4/p1733936717990789

[DJAndries]

Reopening as the fix for this was incomplete, but feel free to close it again and just open a follow-up if that's easier: https://bravesoftware.slack.com/archives/C6R461GF4/p1733936717990789

Merged brave/brave-core#26989 as a follow up last week

[LaurenWags]

@fmarier do you think this one should be tested on all desktop OSes (Windows, macOS, Linux) or is verification on one OS sufficient? If you think this is important enough to test on all desktop OSes please add the QA/Test-All-Platforms label. Thanks!

[fmarier]

@LaurenWags It should be fine on just one desktop platform. There's nothing OS-specific about this.

[LaurenWags]

Excellent, thanks for the input @fmarier 😄

[MadhaviSeelam]

Verification PASSED using

Brave | 1.75.159 Chromium: 132.0.6834.83 (Official Build) beta (64-bit)
-- | --
Revision | a7b6f5093a2dad31fc3f56241a7d55be255e69e2
OS | Windows 11 Version 24H2 (Build 26100.2605)

Reproduced the issue in 1.73.105 using the STR from #38282 (comment)

1. Installed 1.75.159
2. launched Brave
3. Enable Brave News in the NTP
4. opened brave://settings/shields
5. set "Upgrade connections to HTTPS" to "Strict"
6. opened WireShark and enable capturing (wifi)
7. Set the display filter in WireShark to http
8. visited https://fmarier.github.io/brave-testing/news-add-rss.html
9. Click the feed icon in the URL bar

Confirmed no http request is shown related to brave-news in the wireshark

example	example	example	example	example