Skip to content

Minor audit findings (omnibus): docstring overclaims, broad fallback, pickle codec, import-time side effects #16

Description

@thorwhalen

Summary

Companion to #13 / #14 / #15 — collecting the lower-priority items from the same audit so they're not lost. Each is minor in isolation. Cherry-pick whichever feel worthwhile; close the rest.


1. EnvironmentVariables docstring overpromises

config2py/util.py:55-64 — class docstring says "wraps environment variables without revealing sensitive information." The __repr__ override at line 64 does redact, but values are still exposed by:

  • dict(envvar), envvar.maps[0], envvar.items(), envvar.values()
  • pickle.dumps(envvar)
  • structured loggers that walk the mapping

The protection covers only the repr / print / f-string path. Either tighten the class (override __iter__ / items / values / __reduce__ to redact) or soften the docstring to "hides values from repr only, not from iteration or serialization."


2. config_not_found_exceptions=(Exception,) swallows everything by default

config2py/base.py:107 and :283 — default is (Exception,), which catches and silently moves to the next (less-trusted) source on any error. Network blips, programming bugs, missing imports — all indistinguishable from "not found."

Suggest narrowing the default to (KeyError, LookupError, FileNotFoundError) and keeping the broad catch as opt-in. Stricter default surfaces real bugs early; opt-in preserves the current ergonomics for callers who genuinely want it.


3. pickle.loads as the default .pkl / .pickle decoder

config2py/codecs.py:350-354 registers pickle.loads as the default decoder. If a user calls decode_by_extension('something.pkl', untrusted_bytes) (e.g. fetched from a remote store), they get arbitrary code execution.

At minimum, document the deserialization risk in the registration's docstring. Stronger fix: require explicit opt-in (register_codec(".pkl", ..., trusted=True) or similar), or only register the encoder (writing pickle is safe; reading untrusted pickle isn't).


4. Filesystem side effects at import time

config2py/tools.py:107 and :122 — import config2py triggers simple_config_getter() and get_configs_local_store(), which create ~/.config/config2py/ and ~/.config/config2py/configs/ and write marker files.

Side effects on import are surprising (containers, read-only filesystems, sandboxed test runners) and slow import config2py for callers who only need a small slice of the API. Move to lazy cached_property / module-level __getattr__.


5. Silent fallback in ConfigReader source-kind detection

config2py/s_configparser.py:227-247 — kind detection is a chain of isinstance checks ending in:

else:
    self.read(source)
    source_kind = "unknown"

configparser.read() silently returns an empty list when given a path that doesn't exist, so a wrong/typo'd source produces an empty config rather than an error. Suggest raise TypeError(f"Unsupported source: {type(source).__name__}") in the else branch.


6. extract_exports silently returns {} for non-existent paths

config2py/tools.py:140 — if the argument has no newline and isn't an existing file, falls through to "no exports." A typo'd path (extract_exports("path/to/.env") where the file doesn't exist) returns {} rather than raising. Same root cause as #5: detect-and-fall-through where detect-and-raise would catch user error earlier.


7. get_configs_local_store OS-inconsistent path detection

config2py/tools.py:31 — if os.path.sep in config_src and os.path.isdir(config_src): decides "treat as path vs. app name" by sniffing os.path.sep. On Windows, simple_config_getter("my.app") and simple_config_getter("my\\app") will be classified differently than on POSIX. Use os.path.isabs(config_src) or accept an explicit kind= argument.


8. Dead commented-out code

config2py/base.py:82-90 — commented block referencing OPENAI_API_KEY and getpass.getpass. Suggests a pattern the package no longer implements; safe to delete.


Why one ticket

These are all small, none warrant maintainer scheduling on its own, and bundling lets you triage in one pass. Happy to split any item into its own ticket if you'd rather track it that way. Top-3 items (mask-input default, KeyError value leakage, file umask) are filed separately as #13 / #14 / #15.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions