Skip to content

Conversation

@thst-nordic
Copy link
Contributor

Potential fix for https://github.com/nrfconnect/sdk-nrf/security/code-scanning/35

To fix this, the code should correctly determine whether a package URL truly references GitHub as its host/domain. Instead of 'github.com' in package.url, we should parse the URL using Python's urllib.parse module and check if the netloc (hostname part) is exactly github.com. Additionally, it's common for GitHub URLs to start with www.github.com, so we should ideally accept both.

Specifically, in the referenced region (lines 101–106), replace the substring search with:

  • Parse the URL using urlparse
  • Check that the netloc/hostname is exactly (or ends with) github.com (optionally including www.github.com)
  • Only then treat it as a GitHub package

You will need to import urlparse from urllib.parse, if not already done.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…g sanitization

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@NordicBuilder NordicBuilder added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Oct 24, 2025
if (package.name is None) and ('github.com' in package.url):
offs = package.url.find('github.com') + len('github.com') + 1
parsed_url = urlparse(package.url)
if (package.name is None) and (parsed_url.hostname and parsed_url.hostname.lower().endswith("github.com")):

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

The string
github.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 6 days ago

To fix the problem, we must parse the hostname and ensure that it represents the expected host. The best way to ensure robust handling is to check that the hostname either exactly matches "github.com" or ends with ".github.com", which safely includes the root domain and any direct subdomains, but excludes things like evil-github.com. Modify the block in scripts/west_commands/sbom/output_pre_process.py that checks parsed_url.hostname.lower().endswith("github.com") to instead check:

hostname = parsed_url.hostname.lower()
if package.name is None and (hostname == "github.com" or hostname.endswith(".github.com")):
    ...

No additional imports are needed, and all changes are local to this function.

Suggested changeset 1
scripts/west_commands/sbom/output_pre_process.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/scripts/west_commands/sbom/output_pre_process.py b/scripts/west_commands/sbom/output_pre_process.py
--- a/scripts/west_commands/sbom/output_pre_process.py
+++ b/scripts/west_commands/sbom/output_pre_process.py
@@ -99,7 +99,8 @@
         if package.url is None:
             continue
         parsed_url = urlparse(package.url)
-        if (package.name is None) and (parsed_url.hostname and parsed_url.hostname.lower().endswith("github.com")):
+        hostname = parsed_url.hostname.lower() if parsed_url.hostname else ""
+        if (package.name is None) and (hostname == "github.com" or hostname.endswith(".github.com")):
             offs = package.url.find(parsed_url.hostname) + len(parsed_url.hostname) + 1
             package.name = package.url[offs:]
             if package.name.endswith('.git'):
EOF
@@ -99,7 +99,8 @@
if package.url is None:
continue
parsed_url = urlparse(package.url)
if (package.name is None) and (parsed_url.hostname and parsed_url.hostname.lower().endswith("github.com")):
hostname = parsed_url.hostname.lower() if parsed_url.hostname else ""
if (package.name is None) and (hostname == "github.com" or hostname.endswith(".github.com")):
offs = package.url.find(parsed_url.hostname) + len(parsed_url.hostname) + 1
package.name = package.url[offs:]
if package.name.endswith('.git'):
Copilot is powered by AI and may make mistakes. Always verify output.
@thst-nordic thst-nordic requested a review from nicu1989 October 24, 2025 12:37
@NordicBuilder
Copy link
Contributor

CI Information

To view the history of this post, click the 'edited' button above
Build number: 1

Inputs:

Sources:

more details

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (0)

Outputs:

Toolchain

Version:
Build docker image:

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ❌ Toolchain
  • ❌ Build twister
  • ❌ Integration tests

Note: This message is automatically posted and updated by the CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants