Skip to content

🐛 fix(api): reject unknown /api/v1/search params and alias separators - #80

Merged
cwest merged 1 commit into
mainfrom
wt/t_ef642832
Aug 3, 2026
Merged

🐛 fix(api): reject unknown /api/v1/search params and alias separators#80
cwest merged 1 commit into
mainfrom
wt/t_ef642832

Conversation

@cwest

@cwest cwest commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Closes #77.

What

GET /api/v1/search accepted any unrecognized query parameter, returned
HTTP 200, and applied nothing. A caller who misspelled an exclusion filter
got back exactly the content they meant to exclude, with a success status and
no signal — a filter failing open, indistinguishable in the response and
access log from a correct query. The trap is structural: filters take hyphens
(not-path) while scoring params take underscores (half_life), so the wrong
separator is the natural mistake and there is no rule a caller can infer.

This change, confined to internal/apiserver/search.go plus tests:

  1. Rejects unknown query params with 400 (fail closed), with a
    did-you-mean when the key is one edit away from a real one:
    unknown query parameter "notpath"; did you mean "not-path"?
  2. Accepts both separators as aliases for every parameter and merges
    hyphen/underscore variants into the same dimension, so a caller cannot
    guess wrong and repeated aliases OR together.

Value-validation 400s keep their exact wording and now also fire through the
alias spelling with the canonical name in the message. Strictness is
confined to /search; /stats and /graph are untouched. No spec change —
OKF v0.2 §4.1 filters are unchanged in meaning; only the HTTP key surface's
handling changes.

Conformance gate (run, not assumed)

$ gofmt -l .            # (empty)
$ go vet ./...          # clean
$ go test ./... -race
ok  github.com/cwest/okfctl/cmd                 9.137s
ok  github.com/cwest/okfctl/cmd/okfctl-api      2.422s
ok  github.com/cwest/okfctl/cmd/okfctl-search   2.381s
ok  github.com/cwest/okfctl/internal/apiserver  2.216s
ok  github.com/cwest/okfctl/internal/okf        6.511s
ok  github.com/cwest/okfctl/internal/okfconfig  1.777s
ok  github.com/cwest/okfctl/internal/plugin     1.588s
ok  github.com/cwest/okfctl/internal/search     1.891s

$ go test ./internal/okf/ ./cmd/ -run Conformance -race
ok  github.com/cwest/okfctl/internal/okf   1.312s
ok  github.com/cwest/okfctl/cmd            1.528s

Real corpus (the layer fixtures cannot substitute for)

Bundle ~/src/knowledge-base/bundles/knowledge, 262 nodes, hash embedder
(hash-test-embedder), served on a copy. BEFORE = binary at 78ae7c3,
AFTER = this branch. Base query ?q=agent+orchestration+workflow&k=12,
baseline body sha256[0:16] = 2c6cb99cb829cd58.

param BEFORE AFTER verdict
not-path=casey 200 casey=0 5d466ac1 200 casey=0 5d466ac1 control — unchanged
not_path=casey 200 casey=2 2c6cb99c 200 casey=0 5d466ac1 alias now honoured; byte-identical to not-path
notpath=casey 200 casey=2 (ignored) 400 did you mean "not-path"? unknown key + suggestion
exclude_path=casey 200 (ignored) 400 unknown (no suggestion) far from any key
nonsense_param=1 200 (ignored) 400 unknown (no suggestion) far from any key
not_tag=research 200 (ignored) 2c6cb99c 200 40b285b2 alias honoured; == not-tag=research
min-relevance=0.30 200 hits=12 (ignored) 200 hits=5 c3f900bd alias honoured; byte-identical to min_relevance=0.30
min_relevance=0.30 200 hits=5 c3f900bd 200 hits=5 c3f900bd control — unchanged
half-life=7 200 2c6cb99c 200 2c6cb99c (== half_life=7) alias resolves (top-12 don't reorder at hl=7)

The card's sharpest control, separator alone deciding whether the filter fires:

                     BEFORE          AFTER
min_relevance=0.30   hits=5          hits=5   <- documented spelling (control)
min-relevance=0.30   hits=12         hits=5   <- hyphen alias now honoured

not_path=casey disclosure closed: the one-character typo returned 2 casey/
nodes at 200 BEFORE; AFTER it is honoured as an alias and returns 0 casey/
nodes, byte-for-byte identical to not-path=casey.

Value-validation 400s — byte-identical wording BEFORE→AFTER (controls)

k=notanumber      400  "k" must be a non-negative integer            [68a2c239]
half_life=abc     400  "half_life" must be a non-negative number of days [a4d444f8]
decay_floor=2     400  "decay_floor" must be in [0, 1]               [4bfc9bc6]
min_relevance=-1  400  "min_relevance" must be a non-negative number [da5c1b2b]

Strictness does not leak off /search (control)

/api/v1/stats?bogus=1        -> 200
/api/v1/graph?not_a_param=x  -> 200
/api/v1/search?q=agent&type= -> 200   (empty value reads as unset)
/api/v1/search?q=agent&path=casey/&path=infra/ -> 200  (repeated OR)

validate + lint --strict on the real corpus (controls, HTTP-only change)

$ okfctl validate ~/src/knowledge-base/bundles/knowledge
OK: bundle conforms to the OKF spec floor

$ okfctl lint --strict ~/src/knowledge-base/bundles/knowledge
1 lint finding(s)   # pre-existing per-node okf_spec_version, unrelated to this change

Both controls proven (per detector change)

  • Positive — the fix fires: notpath/exclude_path/nonsense_param → 400;
    not_path/min-relevance/not_tag aliases now honoured with reordered/
    filtered output.
  • Negative (load-bearing) — the fix stays silent on good cases: every
    documented spelling still 200; ?type= empty reads as unset; repeated params
    still OR; value-validation 400 wording unchanged; /stats and /graph
    untouched. The unit test TestSearch_HyphenAliasHonouredForScoringParam uses
    the dated decay fixture at half_life=90 so the honoured-with-reorder path is
    proven where the top-12 real-corpus rows do not reorder at hl=7.

Ordering note (#78)

The alias set lands both separators for every parameter, so the sibling
lexical-gate card can name its parameter in either convention and inherit the
canonicalization for free.

§-citations

OKF v0.2 SPEC.md verified upstream (Version 0.2); §4.1 is Frontmatter —
the filters key on type/tag/path metadata defined there and are unchanged
in meaning.

GET /api/v1/search accepted any unrecognized query parameter, returned
200, and applied nothing. A caller who misspelled an exclusion filter
got back exactly the content they meant to exclude with a success code
and no signal — a filter failing OPEN, indistinguishable in the response
and access log from a correct query. The trap is easy to hit because
filters take hyphens (not-path) while scoring params take underscores
(half_life), so the wrong separator is the natural mistake.

Canonicalize every incoming key to a closed accepted set before reading
any value: an unknown key is now a 400 (fail closed), with a did-you-mean
when it is one edit away from a real one. Both separators are accepted as
aliases for every parameter and merged into the same dimension, so a
caller cannot guess wrong and hyphen/underscore variants OR together.
Value-validation 400s keep their exact wording and now fire through the
alias spelling with the canonical name in the message. Strictness is
confined to /search; /stats and /graph are untouched (§4.1 filters
unchanged in meaning — only the HTTP key surface changes).

Closes #77

@cwest cwest left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes needed.

The key handling is sound: canonicalization runs before any value is read, an unknown key fails closed with a 400 rather than the old silent 200, and both separators resolve to the same canonical name so a caller can't guess wrong. The did-you-mean bound scales with key length, so a near miss like notpath suggests not-path while nonsense_param gets a bare 400. Canonicalization is confined to the search handler; /stats and /graph keep their own closures and never touch it, so strictness can't leak.

Ran the full suite under -race (8 packages green), the conformance suite green, and reproduced every acceptance case against the real 262-node corpus at this head:

  • notpath / exclude_path / nonsense_param now 400; notpath names not-path in the suggestion.
  • The disclosure path is closed: not_path=casey is now a valid alias returning 0 casey/ nodes, byte-identical to not-path=casey (both 3b5d4e07), not the old 200-with-2-leaked-nodes.
  • min-relevance=0.30 is honoured via the alias: hits 12 to 5, byte-identical to min_relevance=0.30 (both 85535cb7).
  • The four value-validation 400s keep their exact wording, and an alias like half-life=abc reports the canonical half_life name.
  • ?type= empty value, repeated path, /stats?bogus=1, and /graph?not_a_param=x all stay 200 — no regression, no leak.

The alias set lands both separators for every parameter, so the sibling lexical-gate work inherits canonicalization without having to pick a convention.

@cwest
cwest marked this pull request as ready for review August 3, 2026 14:40
@cwest
cwest merged commit 9462e7d into main Aug 3, 2026
1 check passed
@cwest
cwest deleted the wt/t_ef642832 branch August 3, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 fix(api): reject unknown query parameters in /api/v1/search instead of ignoring them

1 participant