bug: a request name containing `-f` silently arms `--force` and overwrites an existing saved request

Open Beginner friendly
#52 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
91/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
python
Domain
cli

Research direction

Start with the save argument handling at cli.py:466-477 and the collision-guard tests in tests/test_save_request.py; run the existing tests first to understand how saves and force are exercised. Add a regression test for save my -f report that checks the intended filename and confirms the existing my_report.json is unchanged with no overwrite warning. Done when the regression test fails before the fix and passes afterward, along with the relevant test suite.

Written by the indexing model from the issue text.

Description

bug

Summary

The collision guard added in 0.1.0 to stop save silently destroying a request
is bypassed by the name itself. save strips -f/--force from anywhere in
the argument list, so a request whose name legitimately contains -f is saved
with force=True — and an existing request at the same sanitized path is
overwritten with no warning.

The guard's own message tells the user the overwrite is safe because they asked
for it:

    Use 'save {name} --force' to replace it, or pick another name.

They did not ask for it. save my -f report means "save this under the name
my -f report" to anyone reading the REPL transcript; internally it means
"save it as my report, replacing whatever is there, and say nothing."

Confirmed data loss: a request saved under my report is silently destroyed by
save my -f report, with 💾 Saved to ... printed as if nothing unusual
happened. Exit code 0, no warning, no ⚠️.

Steps to reproduce

cd /tmp && rm -rf gapi-demo && mkdir gapi-demo && cd gapi-demo
git-api

Inside the REPL:

var base_url=http://127.0.0.1:9
GET {{base_url}}/original
save my report
GET {{base_url}}/impostor
save my -f report
list

Actual vs Expected

Actual — verbatim REPL stdout:

default>
→  GET http://127.0.0.1:9/http://127.0.0.1:9/original
←  0
[Errno 111] Connection refused
default>
💾  Saved to .git-api/requests/my_report.json
default>
→  GET http://127.0.0.1:9/http://127.0.0.1:9/impostor
←  0
[Errno 111] Connection refused
default>
💾  Saved to .git-api/requests/my_report.json
default>   my_report: GET http://127.0.0.1:9/http://127.0.0.1:9/impostor → 0
default> Bye.

Exit code 0. The ⚠️ A saved request named ... already exists warning that
save_request() is written to print never appears. my_report now holds
impostor; the original request is gone from the directory.

Contrast with the same sequence and no -f in the name, where the guard fires
correctly:

default>
💾  Saved to .git-api/requests/my_report.json
default>   my_report: GET http://127.0.0.1:9/http://127.0.0.1:9/other → 0

(save my report twice is an update under the same name, which is documented
and correct. The bug is specific to -f/--force appearing inside a
different name.)

Expected — save my -f report saves under the filename for my -f report
(my_-f_report.json), leaving my_report.json untouched. If the user did mean
to overwrite, they typed save my report --force — flag last, where every other
CLI in the world puts it.

Root cause

cli.py:466-472:

force = False
args = [p for p in parts[1:] if p not in ("--force", "-f")]
if len(args) != len(parts) - 1:
    force = True
    if not args:
        print("Usage: save NAME [--force]")
        continue

The filter is position-independent: it removes every token equal to -f or
--force and treats the presence of any such token as "the user asked for
force". args is then rejoined at cli.py:477 (name = " ".join(args)), so the
-f is not merely misread as a flag — it is deleted from the name. The two
mistakes compound: the user gets a different filename and force semantics from
one keystroke.

This matters because safe_name() maps - to itself (cli.py:162 keeps
alnum, - and _), so my -f report and my report sanitize to
my_-f_report.json and my_report.json — different files. The overwrite is
purely the spurious force=True, not a sanitized collision.

The --force-only invocation is handled (if not args: print("Usage...")), so
the author thought about the empty case. What was missed is that args can be
non-empty and still have had a flag removed from the middle.

Suggested fix

Only treat a trailing token as the flag. help already documents the flag
after the name (save NAME [--force]), so the positional form is the documented
one:

force = False
args = list(parts[1:])
if args and args[-1] in ("--force", "-f"):
    force = True
    args.pop()
if not args:
    print("Usage: save NAME [--force]")
    continue
name = " ".join(args)

This deletes one if and one len() comparison, and it makes -f inside a
name behave like every other character. The if not args check now also covers
the bare save --force case that the current if len(args) != len(parts) - 1
guard handles only incidentally.

If leading-flag tolerance is wanted as well (save --force NAME), match the
flag explicitly at position 0 instead of filtering — but position-only matching
at the end is the smaller and less surprising fix.

Regression test

tests/test_save_request.py is the natural home; it already covers the
collision guard. The case that matters is the negative one:

def test_name_containing_force_flag_is_not_treated_as_the_flag(tmp_path, monkeypatch):
    """A name that happens to contain -f must not arm overwrite or lose the -f."""
    monkeypatch.chdir(tmp_path)
    monkeypatch.setattr(cli, "last_request", (...), raising=False)
    # drive `save my -f report` through main()'s argument handling
    ...
    # assert: saved file is my_-f_report.json AND my_report.json is untouched
    # assert: the collision warning was NOT printed

Assert on the absence of the overwrite, not just the presence of a file. A
test that only checks my_-f_report.json exists would pass today if the
overwrite also happened — the data loss is the part that needs pinning.

Environment

git-api-py 0.1.0, commit 1a2e49b
python 3.14.7
repro: repro/probe_round4.py (cases save_force_leak, save_force_leak2)
Dominant language
Python
Stars
0
Forks
1
PR merge metrics
No merged PRs in 30d

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.