Skip to content

fix(miner): hooks check --tool/--input don't guard against consuming another flag as their value #5833

Description

@JSONbored

Context

packages/loopover-miner/lib/deny-check.js's parseDenyCheckArgs parses loopover-miner hooks check --tool <name> --input <json> [--json]. Its --tool/--name and --input value-consuming branches take the next token unconditionally:

if (token === "--tool" || token === "--name") {
  const tool = args[++index];
  if (!tool) return { error: "Missing value for --tool." };
  options.tool = tool;
  continue;
}
if (token === "--input") {
  const parsed = parseToolInput(args[++index]);
  if ("error" in parsed) return { error: parsed.error };
  options.input = parsed.value;
  continue;
}

Neither branch checks whether the consumed token itself looks like another flag (startsWith("-")). Every sibling CLI flag parser in this package already guards this: attempt-cli.js (if (!value || value.startsWith("-")) return { error: ATTEMPT_USAGE };, used for both its value-taking flags), and claim-ledger-cli.js (four separate flag parsers, each with the same !value || value.startsWith("-") guard for --note, --api-base-url, --repo, --status).

Without the guard, loopover-miner hooks check --tool --input '{}' silently consumes --input as the tool name (options.tool = "--input"), then the loop's next iteration sees the literal token '{}', which doesn't match any known flag and doesn't start with -, so it falls through to the generic return { error: DENY_CHECK_USAGE }; — a plain "here's the usage string" message instead of the same specific, actionable "Missing value for --tool"-style error the sibling parsers give for the exact same class of mistake (a flag consumed as another flag's value).

Requirements

  • Add the same !value || value.startsWith("-") guard (matching the exact convention already used in attempt-cli.js and claim-ledger-cli.js) to both the --tool/--name and --input value-consuming branches in parseDenyCheckArgs.
  • The rejected case should produce the same specific, actionable error style already used elsewhere in this file (Missing value for --tool. / an equivalent for --input), not fall through to the generic usage string.

Deliverables

  • --tool/--name branch in parseDenyCheckArgs rejects a value that starts with -.
  • --input branch in parseDenyCheckArgs rejects a value that starts with - before attempting to JSON-parse it.
  • Regression tests in the deny-check test file (search test/unit/ for the existing deny-check/hooks check test coverage) covering hooks check --tool --input '{}' and hooks check --tool foo --input --json, each asserting the specific "missing value" error rather than the generic usage fallback.

Test Coverage Requirements

packages/loopover-miner/lib/** is in Codecov's coverage.include; target 99%+ patch coverage on both new guard branches (triggered and not-triggered) plus the regression tests above.

Expected Outcome

hooks check's --tool/--input flags reject an adjacent flag being consumed as their value with the same specific, actionable error message every other flag-value parser in this package already gives for the identical mistake — no more falling through to a generic usage string that doesn't say what actually went wrong.

Links & Resources

  • packages/loopover-miner/lib/deny-check.js (parseDenyCheckArgs)
  • packages/loopover-miner/lib/attempt-cli.js (the !value || value.startsWith("-") guard — precedent)
  • packages/loopover-miner/lib/claim-ledger-cli.js (four more instances of the same guard — precedent)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions