Repository navigation
Conversation
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.
Owner
Author
|
Landed on master in the v0.1.0 chain: the branch was fast-forward merged as part of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(server): rate-limit scoring edits per cell
RFC-AMP-001 §5 lists decay-score manipulation as a threat: a caller PATCHing
scoringin a loop can keep a cellactivepast its intended relevance window, ordrive a competing memory into archive. Its mitigation column ends with
"Implementations SHOULD rate-limit
scoringPATCH frequency per cell". Nothingdid - 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.ratelimitadds the budget. The choices worth naming:scoringis counted. Rewritingcontent,provenanceor
access_policyis not what the threat describes. Counting those would refuseordinary edits, which is not a mitigation.
its own lockout by hammering and would make
Retry-Aftergrow 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.
403andlearns nothing about the remaining budget.
429carriesRetry-After, plus the same number inerror.details.retry_after_seconds. A rate limit that does not say how long towait leaves the client to guess, so
AMPErrorcan now carry response headers.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.
that is five edits per cycle, generous for a person and useless for a loop.
AMP_SCORING_PATCH_LIMIT/AMP_SCORING_PATCH_WINDOW_SECONDSchange it,0disables it, and
GET /specreportsscoring_patch_limit(nullwhen off) sothe 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 --strictclean.