Skip to content

feat(gateway): per-user permission tiers#3156

Closed
ReqX wants to merge 8 commits into
NousResearch:mainfrom
ReqX:feature/user-permission-tiers
Closed

feat(gateway): per-user permission tiers#3156
ReqX wants to merge 8 commits into
NousResearch:mainfrom
ReqX:feature/user-permission-tiers

Conversation

@ReqX

@ReqX ReqX commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Introduces opt-in permission tiers for the Hermes gateway, letting operators restrict tools, time windows, exec approvals, and admin commands per user or role — without modifying the agent itself.

Strictly opt-in: if permission_tiers is absent from config, zero behavior change. Every user has full access, identical to legacy behavior.

Related Issue

No existing issue — new feature.

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • gateway/config.py — tier config schema, validation, type coercion, empty-config warning
  • gateway/run.py — tier resolution, enforcement gates, approval isolation, most-restrictive fallback
  • hermes_cli/commands.pyadmin_only flag on CommandDef for admin command gating
  • docs/feature-user-permissions.md — full feature docs with platform ID reference table
  • tests/gateway/test_permission_tiers.py — 88 tests
  • cli-config.yaml.example — commented-out permission tiers example block

How to Test

  1. Add a permission_tiers block to ~/.hermes/config.yaml (see cli-config.yaml.example or docs)
  2. Start gateway with any messaging platform
  3. Verify tier resolution: send a message and check logs for resolved tier
  4. Verify restrictions: restricted user should not see admin commands, exec approvals, or tools outside their allowed toolsets
  5. Run tests: python -m pytest tests/gateway/test_permission_tiers.py -q

Security Hardening

Reviewed through 2 rounds — human audit followed by a 4-LLM verification run. 24 findings total (2 critical, 6 high, 11 medium, 5 low) — all resolved. Key areas addressed:

  • Fail-closed on misconfiguration (missing tiers, typos, invalid times)
  • Type coercion for YAML bool/string confusion
  • Approval isolation in shared sessions
  • Quick exec command gating
  • Most-restrictive fallback instead of hardcoded admin

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (88 tier tests + 6538 project tests, 22 pre-existing failures in unrelated areas)
  • I've added tests for my changes
  • I've tested on my platform: Linux

Documentation & Housekeeping

  • I've updated relevant documentation (docs/feature-user-permissions.md)
  • I've updated cli-config.yaml.example with permission_tiers example

@ReqX ReqX force-pushed the feature/user-permission-tiers branch from c685595 to bdefe7e Compare March 29, 2026 11:18
ReqX added 7 commits March 29, 2026 20:35
Phase 1: Add dataclasses for user permission tiers (opt-in).

New classes:
- TimeRestrictions: start/end times, timezone, optional day filter
- TierDefinition: allowed_toolsets, allow_exec, allow_admin_commands,
  time_restrictions, i18n messages
- UserTierConfig: per-user tier mapping with locale
- PermissionTiersConfig: default_tier, tiers dict, users dict

GatewayConfig gains permission_tiers field (Optional, defaults to None).
load_gateway_config() reads permission_tiers from config.yaml.

All classes include from_dict()/to_dict() for YAML serialization.
Round-trip verified: GatewayConfig() -> to_dict() -> from_dict() preserves state.
Phase 2: Hook permission tiers into the agent pipeline.

New methods on GatewayRunner:
- _resolve_user_tier(source) -> str: user -> default_tier -> 'admin'
- _get_tier_config(tier_name) -> Optional[TierDefinition]
- _get_tier_allowed_toolsets(tier_name) -> List[str]
- _is_within_time_window(tier) -> (bool, Optional[str])

Hooks:
- In _run_agent(): intersect enabled_toolsets with tier-allowed list
- In _agent_config_signature(): include user_tier for cache isolation
- Inject tier system prompt into combined_ephemeral (soft enforcement)

All paths are opt-in: if permission_tiers is None, no filtering occurs.
…ands

Phase 3: Exec approval gating
- /approve and /deny handlers check allow_exec permission
- Approval hint suppressed for users without exec permission
- Uses _format_tier_message() for i18n denial response

Phase 4: Time restriction early gate
- _handle_message() checks time window before agent dispatch
- Users outside allowed hours get localized denial message
- Cross-midnight windows and day-of-week filters supported

Phase 5: Admin slash command restrictions
- /model, /provider, /update, /reload-mcp, /config gated by allow_admin_commands
- Safe commands (/new, /reset, /help, /status) always allowed
- Check happens after alias resolution, before handler dispatch
Documents the implemented permission tier system:
- Architecture diagram and config schema
- All 6 phases with actual code patterns (not planned)
- Design decisions and rationale
- Test coverage summary (45 tests)
- Sample config for onboarding
- Commit history and diff stats
…strictions

- _resolve_user_tier() validates tier exists in config; falls back to
  default_tier on typos (fail-closed, not fail-open)
- _is_within_time_window() wraps HH:MM parsing in try/except for
  ValueError/TypeError; validates 0-1439 minute range
- Both guards log warnings and allow access (fail-open for time,
  fail-closed for tier resolution)

4 new tests: typo tier fail-closed, cascade to admin, user_id=None,
  invalid time format.
PermissionTiersConfig.from_dict() now auto-injects TierDefinition entries
for any tier name referenced by default_tier or users but missing from
the tiers dict. Injected tiers are permissive (allow_exec=True,
allowed_toolsets=[*]) as a safe fallback.

This closes the theoretical edge case where all three fallback levels
(user tier, default_tier, admin) could return a name not in tiers,
causing _get_tier_config() to return None and bypass all restrictions.
Phase 5 — Type validation hardening:
- H5: coerce bool/string types in TierDefinition.from_dict()
- M8: handle allowed_toolsets: null gracefully
- M7: invalid days values produce empty list (always blocked)
- M9: catch AttributeError in time parsing

Phase 6 — Quick command and approval hardening:
- H6: gate quick exec commands behind allow_exec
- M11: key pending approvals by (session_key, user_id) to prevent
  cross-user approval in shared sessions

Phase 7 — Config boundary hardening:
- M10: warn when permission_tiers block has no tiers defined
- L4: runtime tier fallback uses most-restrictive tier, not admin
- L5: UserTierConfig.tier defaults to None, not admin

Phase 8 — Documentation:
- update user mapping docs for null tier default
- add platform ID reference table with stability notes
- add troubleshooting for type coercion, days behavior, quick exec,
  approval isolation, and empty config blocks

88 tier tests passing. 6538 project tests passing. 0 regressions.
@ReqX ReqX force-pushed the feature/user-permission-tiers branch from bdefe7e to 5a77667 Compare March 29, 2026 20:45
@ReqX

ReqX commented Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

👋 PR updated with security hardening.

One known limitation worth flagging:
Mattermost uses user_name (mutable username) as the user identifier rather than a stable user ID. If a user changes their username, their tier mapping breaks. This is a platform adapter limitation, not something fixable at the permission tiers layer. Operators should either use the wildcard "*" entry as a conscious safety net, or accept the potential hit on rename. Documented in docs/feature-user-permissions.md.

Happy to rework anything — appreciate the review.

@ReqX

ReqX commented Mar 30, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR. I'll file a new PR that fully implements Issue #527, including:

  • All security hardening from 4 audit rounds
  • Auto-tier from env vars and pairing
  • Cross-platform isolation (composite keys)
  • Owner-only command restriction
  • Defense-in-depth enforcement (exec tool stripping, delegate propagation)
  • Full feature documentation

This v1 PR served as the initial implementation pass. All its functionality is preserved and extended in the new PR.

Will link the new PR here once opened.

@ReqX

ReqX commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #4443.

This v1 PR was a first implementation pass. The replacement is a complete rewrite that adds:

  • Security hardening (5 audit rounds, 17 fixes)
  • Auto-tier from env vars and pairing
  • Composite keys for cross-platform isolation
  • Owner-only command restriction + owner escalation guard
  • Defense-in-depth: exec tools stripped from schema, delegate propagation
  • Full documentation (docs/feature-user-permissions.md)
  • 448 tests (vs 88 in v1)

Plus: per-user tool overrides, per-command allowlists, Telegram/Discord role mapping,
audit logging, persistent usage tracking, /promote and /audit commands.

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