m6 command line interface

M6: Command-line interface

Status: Delivered. Tracked by issue #8 and sub-issues #53 through #57 plus #105, the rules-hardening sub-issue this review added. All six shipped as PRs #107 through #112 on a stacked branch chain and are merged; syncwingetlink.exe links for the first time as of #56.

Two later reviews of this page corrected it against the merged code:

• the delivery sequence's fourth item now names #105 and its actual branch, rather than the placeholder it was written with before the issue existed; and
• the "Documentation carried by these PRs" section's ADR list was wrong — M6 produced six ADRs (ADR-0020 through ADR-0025), not the two this page originally predicted, and the subject matter it attributed to ADR-0020 and ADR-0021 does not match what docs/adr-phase-5.md actually records. Corrected below.

One divergence found after M6 merged is not M6's to fix and is tracked separately: --tui, --verbose, and --quiet are parsed into AppOptions but never read by any consumer, so they are silently ignored at runtime while --help and README.md present them as working. See issue #113 and the M8 plan page.

Scope

M6 is the first code that accepts input from outside the process — argv, a user-supplied rules.json, and file names harvested from the filesystem by M2's package sources — and the first code that writes to a terminal. It is also the only milestone that owns main.cpp, so process-wide COM apartment lifetime and the final exit code both land here.

M6 covers:

• parsing scan/fix/test-rule and every documented option into AppOptions;
• console output (Unicode, color, confirmation prompts, --yes);
• --json output for scripting;
• dispatching to core/ and mapping every failure to a documented exit code; and
• --help/--version.

M6 does not implement the interactive TUI checklist or progress display (M7).

This page originally reserved test-rule's file-name → matched-rule → alias presentation for M3's #40, with #53 parsing the name, #56 dispatching, and #40 owning the output. That boundary did not survive delivery: #56's runTestRule() (src/cli/Dispatch.cpp) prints the full file-name → matched-rule-name → alias line itself, which is exactly what #40 was asking for. #40 was reviewed against the merged code and closed as already implemented; docs/TODO.md M3's checkbox is corrected by M8's #64.

Security contract

M6 is the milestone where untrusted data first reaches a human or a script. Every rule below must hold regardless of which sub-issue implements it.

Rule	Requirement
Escape/control sanitization	Package ids, executable file names, and alias names are attacker-influenceable — anyone who can place a file under Packages chooses its name. Before printing any such string (console or JSON), strip or escape C0 control characters, ESC/CSI sequences, and bidi override characters (U+202A–U+202E, U+2066–U+2069). This applies whether or not VT processing is enabled, since M7 turns it on globally.
--json stream purity	When --json is set, stdout carries the JSON document and nothing else — no diagnostics, warnings, or prompts. Anything else goes to stderr. Escaping is strict JSON string escaping; the surrogate policy is explicit and documented (see below), not left to whatever WideCharToMultiByte happens to do.
Non-interactive stdin is not consent	fix without --yes prompts for confirmation. EOF on a closed/redirected stdin, or a bare Enter, must never be treated as "yes" — both must produce a refusal, not silent execution. Corrected during #56/#57's review: refusal here is not itself an error and does not map to exit code 3 — a declined/EOF'd candidate is simply run in dry-run mode instead (reusing WouldCreate/WouldReplaceBroken), contributing to exit 0/10 like any other outcome. The only exit-3 case involving consent is a parse-time one: --json combined with fix and no --yes (ArgParseErrorKind::ConflictingOptions), which is rejected before any prompting could happen at all.
Untrusted rules-file input is bounded	A rules file loaded via --rules or the user rules path is untrusted. loadRuleSetFromFile() must cap the file's byte size before reading it whole; RuleSet(std::vector<AliasRule>) must cap rule count and per-field (name/pattern/replacement) length. RuleSet::resolve()'s call to std::regex_match must be wrapped in try/catch for std::regex_error — MSVC's std::regex can throw error_complexity/error_stack at match time, not only at compile time, so today only the compile path (RuleSet.cpp:176) is guarded and the match path (RuleSet.cpp:290/297) is not. All of these funnel into exit code 3, same as a parse error. This is a self-contained change against rules/, tracked as its own sub-issue rather than folded into #53.
Path overrides are validated, not trusted	--links-dir, --packages-dir, and --rules are rejected if empty or if they resolve to a \\.\ device path, and are otherwise made absolute (std::filesystem::absolute + lexically_normal) before being stored. Correction from this review's first pass: they are not required to already exist. An absent Packages directory is a normal, tolerated state — FsScanSource reports it as zero packages found, not a failure (ADR-0010) — and an absent Links directory is exactly the condition fix exists to correct; rejecting either at parse time would contradict established behavior elsewhere in the codebase. --rules existence/readability stays RuleSetSelector's job (ADR-0013 already distinguishes absent-falls-through from malformed-is-fatal); ArgParser does not duplicate that check. The resolved effective paths are still echoed before any mutating operation runs. Paths::getLinksDirectory/getPackagesDirectory remain the only source of the default path via SHGetKnownFolderPath(FOLDERID_LocalAppData) — never an environment-variable read of %LOCALAPPDATA%, which can be spoofed by the calling environment. See ADR-0020.
No self-elevation	The manifest stays asInvoker (src/app.manifest). On the InsufficientPermission exit path, M6 prints guidance — Developer Mode first, elevation second — and never calls ShellExecuteW/runas itself. An elevated re-launch would resolve a different user's %LOCALAPPDATA% under "run as different user", silently changing which Links/Packages directory is in play.
Collisions never reach fix	The M5 Wiki page states plainly that M5's SymlinkService does not pick a winner among colliding aliases and that M6/M7 must remove colliding candidates from the repair set before calling repairLink(). --yes must not bypass this — a collision is reported, never auto-resolved.
test-rule NAME takes a bare file name	#53 validates NAME with the existing isValidAliasFileName() (src/rules/RuleSet.h) before treating it as one — no path separators, no drive letters. Its echo goes through the same sanitizer as any other file name.
A top-level exception is caught and mapped	An exception escaping wmain produces an undocumented crash exit code (0xC0000409 under /GS). wmain wraps its body in try/catch, mapping every known exception type through the exit-code table below and anything else to a generic non-zero failure — never letting the process terminate via an unhandled exception.
Entry-point DLL search hardening	wmain's first statement is SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32), and the executable project sets /DEPENDENTLOADFLAG:0x800. Most current imports (kernel32, combase, shell32) are KnownDLLs already, so the practical exposure this closes is small — this is inexpensive defense-in-depth ahead of the M8 loose-exe release, not a fix for a demonstrated vulnerability, and the acceptance criteria should not overclaim it.

Win32 contract

Rule	Requirement
Entry point	wmain(int argc, wchar_t* argv[]), kept as a thin shim over a core run() that takes wide arguments — main.cpp cannot grow real logic because the MSTest DLL cannot link an executable's object files (ADR-0002), and re-deriving wide argv from GetCommandLineW()/CommandLineToArgvW inside a narrow main would only reintroduce quoting bugs wmain avoids for free.
Console output selection	Probe with GetConsoleMode on the output handle. A real console gets WriteConsoleW; a redirected handle gets UTF-8 bytes via WriteFile. Never set SetConsoleOutputCP(CP_UTF8) process-wide without an RAII restore of the previous code page — it otherwise outlives the process in an interactive parent console. Never mix _setmode(_O_U16TEXT) with narrow-byte writes on the same stream. Chunk very large writes, since WriteConsoleW can fail with ERROR_NOT_ENOUGH_MEMORY on oversized buffers.
Handle validation	GetStdHandle can return NULL (no handle) or INVALID_HANDLE_VALUE (closed/detached descriptor). Both must be handled as "not a usable stream" rather than passed straight to a Win32 I/O call.
Color as a probed capability	Color/VT support is established by attempting SetConsoleMode(handle, mode | ENABLE_VIRTUAL_TERMINAL_PROCESSING) and checking the result — never assumed. The original mode is restored via RAII on every exit path. Redirected output disables color outright; NO_COLOR (any value) and a new --no-color flag both force it off regardless of TTY state. M6 owns detection/restore; the full VT-driven UI is M7's.
Prompt input	Confirmation prompts read via ReadConsoleW on a real console handle, with ENABLE_LINE_INPUT | ENABLE_ECHO_INPUT set and the original mode restored afterward; a redirected stdin is read as bytes and UTF-8-decoded. This is also where the "EOF/bare-Enter is not consent" rule from the security contract is enforced.
Ctrl+C handling	SetConsoleCtrlHandler restores the console mode, stops the batch between items (not mid-item), and yields a defined exit code. A CreateSymbolicLinkW call already in flight is not cancellable — the guidance text must not imply otherwise.
Decode existing error messages as UTF-8	PackageSourceError, RuleSetError, and SymlinkServiceError messages are built as UTF-8 std::string (via WideCharToMultiByte(CP_UTF8, …) in the core). Console must decode them with CP_UTF8, not the process ACP, or non-ASCII paths in error text get mangled.
Relative-path overrides are stored absolute	Paths::toExtendedLengthPath() already makes a relative path absolute itself before prefixing it (it does not blindly prefix a relative path — corrected from this review's first pass, which mischaracterized the existing implementation). ArgParser still normalizes --links-dir/--packages-dir/--rules to an absolute path at parse time, but for its own reason: a stable, displayable value in AppOptions, independent of whatever the process's current directory happens to be by the time it is echoed or used. Use fromExtendedLengthPath() (src/core/Paths.h) when displaying a path that has already gone through toExtendedLengthPath() back to the user.
Single process-wide COM apartment	main.cpp constructs exactly one core::ComApartment for the process lifetime, before any other core call — RuleSet::parse() needs an initialized apartment even for --source fs and for test-rule (ADR-0011: it uses winrt::Windows::Data::Json). WingetComSource currently owns its own apartment only because main.cpp does not exist yet; ADR-0009 explicitly asks M6 to move that ownership here. Not currently assigned to any sub-issue — this review assigns it to #56.
Total exit-code mapping	The dispatch layer's exit-code function must be exhaustive over PackageSourceErrorKind, RuleSetErrorKind, and SymlinkServiceErrorKind — today PackageSourceErrorKind has no mapping destination at all. See the table below.

Exit-code map

0/1/2/3/10 are the codes already documented in docs/PLAN.md §8 and AGENTS.md §6. This review adds 4, needed because an explicit --source com failure (server missing, policy-blocked) previously had nowhere to map — 3 means "your arguments/config are wrong," which a data-source outage is not, and 10 means "some repairs failed," which does not apply to a scan that never got to run. scan can therefore return 0, 3, or 4, but never 10.

Code	Meaning	Triggers
0	Success	Nothing to fix, or fix fully succeeded
1	Fix needed but not performed	scan --fail-on-missing found a Missing/Broken/Mismatch candidate
2	Insufficient permission	SymlinkServiceErrorKind::InsufficientPermission
3	Argument/config error	Unknown/malformed CLI option; any RuleSetErrorKind (including the new match-time regex_error capture); invalid test-rule name; a path override that fails validation; --json combined with fix and no --yes (a parse-time conflict — a declined/EOF'd confirmation during a real fix run is normal refusal, not this code; see the security contract's correction above); an unexpected LinkInspectionError (a link-inspection failure has no PackageSourceErrorKind of its own, so it falls into this generic bucket rather than 4)
4	Package enumeration failed	Any PackageSourceErrorKind surfaced by an explicit --source com or --source fs (④auto degrading to FS on a PackageSourceError is not itself a failure — see ADR-0010 — so auto only reaches 4 if FS then also fails)
10	Some repairs failed	One or more SymlinkServiceErrorKind values other than InsufficientPermission (DeleteFailed/CreateFailed/VerificationFailed) during a batch that otherwise made progress

Ctrl+C during fix exits non-zero (reuse 10, since a partial batch is exactly what that code already means) after restoring console mode and finishing the in-flight item.

Guidance text for the four (elevation, developerMode) combinations that produce exit code 2 (ADR-0019 parks these here for M6 to render):

• non-elevated + Developer Mode disabled → suggest enabling Developer Mode, or running elevated;
• non-elevated + Developer Mode unknown → suggest checking Developer Mode, or running elevated;
• elevated + privilege failure → state that the elevated token or local policy still lacks symlink privilege;
• access denied → suggest checking write/delete access to the Links directory.

None of these guidance paths self-elevate (security contract, above).

Delivery sequence

1. #53 — Parse commands and options. cli/ArgParser: scan/fix/test-rule and every option in docs/PLAN.md §8 plus --fail-on-missing and the new --no-color (both missing from §8 today — corrected as part of this issue). Validates path overrides (security contract) and the test-rule name. Unknown option, missing required value, and --json + fix without --yes all fail parsing with exit code 3. Supports a -- terminator. Never reads %LOCALAPPDATA% directly. Branch: feature/53-parse-commands-and-options.
2. #54 — Implement console interaction. cli/Console: the Win32 contract's output selection, handle validation, color probing/restore, prompt input, and the sanitizer required by the security contract. --yes bypasses the prompt but not the collision check. Branch: feature/54-console-interaction.
3. #55 — Emit machine-readable JSON. Stdout-only JSON, strict escaping, a documented surrogate policy, no BOM, a stable schema recorded in docs/PLAN.md. Diagnostics move to stderr when --json is set. Branch: feature/55-json-output.
4. #105 — Harden untrusted rules-file input. The rules-file caps and the RuleSet::resolve() try/catch from the security contract. Lives in rules/ rather than cli/, so it is its own reviewable change; #56 depends on its exit-code behavior. Branch: feature/105-harden-rules-input.
5. #56 — Dispatch commands and exit codes. main.cpp: single ComApartment construction, wmain with the top-level exception guard, SetDefaultDllDirectories, the total exit-code map (including code 4), the no-self-elevation guidance text, and the collision-exclusion gate before any call into SymlinkService. Branch: feature/56-dispatch-and-exit-codes.
6. #57 — Provide help and version output. --help to stdout with exit 0 and the exit-code table reproduced in the help text; --version to stdout with exit 0 from one source of truth; any other parse failure prints usage to stderr with exit 3. No network access, no ShellExecute. Branch: feature/57-help-and-version.

#53 is the foundation every other sub-issue's AppOptions depends on. #54, #55, and the #105 may proceed independently after #53. #56 depends on all three. #57 can proceed in parallel with #54/#55 once #53 lands, since help/version do not touch AppOptions parsing beyond recognizing the flags.

Test plan

• Sanitizer — escape sequences, C0 controls, and bidi override characters embedded in a package id/file name/alias are neutralized in both console and JSON output.
• JSON escaping — quote, backslash, control characters, non-BMP characters, and an unpaired surrogate all produce the documented, stable output rather than silent substitution.
• Exit-code totality — a table-driven test asserts every PackageSourceErrorKind, RuleSetErrorKind, and SymlinkServiceErrorKind enumerator maps to exactly one documented code.
• Non-interactive consent — closed/redirected stdin and a bare Enter both refuse fix without --yes (exit code 3), rather than proceeding.
• Path-override validation — an empty value and a \\.\ device path are rejected; a relative path is accepted and normalized to absolute; a path that does not (yet) exist is accepted rather than rejected, matching ADR-0010's tolerance for an absent Packages/Links directory.
• Rules-input bounds — an oversized rules file, an oversized rule count, and a pathological pattern that throws at match time all resolve to exit code 3 instead of an uncaught exception.
• Collision exclusion — a repair set containing a collision never reaches SymlinkService::repairLink(), with and without --yes.
• test-rule name validation — a path-separator or drive-letter-bearing argument is rejected before runTestRule()'s presentation logic ever runs.

Each pull request builds core, cli, and tests at warnings-as-errors in Debug|Release times x64|ARM64. Debug and Release x64 tests run through vstest.console.exe and report the actual result. ARM64 is reported as cross-built, not run, unless execution occurs on an ARM64 host. No third-party dependency is introduced.

Documentation carried by these PRs

• docs/PLAN.md §8: add --fail-on-missing and --no-color to the option table, add exit code 4 to the exit-code table.
• docs/TODO.md M6: check off items as their sub-issue lands, each pointing at the PR that completed it (matching the M4/M5 closing style).
• AGENTS.md §6: add exit code 4 to the table there too.
• README.md: update the CLI usage/exit-code sections to match.
• docs/adr-phase-5.md (new). This page originally predicted two ADRs and misdescribed both; M6 actually produced six, and this is what they record:
  • ADR-0020 — the CLI argument model, and the path-override validation scope narrowed from this review's first draft (the "validated, not required to exist" correction in the security contract above).
  • ADR-0021 — Console: the operations seam, the sanitization scope, and why input mode is restored per call rather than process-wide. Also the UTF-8 convention for diagnostic text produced by the core.
  • ADR-0022 — the JSON schema, string-escaping rules, and the explicit surrogate policy, including stdout purity under --json.
  • ADR-0023 — rules-file input bounds: size, rule-count, and field-length caps, plus the match-time regex_error guard (#105).
  • ADR-0024 — main.cpp: the total exit-code map (including the new code 4), the scan/fix pipeline, the ComApartment ownership move out of WingetComSource, the top-level exception guard, and the SetDefaultDllDirectories / /DEPENDENTLOADFLAG:0x800 entry-point hardening.
  • ADR-0025 — a single version constant (cli::kVersion) and help-text polish, including why src/app.manifest's version was left unlinked from it at build time.

Completion criteria

• #53 through #57, and #105, are merged and closed, and issue #8 reflects their completion.
• Every security-contract and Win32-contract rule above is satisfied and covered by a test in the plan above.
• The exit-code map is total and matches PackageSourceError.h, RuleSet.h, and SymlinkService.h exactly.
• docs/PLAN.md, docs/TODO.md, AGENTS.md, README.md, docs/adr-phase-5.md, and this Wiki page agree.
• Canonical documentation and comments are English. Localized *_ja.md files are not read or changed.