fix(go-policy): key rate-limit state by (action, agent_id, tenant)

[Ken Tannenbaum (finnoybu)]

Summary

PolicyEngine.checkRateLimit keyed pe.rateLimits by rule.Action alone. With a 60-second window of N calls, any caller's traffic counted against the same budget — one noisy agent (or tenant) could exhaust the budget and starve every other caller for the rest of the window.

In a multi-tenant deployment that's a denial-of-service vector: agent A's misbehavior reaches agent B's API budget through nothing more than sharing an action name.

Change

policy.go:

• checkRateLimit now takes context map[string]interface{} and composes the key from (rule.Action, agent_id, tenant). Both context keys are pulled with a small helper that handles non-string values via fmt.Sprint.
• Evaluate passes context through to checkRateLimit.
• The key delimiter is 0x1f (ASCII Unit Separator) so the join is unambiguous even if action / agent_id / tenant contain \":\" or other printable characters.

Compatibility note

Callers that pass nil context — for example middleware.go:256 — collapse to empty segments for both agent_id and tenant, so they retain the previous globally-scoped behavior. This is a deliberate safe fallback: nothing in the codebase currently relies on cross-caller starvation, and upgrading callers later to plumb agent_id is a no-op for the nil case until they actually pass real values.

Tests

go test -run TestRateLimit ./... -v
  --- PASS: TestRateLimiting
  --- PASS: TestRateLimitIsolatedPerAgent
  --- PASS: TestRateLimitIsolatedPerTenant
  --- PASS: TestRateLimitNilContextRetainsGlobalBehavior

• TestRateLimitIsolatedPerAgent — alice exhausts her budget; bob's calls are unaffected.
• TestRateLimitIsolatedPerTenant — same agent_id, different tenant, independent budget.
• TestRateLimitNilContextRetainsGlobalBehavior — nil context still rate-limits at the action level (legacy caller compatibility).

Companion

This is part 2 of REVIEW.md item #7. Part 1 — the check-then-increment counter fix — is filed as #2021. The two PRs can be merged in either order; this one depends only on the public Evaluate / checkRateLimit signatures.

Test plan

• CI passes
• Existing TestRateLimiting continues to pass under the nil-context default

---

Surfaced as part of the AEGIS Initiative review of the agent-governance-toolkit (HIGH severity, Go section). Filed by Ken Tannenbaum (AEGIS Initiative).

[github-actions]

🤖 AI Agent: breaking-change-detector — API Compatibility

API Compatibility

Severity	Change	Impact
Breaking	checkRateLimit method signature changed to accept context map[string]interface{}	Callers that do not provide a context will still function, but those expecting the previous signature will break.

[github-actions]

🤖 AI Agent: test-generator — `policy.go`

policy.go

• TestRateLimitKeyCollision -- Validate that different (action, agent_id, tenant) combinations do not collide in the rate-limit key.
• TestRateLimitInvalidContextHandling -- Ensure checkRateLimit handles unexpected or malformed context values gracefully.
• TestRateLimitEmptySegments -- Verify behavior when agent_id or tenant is explicitly set to an empty string in the context.

policy_test.go

• TestRateLimitWindowExpiration -- Confirm that rate limits reset correctly after the specified time window.
• TestRateLimitConcurrentAccess -- Validate thread safety of checkRateLimit under concurrent calls with overlapping keys.

[github-actions]

🤖 AI Agent: docs-sync-checker — Docs Sync

Docs Sync

• checkRateLimit(rule PolicyRule, context map[string]interface{}) in policy.go -- missing docstring
• README.md -- section on rate limiting needs update
• CHANGELOG.md -- missing entry for behavioral change regarding rate limiting by (action, agent_id, tenant)

[github-actions]

🤖 AI Agent: code-reviewer — View details

TL;DR: 0 blockers, 0 warnings. No issues found. Clean change.

#	Sev	Issue	Where

| 1 | Info | Rate limit state is now keyed by (action, agent_id, tenant) | policy.go |
| 2 | Info | Tests added for isolated rate limits per agent and tenant | policy_test.go |
| 3 | Info | Nil context retains global behavior for backward compatibility | policy.go |

Action items: None.

Warnings:

#	Issue	Where
1	Fine as follow-up PRs	None

[github-actions]

🤖 AI Agent: security-scanner — View details

No security issues found.

[github-actions]

🟡 Contributor Check: MEDIUM

Check	Result
Profile	MEDIUM
Credential	NONE
Overall	MEDIUM

Automated check by AGT Contributor Check.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	✅ Passed	No issues found
🛡️ Security Scan	✅ Passed	No issues found
🔄 Breaking Changes	✅ Completed	Analysis complete
📝 Docs Sync	✅ Completed	Analysis complete
🧪 Test Coverage	✅ Completed	Analysis complete

Verdict: ✅ Ready for human review

[Imran Siddique (imran-siddique)]

Reviewed. Security fix from finnoybu audit.