Release v0.12.0 - #29
Merged
Merged
Conversation
Everything about how a map is drawn had to be typed on every run, and so did --overrides. This adds ~/.config/unifi-map/config.toml (honouring XDG_CONFIG_HOME, or $UNIFI_MAP_CONFIG) and a matching UNIFI_MAP_* variable for each of theme, layout, icons, formats, overrides and the three directories. Both mechanisms rather than one. A file suits somebody whose taste differs from the defaults; environment variables are what make the tool configurable in a container without mounting anything. Precedence is flag, then environment, then config file, then default: environment above file so a config baked into an image can be overridden per deployment. --obfuscate and --force are deliberately excluded. One asserts that the output is safe to share and the other overwrites files, and neither should be answerable by ambient state invisible at the call site. Two things this turned up. The render flags had real argparse defaults rather than SUPPRESS, so nothing from the environment could ever have applied to them; they now suppress with their defaults in GLOBAL_DEFAULTS, which needed no change to the doc generator since it already falls back there. And a value injected this way never passes argparse's own choices check, so _validate_injected checks it against the same constants the arguments are declared with and names the source in the error. Every render now reports which settings it did not get from the command line and where each came from. Supporting three sources is what makes "it looks different on your machine" possible, and one log line answers it. UNIFI_CACHE_DIR, UNIFI_ASSET_CACHE and UNIFI_OUT_DIR are renamed to the UNIFI_MAP_* forms. UNIFI_* is the console's namespace and these were never console settings. The old names still work and warn. The tests were reading the developer's own environment, which this turned from latent into a failure: a deprecation warning from a real UNIFI_CACHE_DIR broke an unrelated test about --site. conftest now strips implicit configuration discovery for the whole suite, replacing a per-class workaround that existed for exactly this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Compared modelling against ScottiBYTE/unifi-topology and merlijntishauser/unifi-topology, then checked every claim against a real snapshot rather than against the other projects' source alone. Four findings became tickets; the reasoning lands here and the summaries in TODO.md. port_overrides[].name holds user-written port labels we read nowhere. port_table[].name is a generic "Port N" on 75 of 79 ports on the reference network, while port_overrides carries a real label on 26. Blocked on an obfuscation guard, because obfuscate.py rebuilds nodes with replace() and so passes any new Node field through untouched, while edges are built field by field and drop one silently. A port named after a person or a room would ride straight into a map that promised to be shareable. The controller's v2 topology payload has a has_unknown_switch boolean we ignore, and several wired clients sharing one switch port says where that switch is. Together they are worth reporting; separately neither locates anything. Reported and not drawn, because the cause is ambiguous between an unmanaged switch and a virtualisation host. LLDP was got wrong twice on the way, and both corrections are recorded rather than quietly fixed. First the wrong field was checked, then the network was called all-UniFi when it has a Netgear switch on port 7 that LLDP cannot see because it does not advertise. ExporterConfig's generated repr contains the API key. Defensive rather than a demonstrated leak, and said so. Also records why a [[merge]] override for multi-interface hosts is not planned: a merged node belongs to every network its interfaces are on, Node.network holds one value, and --per-network already renders each interface alone in its own VLAN's diagram. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dates the changelog section, bumps the version and regenerates the man page, in that order: the man page takes its date from the changelog entry for the version, so bumping before dating generates an empty date that the regenerate-and-compare check cannot catch. Also updates the version in the JSON export example in docs/output.md, which a test compares against real output. `make demo-images` was deliberately not run. The committed screenshots were generated by Graphviz 15.1.0 and the only dot available here is 2.43.0, so regenerating would have rewritten all eight PNGs and both HTML viewers with older output: different sizes, different text attributes, stroke="transparent" instead of stroke="none". That is a downgrade rather than a refresh, and nothing in this release changes how a map is drawn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A malformed config file produced a traceback. Parsing reads the config file, so invalid TOML, an unreadable file or an unknown key raised ConfigError from inside parse_args, which ran before main's try block. That affected every command, including shape, which exists specifically to be safe to paste into a bug report. Logging is now configured before parsing, reading -v straight from argv since no namespace exists yet, and parsing is wrapped so these produce the ordinary message and exit 2. formats = [] in a config file silently produced nothing. -f is nargs="+" so argparse refuses an empty list on the command line, but _validate_injected iterated the list and so checked nothing when it was empty. The run did the full topology, override and artwork work, printed "Full map:" with nothing under it, wrote no files and exited 0. An empty list setting is now refused, naming its source, restoring parity with the command line. Also corrects SECURITY.md, which claimed that resolving Graphviz to an absolute path guards against "the file at that path being swapped". It does not: anyone who can write to that path can replace the binary and the absolute path will run the replacement. What it actually buys is that there is no second PATH lookup, so a PATH change after startup cannot redirect the call. The narrower claim is now stated, along with what it does not cover. Regression tests go through main() rather than the loader, which was already tested and was never where the defect was. Both mutation-tested by reverting each fix and confirming they fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A frozen dataclass reprs every field, so repr(CONFIG) rendered the live key.
Declaring api_key with field(repr=False) hides it while leaving host, site
and verify_tls visible, because a redacted repr nobody can debug with would
be its own problem, and leaves the value readable on the object.
Defensive rather than a fix for a demonstrated leak, and the changelog says
so. Nothing reprs, formats or logs the config today; -v does not enable
http.client debug, urllib3 at DEBUG logs the request line rather than
headers, and an ordinary traceback does not print frame locals. What this
closes is the ways a repr escapes later: a pytest assertion diff, a
--showlocals traceback pasted into an issue, or a well-meant
log.debug("config: %r", config).
Included in 0.12.0 rather than deferred. It was first held back because the
release had already grown, which does not survive scrutiny: the change is
two lines, was fully specified in the ticket, and RELEASING.md says security
fixes should not sit unreleased because anyone tracking tags is running the
last one.
Four tests, mutation-tested against removing repr=False. One covers f"{config}"
separately from repr(config): a dataclass aliases __str__ to __repr__, and
interpolation is the likelier accident of the two.
Co-Authored-By: Claude Opus 5 <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
2 participants
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.
Cuts 0.12.0.
Added
A config file and
UNIFI_MAP_*environment variables for preferences.~/.config/unifi-map/config.toml(honouringXDG_CONFIG_HOME, or$UNIFI_MAP_CONFIG) plus a matching variable fortheme,layout,icons,formats,overridesand the three directories.Both mechanisms rather than one. A file suits somebody whose taste differs from
the defaults; environment variables are what make the tool configurable in a
container without mounting anything. Precedence is flag, then environment, then
config file, then default, with environment above file so a config baked into
an image can be overridden per deployment.
--obfuscateand--forceare excluded on purpose: one asserts the output issafe to share and the other overwrites files, and neither should be answerable
by ambient state invisible at the call site.
Every render now reports which settings it did not get from the command line
and where each came from.
Deprecated
UNIFI_CACHE_DIR,UNIFI_ASSET_CACHEandUNIFI_OUT_DIRare renamed to theUNIFI_MAP_*forms.UNIFI_*is the console's namespace and these were neverconsole settings. The old names still work and warn.
Fixed
Three defects, all found by external review of this branch before merge:
Configuration error: ...and exit 2, for every command includingshape.formats = []in a config file passed validation, did the full topology andartwork work, printed
Full map:with nothing under it, wrote no files andexited 0.
SECURITY.mdclaimed that resolving Graphviz to an absolute path guardsagainst the file at that path being swapped. It does not; it only removes the
second
PATHlookup.Plus the API key no longer appears in
ExporterConfig's repr (KAN-198),defensive rather than a fix for a demonstrated leak.
Notes
make demo-imageswas deliberately not run: the committed screenshots weregenerated by Graphviz 15.1.0 and only 2.43.0 is available here, so regenerating
would downgrade all eight PNGs and both HTML viewers. Nothing in this release
changes how a map is drawn.
make checkpasses at 721 tests. Every new guard was mutation-tested.