Skip to content

Security fixes: B310, B324, B110 (bandit scan) - #354

Closed
lawrencel1ng wants to merge 4 commits into
nesquena:masterfrom
lawrencel1ng:master
Closed

lawrencel1ng wants to merge 4 commits into
nesquena:masterfrom
lawrencel1ng:master

Conversation

@lawrencel1ng

Copy link
Copy Markdown

Automated security fixes from cron job:

  • B324: Weak MD5 hash → added usedforsecurity=False
  • B310: Unsafe urlopen → added URL scheme validation
  • B110: Bare except/pass → replaced with logger.debug() calls

All tests pass.

lawrencel1ng added 4 commits April 13, 2026 15:38
- Add usedforsecurity=False to MD5 hash in gateway_watcher.py
- Add URL scheme validation to prevent file:// access in config.py
- Add URL validation to bootstrap.py health check
- Add nosec comments where runtime validation exists
- Add QuietHTTPServer class to suppress noisy connection reset errors
  caused by clients disconnecting abruptly (fixes log spam from
  'ConnectionResetError: [Errno 54] Connection reset by peer')

- Replace silent 'pass' statements with logger.debug() calls across
  api/auth.py, api/config.py, api/gateway_watcher.py, api/models.py,
  and api/onboarding.py for better observability during troubleshooting

- All tests pass (25 passed in test_regressions.py)
- Replace silent 'pass' statements with logger.debug() calls in
  api/profiles.py for better error visibility during profile switching
  and module patching

- Add logger initialization to api/routes.py
- Replace bare except/pass patterns with logger.debug() calls
- Fixes CWE-703 (improper check/handling of exceptional conditions)
- Files affected: routes.py, state_sync.py, streaming.py, workspace.py, server.py
- All tests pass successfully
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Review: Security fixes — B310, B324, B110 (bandit scan)

Thanks for this! All three bandit findings are addressed correctly. Here's the breakdown:


✅ B324 — MD5 usedforsecurity=False (gateway_watcher.py)

return hashlib.md5(key.encode(), usedforsecurity=False).hexdigest()

Correct fix. The MD5 here is used purely for change-detection hashing (snapshot comparison), not for any security purpose. The usedforsecurity=False flag is the right suppression — it tells bandit and FIPS-mode Python that this is intentional.


✅ B310 — URL scheme validation (config.py, bootstrap.py)

if parsed_url.scheme not in ("", "http", "https"):
    raise ValueError(f"Invalid URL scheme: {parsed_url.scheme}")

Good approach. Validating the scheme before calling urlopen is the correct pattern. The # nosec B310 annotation is appropriate because the runtime guard precedes each call.

One small note: In config.py the scheme check runs on a URL that may have had http:// prepended (endpoint_url if "://" in endpoint_url else f"http://{endpoint_url}"). After the prepend, the scheme will always be http or https if the raw input had :// in it — the validation still holds, just worth being aware of.


✅ B110 — Bare except: passlogger.debug() (all modules)

All silent swallows replaced with logger.debug() calls. This is a quality improvement beyond just silencing bandit — it makes the error paths observable during troubleshooting without adding noise to production logs.

The QuietHTTPServer addition in server.py is a nice companion fix — it stops the ConnectionResetError log spam that happens when clients disconnect abruptly, which is separate from the B110 fixes but improves the signal-to-noise ratio in logs.


Tests

The PR description says all tests pass. No new tests added for these changes, which is appropriate — these are mechanical substitutions with no behavior change (debug logging is no-op at default log levels, and the URL scheme check throws on inputs that would have caused urlopen to fail anyway).


Summary

Finding Fix Status
B324 (MD5 hash) usedforsecurity=False ✅ Correct
B310 (unsafe urlopen) Scheme validation + # nosec ✅ Correct
B110 (bare except/pass) logger.debug() replacements ✅ Correct
Bonus: connection reset spam QuietHTTPServer ✅ Good addition

Looks good to me — clean, targeted fixes with no behavior regressions.

nesquena-hermes pushed a commit that referenced this pull request Apr 13, 2026
- api/gateway_watcher.py: MD5 usedforsecurity=False (B324)
- api/config.py, bootstrap.py: URL scheme validation before urlopen (B310)
- 12 files: replace bare except/pass with logger.debug() (B110)
- server.py: QuietHTTPServer suppresses client disconnect log noise
- server.py: fix sys.exc_info() (was traceback.sys.exc_info(), impl detail)
- tests/test_sprint43.py: 19 new tests covering all security fixes
- CHANGELOG.md: v0.50.14 entry; 841 tests total (up from 822)
nesquena-hermes added a commit that referenced this pull request Apr 13, 2026
* security: fix bandit security issues (B310, B324)

- Add usedforsecurity=False to MD5 hash in gateway_watcher.py
- Add URL scheme validation to prevent file:// access in config.py
- Add URL validation to bootstrap.py health check
- Add nosec comments where runtime validation exists

* fix: handle ConnectionResetError gracefully and add debug logging

- Add QuietHTTPServer class to suppress noisy connection reset errors
  caused by clients disconnecting abruptly (fixes log spam from
  'ConnectionResetError: [Errno 54] Connection reset by peer')

- Replace silent 'pass' statements with logger.debug() calls across
  api/auth.py, api/config.py, api/gateway_watcher.py, api/models.py,
  and api/onboarding.py for better observability during troubleshooting

- All tests pass (25 passed in test_regressions.py)

* chore: add debug logging to profiles and routes modules

- Replace silent 'pass' statements with logger.debug() calls in
  api/profiles.py for better error visibility during profile switching
  and module patching

- Add logger initialization to api/routes.py

* security: fix B110 bare except/pass issues (bandit security scan)

- Replace bare except/pass patterns with logger.debug() calls
- Fixes CWE-703 (improper check/handling of exceptional conditions)
- Files affected: routes.py, state_sync.py, streaming.py, workspace.py, server.py
- All tests pass successfully

* security: bandit fixes B310/B324/B110 + QuietHTTPServer (#354)

- api/gateway_watcher.py: MD5 usedforsecurity=False (B324)
- api/config.py, bootstrap.py: URL scheme validation before urlopen (B310)
- 12 files: replace bare except/pass with logger.debug() (B110)
- server.py: QuietHTTPServer suppresses client disconnect log noise
- server.py: fix sys.exc_info() (was traceback.sys.exc_info(), impl detail)
- tests/test_sprint43.py: 19 new tests covering all security fixes
- CHANGELOG.md: v0.50.14 entry; 841 tests total (up from 822)

---------

Co-authored-by: lawrencel1ng <lawrence.ling@global.ntt>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Closed in favor of #361 which merged this fix rebased onto master (resolved api/profiles.py conflict with v0.50.12 env isolation), fixed traceback.sys.exc_info() → sys.exc_info(), and added 19 tests (test_sprint43.py). 841 tests pass on master.

JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
* security: fix bandit security issues (B310, B324)

- Add usedforsecurity=False to MD5 hash in gateway_watcher.py
- Add URL scheme validation to prevent file:// access in config.py
- Add URL validation to bootstrap.py health check
- Add nosec comments where runtime validation exists

* fix: handle ConnectionResetError gracefully and add debug logging

- Add QuietHTTPServer class to suppress noisy connection reset errors
  caused by clients disconnecting abruptly (fixes log spam from
  'ConnectionResetError: [Errno 54] Connection reset by peer')

- Replace silent 'pass' statements with logger.debug() calls across
  api/auth.py, api/config.py, api/gateway_watcher.py, api/models.py,
  and api/onboarding.py for better observability during troubleshooting

- All tests pass (25 passed in test_regressions.py)

* chore: add debug logging to profiles and routes modules

- Replace silent 'pass' statements with logger.debug() calls in
  api/profiles.py for better error visibility during profile switching
  and module patching

- Add logger initialization to api/routes.py

* security: fix B110 bare except/pass issues (bandit security scan)

- Replace bare except/pass patterns with logger.debug() calls
- Fixes CWE-703 (improper check/handling of exceptional conditions)
- Files affected: routes.py, state_sync.py, streaming.py, workspace.py, server.py
- All tests pass successfully

* security: bandit fixes B310/B324/B110 + QuietHTTPServer (nesquena#354)

- api/gateway_watcher.py: MD5 usedforsecurity=False (B324)
- api/config.py, bootstrap.py: URL scheme validation before urlopen (B310)
- 12 files: replace bare except/pass with logger.debug() (B110)
- server.py: QuietHTTPServer suppresses client disconnect log noise
- server.py: fix sys.exc_info() (was traceback.sys.exc_info(), impl detail)
- tests/test_sprint43.py: 19 new tests covering all security fixes
- CHANGELOG.md: v0.50.14 entry; 841 tests total (up from 822)

---------

Co-authored-by: lawrencel1ng <lawrence.ling@global.ntt>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
* security: fix bandit security issues (B310, B324)

- Add usedforsecurity=False to MD5 hash in gateway_watcher.py
- Add URL scheme validation to prevent file:// access in config.py
- Add URL validation to bootstrap.py health check
- Add nosec comments where runtime validation exists

* fix: handle ConnectionResetError gracefully and add debug logging

- Add QuietHTTPServer class to suppress noisy connection reset errors
  caused by clients disconnecting abruptly (fixes log spam from
  'ConnectionResetError: [Errno 54] Connection reset by peer')

- Replace silent 'pass' statements with logger.debug() calls across
  api/auth.py, api/config.py, api/gateway_watcher.py, api/models.py,
  and api/onboarding.py for better observability during troubleshooting

- All tests pass (25 passed in test_regressions.py)

* chore: add debug logging to profiles and routes modules

- Replace silent 'pass' statements with logger.debug() calls in
  api/profiles.py for better error visibility during profile switching
  and module patching

- Add logger initialization to api/routes.py

* security: fix B110 bare except/pass issues (bandit security scan)

- Replace bare except/pass patterns with logger.debug() calls
- Fixes CWE-703 (improper check/handling of exceptional conditions)
- Files affected: routes.py, state_sync.py, streaming.py, workspace.py, server.py
- All tests pass successfully

* security: bandit fixes B310/B324/B110 + QuietHTTPServer (nesquena#354)

- api/gateway_watcher.py: MD5 usedforsecurity=False (B324)
- api/config.py, bootstrap.py: URL scheme validation before urlopen (B310)
- 12 files: replace bare except/pass with logger.debug() (B110)
- server.py: QuietHTTPServer suppresses client disconnect log noise
- server.py: fix sys.exc_info() (was traceback.sys.exc_info(), impl detail)
- tests/test_sprint43.py: 19 new tests covering all security fixes
- CHANGELOG.md: v0.50.14 entry; 841 tests total (up from 822)

---------

Co-authored-by: lawrencel1ng <lawrence.ling@global.ntt>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants