Skip to content

grass.script: Validate URL schemes before opening remote URLs - #7715

Open
cwhite911 wants to merge 1 commit into
OSGeo:mainfrom
cwhite911:urlopen-scheme-check
Open

grass.script: Validate URL schemes before opening remote URLs#7715
cwhite911 wants to merge 1 commit into
OSGeo:mainfrom
cwhite911:urlopen-scheme-check

Conversation

@cwhite911

Copy link
Copy Markdown
Contributor

Addresses the 9 open Bandit B310 (urllib_urlopen / "audit url open for permitted schemes") code scanning alerts.

These are low-severity: the URLs come from the user's own arguments, so this is defense-in-depth rather than a fix for a distinct vulnerability. (The related XML entity-expansion issue in the same tools is being handled separately.)

Change

  • Adds grass.script.utils.check_url_scheme(url, allowed_schemes=("http", "https")), which raises ValueError when a URL uses an unexpected scheme (e.g. file:).
  • Calls it before urlopen/urlretrieve in the tools that only ever contact a remote service: r.in.wms, v.in.wfs, g.extension, g.extension.all, and g.manual. In g.extension the check goes in the shared urlopen/urlretrieve wrappers, so every remote fetch is covered; its validate_url() already handles local paths before reaching these wrappers, so nothing local is affected.
  • g.download.project (via grass.utils.download) documents that its source may be a local file path, so the scheme there is intentionally left unrestricted (restricting it would also be fragile with bare paths and Windows drive letters). Those two calls are annotated instead.

Because Bandit flags the urlopen/urlretrieve call site regardless of a runtime guard, the validated (or fixed-URL) calls are marked # nosec B310. S310 is already in the ruff ignore list, so no ruff change is needed.

Verification

  • Bandit 1.9.4 on the affected files: 9 B310 findings before, 0 after.
  • check_url_scheme accepts http/https and rejects file:, ftp:, and gopher: (unit-checked on Python 3.10 and 3.12).
  • ruff check/ruff format clean.

Written with the assistance of Claude Code.

@github-actions github-actions Bot added vector Related to vector data processing raster Related to raster data processing Python Related code is in Python libraries module general labels Jul 14, 2026
Add grass.script.utils.check_url_scheme(), which rejects URLs whose
scheme is not in an allowlist (http/https by default), and call it
before urlopen/urlretrieve in the tools that only ever contact a remote
service: r.in.wms, v.in.wfs, g.extension, g.extension.all, and
g.manual. This prevents opening unexpected schemes such as file: on a
user-supplied service URL.

g.download.project intentionally accepts a local file path via
grass.utils.download, so its scheme is left unrestricted and the two
calls are annotated accordingly.

Clears 9 Bandit B310 alerts (the calls are marked nosec B310 where the
scheme is now validated or fixed).
@cwhite911
cwhite911 force-pushed the urlopen-scheme-check branch from 1740b88 to 61abf1b Compare July 18, 2026 10:54
@cwhite911

Copy link
Copy Markdown
Contributor Author

Rebased onto current main to resolve the conflict.

Since this PR was opened, main gained a download.py refactor (supported_url_schemes + _download_file(), which validates the scheme and annotates the urlretrieve call). That already covers the grass.utils.download / g.download.project path, including file:// for local archives, so I dropped this PR's download.py edits and took main's version.

What remains here is the scheme validation for the tools main did not touch, whose urlopen calls were still unguarded: r.in.wms, v.in.wfs, g.extension / g.extension.all, and g.manual, via the reusable grass.script.utils.check_url_scheme() helper (http/https only for these service URLs).

There is a small redundancy worth noting: download.py now has its own inline supported_url_schemes check while this adds check_url_scheme in grass.script.utils. They cover different code paths and scheme sets (download allows file; these services only http/https). Happy to unify them (e.g. have download.py reuse check_url_scheme) in this PR or a follow-up if you prefer.

Verified: bandit 1.9.4 reports 0 B310 on the touched tools, check_url_scheme accepts http/https and rejects file:/ftp:/gopher:, ruff/pre-commit pass.

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

Labels

general libraries module Python Related code is in Python raster Related to raster data processing vector Related to vector data processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant