throttle: stop claiming to limit pushes we cannot see
I reintroduced the exact defect I had just criticised. ACTION_BASE listed
"push" and "push.force", but git push goes straight to Gitea over HTTPS and
never touches this API — so nothing records a push, a count would be zero
forever, and enforce() would look up a limit, count nothing, and allow
everything. A silent no-op wearing the costume of a control, made worse by a
config name that implies the protection exists.
Split into ACTION_BASE (actually enforced: repo.create, grant.create) and
NOT_ENFORCED_HERE (push, push.force) with the reason and the remedy written
down: enforcing push velocity needs a Gitea-side pre-receive or push webhook
reporting into agent_actions.
enforce("push") now raises rather than silently allowing, and a test asserts the
two sets stay disjoint.
Found by auditing whether the auth fix could be walked around — every
/api/v1/repos/* route does require a caller, and the only unauthenticated
endpoints are /health, /version and the HMAC-verified webhook.
83 tests green.
Co-Authored-By: Claude (Fable 5) <noreply@anthropic.com>
This commit is contained in:
@@ -36,10 +36,28 @@ log = logging.getLogger(__name__)
|
||||
|
||||
WINDOW = timedelta(days=1)
|
||||
|
||||
# action name -> the settings field holding its per-day base for a standard band
|
||||
# Actions this module ACTUALLY enforces: they pass through our API, so we can
|
||||
# both count and refuse them.
|
||||
ACTION_BASE: dict[str, str] = {
|
||||
"repo.create": "rate_repo_creates_per_day",
|
||||
"grant.create": "rate_grants_per_day",
|
||||
}
|
||||
|
||||
# ⚠️ DECLARED BUT NOT ENFORCEABLE HERE — and named, rather than quietly listed
|
||||
# alongside the real ones.
|
||||
#
|
||||
# `git push` goes straight to Gitea over HTTPS and never touches this API, so
|
||||
# nothing records a `push` action and a count of them would be zero forever.
|
||||
# Listing these in ACTION_BASE (as this module first did) would make `enforce`
|
||||
# look up a limit, count nothing, and allow everything — a silent no-op wearing
|
||||
# the costume of a control. That is the same dead-code pattern this module was
|
||||
# written to remove, and it is worse here because the config name implies the
|
||||
# protection exists.
|
||||
#
|
||||
# Enforcing push velocity requires a Gitea-side hook (pre-receive or the push
|
||||
# webhook) that reports into `agent_actions`. Until that exists these settings
|
||||
# are inert, and saying so is the honest option.
|
||||
NOT_ENFORCED_HERE: dict[str, str] = {
|
||||
"push": "rate_pushes_per_day",
|
||||
"push.force": "rate_force_pushes_per_day",
|
||||
}
|
||||
|
||||
@@ -283,3 +283,30 @@ def test_routing_does_not_depend_on_signature_wellformedness():
|
||||
|
||||
assert not looks_like_ept("not-a-token")
|
||||
assert not looks_like_ept("")
|
||||
|
||||
|
||||
def test_only_actions_that_route_through_this_api_are_claimed_enforced():
|
||||
"""git push never touches this API, so a push limit here would count zero
|
||||
forever and allow everything — a silent no-op wearing the costume of a
|
||||
control. Push limits must stay in NOT_ENFORCED_HERE until a Gitea-side hook
|
||||
reports pushes into agent_actions."""
|
||||
from api.app.throttle import ACTION_BASE, NOT_ENFORCED_HERE
|
||||
|
||||
assert "push" not in ACTION_BASE
|
||||
assert "push.force" not in ACTION_BASE
|
||||
assert "push" in NOT_ENFORCED_HERE
|
||||
assert not set(ACTION_BASE) & set(NOT_ENFORCED_HERE)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_enforce_refuses_an_action_it_cannot_actually_limit():
|
||||
"""Asking to throttle 'push' must raise, not silently allow."""
|
||||
from api.app.auth import ActorType, Caller
|
||||
from api.app.config import Settings
|
||||
from api.app.errors import RepairPointer
|
||||
from api.app.throttle import enforce
|
||||
|
||||
caller = Caller(actor_type=ActorType.agent, passport="ET26-X", band="gold")
|
||||
with pytest.raises(RepairPointer) as exc:
|
||||
await enforce(None, Settings(), caller, "push")
|
||||
assert exc.value.code == "throttle_unknown_action"
|
||||
|
||||
Reference in New Issue
Block a user