Harden blueman/Functions.py: bug fixes, portability, argv launch, tests - #3317
Open
geraldo-netto wants to merge 11 commits into
Open
Harden blueman/Functions.py: bug fixes, portability, argv launch, tests#3317geraldo-netto wants to merge 11 commits into
geraldo-netto wants to merge 11 commits into
Conversation
Exact powers of 1024 fell through to the GB branch because both band edges used strict `<`, so format_bytes(1024) returned (9.5e-07, "GB") instead of (1.0, "KB"); 1 MiB and 1 GiB were mislabelled the same way. Drop the lower-bound comparison and rely on the cascading upper bounds so each boundary lands in its own unit. Add test/test_functions.py covering the 1024/1048576/1073741824 boundaries plus zero, sub-KB, mid-band, and a huge value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
adapter_path_to_name parses a D-Bus object path with a greedy `re.search(r".*(hci[0-9]*)", path)` and had no tests. Add cases pinning the current contract: normal paths, None/empty -> None, no-hci -> None, case sensitivity, trailing device segments, zero-digit "hci", greedy last-occurrence selection, and embedded matches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…zz-5, data-3)
parse_os_release was nested inside log_system_info and split each line
with `line.split("=")`, so a valid quoted value containing "=" (e.g.
PRETTY_NAME="Name=Variant") raised ValueError and was dropped from the
logged system info.
Promote it to module scope, parse with str.partition("=") so only the
first "=" separates key from value, and skip blank lines explicitly.
Add tests for basic keys, a value containing "=", unquoted values,
comment/blank lines, lines without "=", and a missing file.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
have() hand-rolled a PATH scan with a hardcoded ":/sbin:/usr/sbin" suffix and checked os.access(path, os.EX_OK) -- os.EX_OK is 0, i.e. F_OK, so it only confirmed existence, not executability. Delegate the lookup to shutil.which, which honours the executable bit, and append the sbin directories to the search path only when they are not already present. Add tests covering found/not-found, sbin-dir augmentation, and de-duplication. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
create_logger unconditionally constructed SysLogHandler(address="/dev/log"), which raises on platforms and minimal containers without that socket, taking down the whole process at logger setup. Guard the handler construction and, on OSError, log a warning and keep the basicConfig stderr handler instead. Add tests for the available, unavailable, and syslog-disabled paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
--loglevel had no choices and no help, so a typo like `--loglevel verbose` silently coerced to WARNING across all entry points, leaving users with quieter logs than intended and no error. Add case-insensitive choices (debug/info/warning/error/critical) via type=str.lower plus help text, so argparse rejects unknown values clearly. Existing consumers compare args.LEVEL.upper(), which is unaffected. Add tests for default, lowercasing, rejection, help/choices metadata, the syslog flag, and disabling loglevel. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "applet needs to be running" and "Failed to enable bluetooth" messages went to stdout via print(), bypassing the logging configuration and leaving no record in syslog/journald. Route both through logging: logging.exception on the DBusProxyFailed path (captures the traceback) and logging.error on the enable-failure path. Add tests for the missing-applet exit path and the no-PowerManager early return. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
set_proc_title unconditionally loaded libc.so.6 and called prctl(PR_SET_NAME), both Linux/glibc specific. On other platforms the LoadLibrary or prctl lookup raises and crashes process startup. Return early as a no-op on non-Linux, wrap the libc/prctl access in try/except returning -1 on failure, and document the behaviour in the docstring. Add tests for the non-Linux no-op, the Linux prctl path, and the libc-unavailable failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
launch() accepted only a full command-line string, so callers embedded
options directly in cmd (e.g. Notes.py built
"blueman-sendto --delete --device={addr}"), making argument boundaries
depend on GLib command-line parsing rather than an argv contract.
Add an optional args iterable: when provided, the program token and each
argument are shell-quoted individually via GLib.shell_quote, so spaces,
quotes, and shell metacharacters can never cross argument boundaries.
The legacy string form still works when args is omitted (deprecated).
Migrate the Notes.py send-note call site to the argv form.
Add tests for the legacy form, argv quoting, metacharacter
neutralization, the launch result, and path-to-GFile conversion.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
doc-1 framed these as dead helpers to "document or remove", but they are live: every entry point in apps/*.in imports and calls them (the audit missed the .in sources). Document them instead of removing, noting their role and the syslog fallback / shared CLI surface. set_proc_title was already documented alongside the leg-5 platform guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
geraldo-netto
force-pushed
the
fix/functions-py-hardening
branch
from
June 19, 2026 18:02
e67b01e to
be6a846
Compare
Inside an except block logging.exception() is the idiomatic call: it logs at ERROR with the active traceback, so the explicit exc_info=True is redundant and the bare logging.error drops the trace. Convert the five in-handler calls (set_proc_title, _netmask_for_ifacename, get_local_interfaces x2, parse_os_release). The remaining logging.error calls are plain error conditions outside any except block, where logging.exception would log a spurious "NoneType: None", so they are left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Works through the open
blueman/Functions.pyfindings. Each commit is one fix; all changes ship with focused tests in a newtest/test_functions.py.Fixes
format_bytes(1024)returned(9.5e-07, "GB")). Corrected the unit boundaries.str.partition("=")so quoted values containing=(e.g.PRETTY_NAME="Name=Variant") survive instead of being dropped.shutil.which(honours the executable bit; the oldos.EX_OKcheck only tested existence) and append the sbin dirs only when missing./dev/logis unavailable instead of crashing at logger setup (non-Linux / minimal containers).choices+help, so typos like--loglevel verboseare rejected instead of silently coercing to WARNING.logginginstead ofprint.args=[...], each token shell-quoted viaGLib.shell_quote) so options can no longer cross argument boundaries; migrated the Notes.py send-note call site. Legacy string form still works (deprecated).create_logger/create_parser.Tests
test/test_functions.py(45 tests) covering all of the above, including boundary/fuzz inputs forformat_bytes,adapter_path_to_name, andparse_os_release.mypy -p blueman --strictclean; flake8 (core codes) clean.🤖 Generated with Claude Code