locationLogs misses the log of a key that needs escaping

Open Beginner friendly
  • Is something wrong?
#1,919 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
77/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
git, typescript
Domain
backend, cli

Research direction

Start with locationLogs in scripts/scrub/git/git-lib.ts and the annexKeyFile escaping used by readLocationLogsAt; then read the URL key case in test/scrub/git/location-logs-at.test.ts and the dead-key test in test/import-scrub.test.ts. Make locationLogs find logs for keys containing &, %, : or /, and confirm the dead URL key test passes.

Written by the indexing model from the issue text.

Description

bug P3

locationLogs(repo, keys) in scripts/scrub/git/git-lib.ts builds each log's path with git annex examinekey --format='${hashdirlower}${key}.log'. That ${key} is the key as written, but git-annex stores a key's logs under its escaped keyFile name: & becomes &a, % becomes &s, : becomes &c and / becomes % (the hash directory is still the md5 of the unescaped key). For a key holding any of those characters the read finds no object and returns "", which every caller reads as "no log": not dead, nobody present.

Measured with git-annex 10.20261006 on a fresh repository after git annex registerurl 'URL--http://example.org/a&b%c' ...:

  • the branch holds bf3/54a/URL--http&c%%example.org%a&ab&sc.log
  • examinekey --format='${hashdirlower}${key}.log\n' prints bf3/54a/URL--http://example.org/a&b%c.log

Who is affected today: restrictManifestToTree in src/lib/import-scrub.ts uses locationLogs for its purge-list read (ADR 0089: a key dead on NEMAR's git-annex branch is never copied). A dead upstream URL key would therefore be copied. The scrub's own callers (annexRegistry, pushedRegistration) only ever see SHA256E keys, whose names need no escaping, so they are not affected. OpenNeuro datasets rarely carry URL keys, so this is low-probability, but the failure direction is the unsafe one.

Phase 5a of epic #1046 (#1911) adds readLocationLogsAt(repo, ref, keys) to the same file, a plain-git read that applies the escaping (annexKeyFile) and is tested against a real URL key in test/scrub/git/location-logs-at.test.ts. The fix here is to build locationLogs' paths the same way (or have it call the shared escaping), plus a case with a URL key in test/import-scrub.test.ts's dead-key test.

  • locationLogs reads the escaped log name for keys containing & % : /.
  • A test with a dead URL key fails on the current code and passes after the fix.
Dominant language
TypeScript
Stars
0
Forks
1
Avg merge
6h 52m
Merged PRs (30d)
224

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.

More from nemarOrg/nemar-cli

All issues in nemarOrg/nemar-cli

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.