Skip to content

feat(server): rate-limit scoring edits per cell - #13

Closed
glatinone wants to merge 1 commit into
fix/conformance-portable-specfrom
feat/scoring-patch-limit
Closed

glatinone wants to merge 1 commit into
fix/conformance-portable-specfrom
feat/scoring-patch-limit

Conversation

@glatinone

Copy link
Copy Markdown
Owner

feat(server): rate-limit scoring edits per cell

RFC-AMP-001 §5 lists decay-score manipulation as a threat: a caller PATCHing
scoring in a loop can keep a cell active past its intended relevance window, or
drive a competing memory into archive. Its mitigation column ends with
"Implementations SHOULD rate-limit scoring PATCH frequency per cell". Nothing
did - the first half of the mitigation (a scoring change only lands on the next
lifecycle pass, so the engine's cadence bounds it) was in place, and the second
half was absent.

amp_server.ratelimit adds the budget. The choices worth naming:

  • Only a PATCH carrying scoring is counted. Rewriting content, provenance
    or access_policy is not what the threat describes. Counting those would refuse
    ordinary edits, which is not a mitigation.
  • A refused attempt is not recorded. Counting refusals would let a caller extend
    its own lockout by hammering and would make Retry-After grow while it waits.
    Not recording means the wait shrinks as the oldest allowed edit ages out - the
    behaviour the number promises. A test advances a fake clock twice and asserts the
    wait goes 50 -> 40.
  • The budget is per cell, so one busy cell cannot spend another's.
  • Access is checked first, so a caller who may not write the cell gets 403 and
    learns nothing about the remaining budget.
  • 429 carries Retry-After, plus the same number in
    error.details.retry_after_seconds. A rate limit that does not say how long to
    wait leaves the client to guess, so AMPError can now carry response headers.
  • Tracked cells are bounded. Counters for cells nobody edits are dropped as
    their window empties, and the rest are capped at 10k: an unbounded map keyed by
    cell id is a slow leak in a server that runs for months.
  • Default 5 per cell per hour - the lifecycle engine runs hourly by default, so
    that is five edits per cycle, generous for a person and useless for a loop.
    AMP_SCORING_PATCH_LIMIT / AMP_SCORING_PATCH_WINDOW_SECONDS change it, 0
    disables it, and GET /spec reports scoring_patch_limit (null when off) so
    the advertised numbers are the enforced ones.

A bad setting falls back with a warning instead of stopping the server, unlike the
embedding provider and the key store: a wrong number here degrades a mitigation,
it does not corrupt data.

Counters are per process - two servers over one Postgres keep two budgets. Stated
in the module, the docs and the changelog rather than left implied; a shared
counter would need its own storage and its own consistency story for a threat that
one process already sees in full.

Verified: 233 server tests (18 new), 38/38 conformance vectors against a live
server, ruff/mypy clean, mkdocs build --strict clean.

RFC-AMP-001 §5 lists decay-score manipulation as a threat: a caller PATCHing
`scoring` in a loop can keep a cell `active` past its intended relevance window, or
drive a competing memory into archive. Its mitigation column ends with
"Implementations SHOULD rate-limit `scoring` PATCH frequency per cell". Nothing
did - the first half of the mitigation (a scoring change only lands on the next
lifecycle pass, so the engine's cadence bounds it) was in place, and the second
half was absent.

`amp_server.ratelimit` adds the budget. The choices worth naming:

- **Only a PATCH carrying `scoring` is counted.** Rewriting `content`, `provenance`
  or `access_policy` is not what the threat describes. Counting those would refuse
  ordinary edits, which is not a mitigation.
- **A refused attempt is not recorded.** Counting refusals would let a caller extend
  its own lockout by hammering and would make `Retry-After` grow while it waits.
  Not recording means the wait shrinks as the oldest allowed edit ages out - the
  behaviour the number promises. A test advances a fake clock twice and asserts the
  wait goes 50 -> 40.
- **The budget is per cell**, so one busy cell cannot spend another's.
- **Access is checked first**, so a caller who may not write the cell gets `403` and
  learns nothing about the remaining budget.
- **`429` carries `Retry-After`**, plus the same number in
  `error.details.retry_after_seconds`. A rate limit that does not say how long to
  wait leaves the client to guess, so `AMPError` can now carry response headers.
- **Tracked cells are bounded.** Counters for cells nobody edits are dropped as
  their window empties, and the rest are capped at 10k: an unbounded map keyed by
  cell id is a slow leak in a server that runs for months.
- Default 5 per cell per hour - the lifecycle engine runs hourly by default, so
  that is five edits per cycle, generous for a person and useless for a loop.
  `AMP_SCORING_PATCH_LIMIT` / `AMP_SCORING_PATCH_WINDOW_SECONDS` change it, `0`
  disables it, and `GET /spec` reports `scoring_patch_limit` (`null` when off) so
  the advertised numbers are the enforced ones.

A bad setting falls back with a warning instead of stopping the server, unlike the
embedding provider and the key store: a wrong number here degrades a mitigation,
it does not corrupt data.

Counters are per process - two servers over one Postgres keep two budgets. Stated
in the module, the docs and the changelog rather than left implied; a shared
counter would need its own storage and its own consistency story for a threat that
one process already sees in full.

Verified: 233 server tests (18 new), 38/38 conformance vectors against a live
server, ruff/mypy clean, `mkdocs build --strict` clean.
@glatinone

Copy link
Copy Markdown
Owner Author

Landed on master in the v0.1.0 chain: the branch was fast-forward merged as part of b940905..92b88ee and released as v0.1.0. Closing so the open list matches reality - the commits are in master, and the tag points at them.

@glatinone glatinone closed this Oct 4, 2026
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.

1 participant