DEV Community

TiltedLunar123
TiltedLunar123

Posted on

the timeout flag on my windows auditor was wired to nothing

winrecon is a windows auditing script i wrote. it runs 20 checks on a box, scores it 0 to 100, and writes one self-contained html report plus a json export. standard library only, no pip, no internet, so it runs on locked-down machines where you can't install anything. that constraint is the whole point. the boxes most worth auditing are usually the ones where you can't add a single dependency.

most of those checks work by shelling out to a built-in windows command and reading what comes back. netstat for listening ports. net user for accounts. sc query for services. wmic and a few powershell one-liners for the rest. every one of those goes through a single helper, run_command, that wraps subprocess and kills the command if it runs too long.

here's the helper, roughly:

def run_command(cmd, timeout=60):
    try:
        result = subprocess.run(
            cmd, capture_output=True, text=True,
            timeout=timeout, shell=False
        )
        return result.stdout
    except subprocess.TimeoutExpired:
        log.warning("command timed out: %s", cmd)
        return ""
Enter fullscreen mode Exit fullscreen mode

fine. and i had a cli flag for it, because some boxes are slow and 60 seconds isn't always enough:

--timeout, -t   Timeout in seconds for each system command (default: 60)
Enter fullscreen mode Exit fullscreen mode

it showed up in --help. argparse parsed it. you could run winrecon --timeout 120 and it took the number without complaining.

it just never used it.

i found this the boring way. i was running the tool on an old file server that had a wmic query hang for what felt like forever, and i thought, fine, i'll drop the timeout so it fails faster and i can move on. set --timeout 10. same hang. set --timeout 5. same hang. it sat there for a full minute every time no matter what i passed.

so i went and looked. the value was getting parsed into args.timeout and then nothing happened to it. every call site was just run_command(cmd). no second argument. the helper fell back to its default of 60 on every single check, and the flag i'd written and documented was decoration.

the reason it survived so long is that the default was reasonable. 60 seconds is fine for almost every command on almost every box, so nobody, me included, ever noticed the number wasn't the number they asked for. the flag wasn't broken in a way that threw an error or printed something wrong. it was broken in the quieter way, where everything looks like it's working and the option you set just gets ignored.

the fix is not interesting. thread the parsed value down instead of leaning on the default:

findings = run_all_checks(timeout=args.timeout)
# ...and every run_command(cmd) inside becomes
run_command(cmd, timeout=timeout)
Enter fullscreen mode Exit fullscreen mode

what's a little more interesting is why the tests didn't catch it. i had tests for run_command. they passed a timeout in and checked it got honored, and they passed, because run_command itself was correct the whole time. what i didn't have was a test that the cli flag actually reached run_command. the unit was right. the wiring between the flag and the unit was the bug, and unit tests by design don't look at the wiring between two things they each test in isolation.

so i added one. it's small and kind of dumb and it's the test i trust most in the project now:

def test_timeout_flag_reaches_commands(monkeypatch):
    seen = {}
    def fake_run(cmd, timeout=60):
        seen["timeout"] = timeout
        return ""
    monkeypatch.setattr("winrecon.core.run_command", fake_run)
    main(["--timeout", "7"])
    assert seen["timeout"] == 7
Enter fullscreen mode Exit fullscreen mode

if the plumbing ever comes loose again, that goes red immediately instead of three releases later.

what's still not great: the timeout is per command and applied globally, so a genuinely wedged command still costs you the whole 7 or 60 seconds before the scan moves on. and there are 20 checks, some firing more than one command, so a bad box can still drag. a smarter version would run the checks concurrently and put a single budget on the whole scan instead of on each call, so one stuck command couldn't dominate the total. the standard library hands me concurrent.futures for exactly that, so it's doable. i just haven't done it yet, and i'd rather ship the honest per-command version than a half-finished concurrent one.

mostly this was a reminder that a flag existing in --help is not evidence it does anything. if your tool takes an option, write the one ugly test that proves the option changes behavior, not just that the function underneath it can. the gap between "the feature works" and "the feature is reachable" is where this kind of thing lives, and it's invisible until someone actually leans on the flag on a bad day.

repo's here if you want to poke at it: https://github.com/TiltedLunar123/WinRecon

Top comments (0)