Merge branch 'main' into fix-ssh-authorized-users-multi-rule

# Conflicts:
#	management/server/types/account.go
#	shared/management/types/networkmap_components.go

AGENTS.md

@@ -1,514 +0,0 @@
-# NetBird Agent Guidelines
-
-**NetBird** is an open-source connectivity platform: a WireGuard®-based overlay
-network with a control plane. The **agent** (`client/`) runs on user machines as
-a privileged daemon and manages the WireGuard interface, routing, firewall, and
-DNS. **Management** (`management/`) is the control plane and REST/gRPC API,
-**Signal** (`signal/`) brokers peer handshakes, **Relay** (`relay/`) carries
-traffic when a direct tunnel is impossible, and **Proxy** (`proxy/`) is the
-identity-aware proxy behind Agent Network.
-
-This file applies to the whole repository, and is the single source of truth for
-agent guidance here. `CLAUDE.md` is a one-line pointer to it — keep the guidance
-in this file, not duplicated there.
-
-## Contents
-
-- [NetBird Agent Guidelines](#netbird-agent-guidelines)
-  - [Contents](#contents)
-  - [STOP and ask the user before](#stop-and-ask-the-user-before)
-  - [Quick reference](#quick-reference)
-  - [Structure](#structure)
-  - [Where to look](#where-to-look)
-  - [Repo-wide principles](#repo-wide-principles)
-  - [Error handling](#error-handling)
-  - [Comments](#comments)
-  - [Testing](#testing)
-  - [Pitfalls](#pitfalls)
-  - [Commits, PRs, releases](#commits-prs-releases)
-  - [After you push: CI and review bots](#after-you-push-ci-and-review-bots)
-  - [Discussion and support](#discussion-and-support)
-
-## STOP and ask the user before
-
-- **Opening a pull request for anything beyond a trivial fix, without an agreed
-  ticket.** Ask the user directly: *"Is there a discussion or issue for this
-  change?"* NetBird is discussion-first — community reports start in
-  [Discussions](https://github.com/netbirdio/netbird/discussions), DevRel
-  validates them, and only validated discussions become issues. A PR that
-  changes behavior with no linked issue may be closed on arrival. If there is no
-  ticket, offer to draft the discussion post **instead of** the PR, and wait for
-  the user's call. Only typos, broken links, documentation corrections, and
-  one-line fixes that already have an issue can skip this.
-- **Designing in any high-risk area** (see
-  [CONTRIBUTING.md](CONTRIBUTING.md#high-risk-areas)): public API and OpenAPI
-  schema, gRPC protos, behavior existing deployments would notice after an
-  upgrade, peer connectivity (ICE, NAT traversal, relay selection, WireGuard® or
-  Rosenpass key handling), client system integration (routing, firewall, DNS,
-  interface), authentication and authorization, CLI or service flags, config
-  file format, daemon IPC, store schema and migrations, or a new feature. The
-  design gets agreed in the ticket before code is written.
-- **Writing a store migration or changing a persisted model.** Migrations are
-  one-way in the field and both the GORM and pgx paths may need the change.
-- **Hand-editing generated code.** `*.pb.go`, `*.gen.go`, and mocks are outputs.
-  Edit the source (`.proto`, `openapi.yml`) and rerun the matching
-  `generate.sh`.
-- **Adding, removing, or bumping a dependency**, and never vendor a fork.
-- **Weakening a security control** — authentication, authorization, certificate
-  verification, privilege dropping, or peer identity checks — even when it is
-  the fastest way to make a test pass.
-- **Force-pushing to `main`**, force-pushing any branch that is already under
-  review, amending pushed commits, or bypassing hooks with `--no-verify`.
-
-## Quick reference
-
-```bash
-# Build
-go build ./...
-cd client && CGO_ENABLED=0 go build .        # agent
-cd management && go build .                  # management service
-cd signal && go build .                      # signal service
-
-# Verify (run before every push)
-go fmt ./...
-make lint            # golangci-lint on files changed vs origin/main (also the pre-push hook)
-make lint-all        # full-repository lint, matches CI
-make test-unit       # host-safe unit tests, -tags devcert, no sudo
-make test-privileged # privileged-tagged suite in a Docker container with NET_ADMIN
-make setup-hooks     # wire make lint into .githooks/pre-push
-
-# Narrow runs
-go test ./client/internal/dns/...
-go test -race -run TestPeerConn ./client/internal/peer/...
-PRIV_RUN=TestNftablesManager PRIV_PKGS=./client/firewall/nftables/... make test-privileged
-
-# Code generation (never hand-edit the output)
-./shared/management/http/api/generate.sh   # REST types from openapi.yml
-./shared/management/proto/generate.sh
-./shared/signal/proto/generate.sh
-./client/proto/generate.sh
-./flow/proto/generate.sh
-
-# Run locally (lab only, never on a machine you rely on)
-sudo ./client/netbird up --log-level debug --log-file console
-sudo ./client/netbird down                   # teardown: restores routing, firewall, DNS
-./signal/signal run --log-level debug --log-file console
-./management/management management --log-level debug --log-file console --config ./management.json
-```
-
-`netbird up` needs root and rewrites the host's routing table, firewall rules,
-DNS configuration, and WireGuard® interface. Run it only in a disposable test
-environment (a VM, container, or throwaway host) that you can rebuild, never on
-a workstation or server whose connectivity matters. Run `sudo netbird down`
-before you stop working, before rebuilding the binary, and on every failure
-path, so the host's networking state is restored instead of left half-applied.
-See [Pitfalls](#pitfalls) for why cleanup on every exit path matters.
-
-## Structure
-
-```text
-netbird/
-├── client/              NetBird agent
-│   ├── cmd/             agent CLI
-│   ├── internal/        agent business logic (engine, peer, dns, routemanager, ...)
-│   ├── server/          daemon for background execution
-│   ├── proto/           daemon gRPC protos
-│   ├── iface/           WireGuard® interface management
-│   ├── firewall/        nftables, iptables, pf, WFP, userspace backends
-│   ├── ssh/             built-in SSH server and client
-│   ├── ui/              desktop UI (Wails v3 + React)
-│   ├── android/, ios/   mobile bindings
-│   ├── wasm/            WebAssembly build
-│   └── mdm/, system/    MDM policy, host information
-├── management/          control plane
-│   └── server/          account, peer, groups, networks, posture, permissions,
-│                        settings, store, http (REST), idp, integrations, migration
-├── signal/              handshake broker (peer/, server/)
-├── relay/               relay service (protocol/, server/, healthcheck/)
-├── proxy/               identity-aware proxy (llm/, acme/, accesslog/, middleware/, tcp/, udp/)
-├── agent-network/       Agent Network overview
-├── shared/              imported by both agent and services
-│   ├── management/      proto/, client/, http/api (OpenAPI + generated types)
-│   ├── signal/          proto/, client/
-│   └── relay/, auth/, sshauth/, metrics/
-├── e2e/                 end-to-end suites and harness
-├── encryption/, dns/, route/, stun/, sharedsock/, util/, flow/
-├── infrastructure_files/  docker compose and getting-started templates
-└── release_files/       files packaged into releases
-```
-
-## Where to look
-
-| Task                        | Location                                                     |
-| --------------------------- | ------------------------------------------------------------ |
-| REST API / OpenAPI          | `shared/management/http/api/` + `management/server/http/`    |
-| Management gRPC protocol    | `shared/management/proto/`                                   |
-| Signal protocol             | `shared/signal/proto/`                                       |
-| Daemon IPC protocol         | `client/proto/`                                              |
-| Peer connection and NAT     | `client/internal/peer/`                                      |
-| Network map handling        | `client/internal/engine.go`, `shared/management/networkmap/` |
-| Routing                     | `client/internal/routemanager/`, `route/`                    |
-| Firewall backends           | `client/firewall/`                                           |
-| DNS                         | `client/internal/dns/`, `dns/`                               |
-| WireGuard® interface        | `client/iface/`                                              |
-| Persistence and migrations  | `management/server/store/`, `management/server/migration/`   |
-| IdP integrations            | `management/server/idp/`                                     |
-| Permissions model           | `management/server/permissions/`                             |
-| LLM routing / Agent Network | `proxy/internal/llm/`, `agent-network/`                      |
-| End-to-end tests            | `e2e/`                                                       |
-
-## Repo-wide principles
-
-1. **Run `go fmt` on every modified Go file.** Formatting is not optional.
-2. **Zero unaddressed diagnostics.** Fix IDE and linter warnings on code you
-   touch, and delete imports, helpers, and parameters your refactor orphaned.
-   Exception: unused parameters in shared code may be consumed by builds outside
-   this repository — do not remove them, ask instead.
-3. **Function comments are mandatory for exported functions**, written as full
-   sentences with a period, starting with the identifier name.
-4. **Prefer private functions and constants.** Export only what a caller outside
-   the package genuinely needs.
-5. **Early returns and guard clauses.** Handle errors and edge cases first
-   instead of nesting `if`/`else` chains.
-6. **Split complex functions.** If a function trips a complexity warning, break
-   it into named helpers rather than silencing the warning.
-7. **Avoid LLM-slop tells:** em dashes, hedging narration, restating the diff in
-   prose, trailing summaries. Defaults, not absolute bans. Applies to code,
-   comments, commit messages, and PR descriptions alike.
-8. **Concurrency: do a two-pass race analysis after every change** that adds
-   shared state. Guard maps and slices with a mutex, keep critical sections
-   short, and run `go test -race` on the touched packages.
-9. **Cross-platform builds must keep working.** The agent targets Linux, macOS,
-   Windows, FreeBSD, Android, and iOS. When you add a platform-specific file,
-   add the counterpart or a build-tagged fallback for the others.
-10. **Never hand-edit generated files.** Change the source and regenerate.
-11. **Never log secrets** — private keys, setup keys, tokens, PAT values — and
-    keep peer IPs and hostnames out of logs above debug level.
-
-## Error handling
-
-Use single-assignment form when the error is only needed inside the `if`:
-
-```go
-// Good
-if err := someCall(); err != nil {
-    return fmt.Errorf("context: %w", err)
-}
-
-// Bad - unnecessary split
-err := someCall()
-if err != nil {
-    return fmt.Errorf("context: %w", err)
-}
-```
-
-Use multiple assignment when the value is needed after the block:
-
-```go
-result, err := someCall()
-if err != nil {
-    return fmt.Errorf("context: %w", err)
-}
-```
-
-Add short, meaningful context, and **do not** start `fmt.Errorf` messages with
-obvious words like "failed to" or "error":
-
-```go
-// Good
-return fmt.Errorf("parse remote address: %w", err)
-return fmt.Errorf("listen on %s: %w", addr, err)
-
-// Bad
-return fmt.Errorf("failed to parse remote address: %w", err)
-return fmt.Errorf("error listening on %s: %w", addr, err)
-
-// "failed" is fine in log messages
-log.Debugf("failed to parse remote address: %v", err)
-```
-
-Skip the wrapping when a function only extracts or delegates and the wrap would
-add nothing:
-
-```go
-func parseAddr(addr string) (string, int, error) {
-    host, portStr, err := net.SplitHostPort(addr)
-    if err != nil {
-        return "", 0, err
-    }
-    // ...
-}
-```
-
-Log the errors you choose not to act on:
-
-- `log.Debugf()` for errors that do not affect program flow but help debugging.
-- `log.Tracef()` for very verbose errors that would otherwise spam logs.
-- **Never ignore** errors from writes, network sends, or critical cleanup.
-- Close errors may be ignored for read-only operations; log them at debug for
-  writes.
-
-## Comments
-
-Comment the **why**, never the **what**. Default to no comment, and add one only
-when a hidden constraint or workaround would surprise a future reader. Never
-reference the current task, PR, or your own changes in a comment.
-
-```go
-// Bad - trailing comments explaining the obvious
-defer localConn.Close() // Close the connection
-if err != nil {         // Check if error occurred
-
-// Good
-defer localConn.Close()
-
-// Good - explains a non-obvious constraint
-// Use incremental checksum update per RFC 1624 for performance.
-checksum = updateChecksum(checksum, oldPort, newPort)
-```
-
-### Length budget
-
-- **90 characters per line.** Wrap the comment, do not run past it.
-- **250 characters per comment**, roughly three wrapped lines. Doc comments on
-  exported identifiers may exceed it when the API genuinely needs the
-  explanation; inline comments inside a function body may not.
-
-The budget is a smell detector, not a rule to game. Do not compress a needed
-explanation into cryptic shorthand to fit — if a block of code needs more than
-250 characters of prose, the code is doing too much. Fix the code:
-
-- **Extract a named function.** A well-named function replaces the comment: the
-  name says *what*, the body shows *how*, and the comment you no longer write
-  was the *what* anyway. Clean Code calls this "explain yourself in code".
-- **Extract a named constant or predicate.** `if isExpiredSetupKey(key)` needs
-  no comment; `if key.ExpiresAt.Before(now) && !key.Revoked && key.UsageLimit > 0`
-  does.
-- **Keep the surviving comment for the why** — the RFC, the kernel quirk, the
-  ordering constraint. That part is usually one or two lines.
-
-### Long switch and if/else chains
-
-A `switch` whose cases carry multi-line explanations is the usual place this
-budget is breached, and the comment is a symptom. In order of preference:
-
-1. **Extract each case body into a named function.** The case becomes one line,
-   the name carries the meaning, and the switch reads as a table of contents.
-2. **Replace the switch with a lookup table** — `map[Kind]handlerFunc` — when the
-   branches are uniform. Adding a case stops meaning editing a growing function.
-3. **Replace conditional with polymorphism** when branches vary by type and the
-   same switch shape starts appearing in more than one place. Clean Code's rule
-   of thumb: tolerate a switch statement if it appears **once**, is buried in a
-   factory that returns an interface, and no other switch dispatches on the same
-   type. A second switch over the same enum is the signal to introduce the
-   interface.
-
-Do not restructure a switch purely to satisfy the budget when the cases are one
-line each and self-evident — a flat, boring `switch` over an enum is fine and
-needs no comments at all.
-
-Explanatory comments in tests are welcome — they document the scenario being set
-up, and the 250-character budget does not apply to them.
-
-## Testing
-
-- **Unit tests** live beside the code as `_test.go`. `make test-unit` runs the
-  host-safe set with `-tags devcert` and no sudo.
-- **Privileged tests** carry the `privileged` build tag and mutate host
-  networking. They run through `make test-privileged`, inside a Docker container
-  with `NET_ADMIN`. Never bypass that harness by running them directly on the
-  host.
-- **End-to-end suites** live in `e2e/` with a shared harness.
-- **Test real behavior, not API existence.** Assert on the observable end state
-  a consumer would see — bytes that arrived, the packet after translation, the
-  row after the write — not merely that a method exists or returns an error.
-- **Avoid mocks for code we own.** Exercise the real store, manager, or
-  controller and assert what the caller actually receives.
-- **`require` for setup and preconditions, `assert` for the conditions under
-  test.** Use `require` whenever a later line would panic or be meaningless
-  otherwise.
-- **Message guidance:** optional for `NoError`/`Error`; always give context for
-  comparison, boolean, and collection assertions.
-
-```go
-server, err := StartTestServer()
-require.NoError(t, err, "Test server setup must succeed")
-defer server.Close()
-
-result, err := client.DoOperation()
-assert.NoError(t, err)
-assert.Equal(t, expectedResult, result, "Result should match expected")
-```
-
-## Pitfalls
-
-- **The agent runs as root.** Anything touching routing, firewall, DNS, or the
-  interface can take a user's machine off the network. Prefer a reversible
-  change and make sure cleanup runs on every exit path.
-- **Management has two account loaders** (GORM and pgx). Adding a relation to an
-  account often means updating both, or it silently comes back empty in
-  production.
-- **`go test ./...` without `-tags devcert` skips tests** that need the
-  development certificate. Use `make test-unit`.
-- **`make lint` only checks the diff against `origin/main`.** CI runs
-  `make lint-all`; run it too before pushing a large change.
-- **Protos are consumed by released clients.** An old agent must keep working
-  against a new Management, so fields are added, never renumbered or removed.
-- **Windows requires the wintun driver**, and the daemon serves a named pipe
-  (`npipe://netbird`) rather than loopback TCP. Loopback TCP carries no caller
-  identity, so privileged operations are refused over it.
-
-## Commits, PRs, releases
-
-- **PR titles must start with a bracketed tag.** Before you propose a title,
-  **read [`.github/workflows/pr-title-check.yml`](.github/workflows/pr-title-check.yml)
-  and take the allowed tags from the `allowedTags` array in that file.** It is
-  the only source of truth, it changes as components are added, and the check
-  runs on every title edit — a tag that is not in that array is a red build. Do
-  not rely on a list memorized from anywhere else, including this file.
-
-  ```text
-  [client] Authorize daemon IPC callers by their local identity
-  [management,client] Add MDM policy support
-  ```
-
-  Multiple tags are comma-separated inside one pair of brackets. Match the tag
-  to the component you actually changed, not to the one you read the most.
-
-- **Use the repository's PR template.** Fill in
-  [`.github/pull_request_template.md`](.github/pull_request_template.md) rather
-  than replacing it with your own summary: describe the change, link the issue,
-  tick the checklist honestly (including "ran locally" and "single purpose"),
-  and complete the documentation section. Do not tick a box you have not
-  verified, and do not delete rows that do not apply.
-
-- **Keep the PR description short.** Under 1000 words on top of the template's
-  own text, and usually far less — a few paragraphs. Reviewers read the diff;
-  the description exists to explain what the diff cannot say for itself. This is
-  well below what an agent will produce by default, so cut before you post.
-
-- **Body: why before what.** Lead with the problem and the reason for this
-  approach, then the shape of the change. No bullet list of files changed, no
-  per-function walkthrough, no restating the diff in prose, no trailing summary
-  section, no self-congratulatory closing line.
-
-- **No `Co-Authored-By` or tool-attribution trailers in the PR description**,
-  and none in commits either. Contributors own their contributions. Whatever
-  tooling produced the diff, the person opening the PR is its author: they have
-  read every line, they can explain why it works, they can answer review
-  questions without going back to a model, and they are accountable for the
-  consequences of merging it. Do not add a trailer, footer, or description line
-  that spreads that ownership onto a tool.
-
-- **Commit subjects follow the same `[scope] Subject` convention.** Keep the
-  subject short, and use the body for why before what. No bullet lists of files
-  changed.
-
-- **Push review fixes as separate commits.** The PR is squashed on merge, so
-  there is no reason to rewrite history mid-review; many small commits make the
-  re-review readable.
-
-- **Do not force-push a branch that is under review.** A force-push detaches
-  existing review comments from the lines they were written against, destroys
-  the "changes since your last review" diff a reviewer relies on, and discards
-  the CI history that showed which commit broke what. Add commits instead —
-  including for fixups and reverts. Force-push only when there is no
-  alternative: a rebase to clear a genuine conflict, or removing a secret or a
-  large binary that was committed by mistake. When you must, ask the user first,
-  then say so in a PR comment so reviewers know their anchors moved. Never
-  force-push `main`, and never force-push a branch you do not own.
-
-- **One PR, one purpose.** Split refactors out of fixes and fixes out of
-  features.
-
-- **Keep the PR small.** Size is the single strongest predictor of how long a PR
-  waits. Aim for **under ~400 changed lines across under ~20 files**; past
-  roughly **1000 lines or 50 files** a community PR is likely to be sent back to
-  be split, or left unreviewed until it is. Large PRs from outside the core team
-  may be blocked outright when the size was never agreed in the ticket —
-  reviewing a sprawling change against a privileged networking daemon is a
-  security risk in itself, not just a time cost.
-
-  Judge the size by hand-written code: exclude generated output, `go.sum`,
-  vendored files, and test fixtures from the estimate, but do not use their
-  presence to argue a 3000-line PR is small.
-
-  When a change genuinely cannot be small — a protocol migration, a
-  cross-component rename — agree the split in the ticket **before** writing
-  code, and land it as a sequence of PRs that each build, test, and make sense
-  on their own. Propose that split to the user rather than opening one large PR
-  and hoping.
-
-- **User-facing changes need a docs PR** in
-  [netbirdio/docs](https://github.com/netbirdio/docs), linked from the PR
-  description.
-
-## After you push: CI and review bots
-
-Opening the PR is not the end of the task. Watch the run, read what the bots
-say, and drive the PR to green before you report the work as done.
-
-```bash
-gh pr checks <pr>            --watch    # all checks, live
-gh run view <run-id> --log-failed       # only the failing steps
-gh pr view <pr> --comments              # bot and human review comments
-```
-
-**Never report a change as finished while checks are pending or red**, and never
-describe a red PR as passing. If you ran out of turn before CI finished, say
-which checks were still running.
-
-### The checks
-
-- **Go tests** — `golang-test-{linux,darwin,windows,freebsd}.yml`, sharded per
-  component. A failure in a component you did not touch is usually a real
-  interaction, not noise; read the log before assuming flake.
-- **golangci-lint** — `golangci-lint.yml` runs the full repository, while
-  `make lint` only checks your diff. A clean local lint does not guarantee green
-  CI on a large change.
-- **PR Title Check** — `pr-title-check.yml`, see above.
-- **Codecov** — uploaded from the Linux test workflow with per-component flags
-  (`unit,client`, `unit,management`, `unit,relay`, `unit,proxy`, `unit,signal`,
-  `integration,management`). Coverage on new code should not go backwards. Add
-  tests for the paths you introduced; do not adjust thresholds or exclude files
-  to clear the report.
-- **CodeRabbit** — configured in [`.coderabbit.yaml`](.coderabbit.yaml): `chill`
-  profile, auto-review on every non-draft PR, TypeScript/JavaScript/SVG paths
-  filtered out. Chat auto-reply is on, so `@coderabbitai` in a comment reaches
-  it.
-- **SonarCloud** — project `netbirdio_netbird`, quality gate on new code (bugs,
-  vulnerabilities, code smells, duplication, coverage).
-- **Snyk** — dependency and code scanning.
-
-Sonar and Snyk report as GitHub App checks rather than workflows in this
-repository, so their detail lives on the PR check, not in the Actions logs.
-
-### Handling bot findings
-
-- **Read every comment and act on it.** Either fix it, or reply with the reason
-  it does not apply. Do not bulk-resolve threads to clear the count, and do not
-  silently ignore a finding because the check is advisory.
-- **Bots are frequently wrong here.** NetBird has privileged, platform-specific,
-  and concurrency-heavy code that static analysis reads poorly. A confident
-  CodeRabbit or Sonar comment can still be nonsense. Verify the claim against
-  the code before you change anything — never edit correct code just to silence
-  a bot.
-- **Security findings get the opposite default.** For a Snyk or Sonar
-  vulnerability, or a CodeRabbit comment about authentication, authorization,
-  certificate verification, or key handling, assume it is real until you have
-  disproved it. Surface it to the user rather than dismissing it yourself.
-- **A new vulnerable dependency is a stop.** Bumping or replacing dependencies
-  needs the user's decision, as above.
-- **Never change a workflow, threshold, lint exclusion, or bot config to make a
-  check pass.** If a check is genuinely wrong, say so and let the user decide.
-- **Do not paper over flakes with blind re-runs.** Identify the failure first. If
-  it is a known flake, name it; if you cannot tell, report it as unresolved
-  rather than re-running until it goes green.
-
-## Discussion and support
-
-- Discussions: <https://github.com/netbirdio/netbird/discussions>
-- Slack: <https://docs.netbird.io/slack-url>
-- Docs: <https://docs.netbird.io>
-- Security: <https://github.com/netbirdio/netbird/security/policy> — never in public
-- Contribution process: [CONTRIBUTING.md](CONTRIBUTING.md)

CLAUDE.md

@@ -1 +0,0 @@
-See [AGENTS.md](AGENTS.md) for the agent guidelines in this repository.

CONTRIBUTING.md

@@ -66,41 +66,11 @@ Typical bug fixes, internal refactors, documentation updates, and tests do not
 need a design discussion, but should still be tied to an issue so the work is
 visible and nobody duplicates it.
 
-### Using AI coding agents
-
-We have no policy for or against using an AI agent to write NetBird code. That
-choice is yours, and we are not going to interrogate anyone about their tools.
-
-What we do have is a lot of incoming contributions that were plainly drafted with
-one, and enough experience reviewing them to see the same avoidable problems
-again and again: no ticket behind the change, a diff far too large to review, a
-description longer than the code it describes, an approach that was never going
-to be accepted, and an author who cannot answer questions about their own PR.
-None of that is caused by the tooling — it is what happens when a tool is pointed
-at a repository whose expectations it has never been told.
-
-So rather than a rule, there is a guide. [AGENTS.md](AGENTS.md) restates the
-expectations from this document in the form agents read automatically
-(`CLAUDE.md` points to it), so pointing your tool at the repository is usually
-enough. Among other things it tells the agent to ask you for the
-discussion or issue before drafting a PR, to keep the change small and
-single-purpose, to run the tests locally, to use this repository's PR template
-and title tags, and to write a description a reviewer can get through.
-
-The guardrails are the point, and they are the same ones we apply to everyone: an
-agreed ticket, a change you have actually run, a diff small enough to review with
-care, and an author who can explain it. Whatever wrote the diff, you are its
-author — you own every line you submit and the consequences of opening a PR with it.
-
-We may assess whether a contribution is maintainable and whether its merged code
-aligns with our security standards and design expectations.
-
 ## Contents
 
 - [Contributing to NetBird](#contributing-to-netbird)
     - [Ticket first, PR second](#ticket-first-pr-second)
         - [High-risk areas](#high-risk-areas)
-        - [Using AI coding agents](#using-ai-coding-agents)
     - [Contents](#contents)
     - [Code of conduct](#code-of-conduct)
     - [Directory structure](#directory-structure)

client/internal/dns/interface_index.go

@@ -1,15 +0,0 @@
-package dns
-
-import (
-	"fmt"
-	"net"
-)
-
-func getInterfaceIndex(interfaceName string) (int, error) {
-	iface, err := net.InterfaceByName(interfaceName)
-	if err != nil {
-		return 0, fmt.Errorf("lookup interface %q: %w", interfaceName, err)
-	}
-
-	return iface.Index, nil
-}

client/internal/dns/interface_index_test.go

@@ -1,35 +0,0 @@
-package dns
-
-import (
-	"net"
-	"testing"
-)
-
-func TestGetInterfaceIndexExisting(t *testing.T) {
-	interfaces, err := net.Interfaces()
-	if err != nil {
-		t.Fatalf("list network interfaces: %v", err)
-	}
-	if len(interfaces) == 0 {
-		t.Fatal("expected at least one network interface")
-	}
-
-	iface := interfaces[0]
-	index, err := getInterfaceIndex(iface.Name)
-	if err != nil {
-		t.Fatalf("look up existing interface %q: %v", iface.Name, err)
-	}
-	if index != iface.Index {
-		t.Fatalf("expected interface index %d, got %d", iface.Index, index)
-	}
-}
-
-func TestGetInterfaceIndexMissing(t *testing.T) {
-	index, err := getInterfaceIndex("netbird-interface-that-does-not-exist")
-	if index != 0 {
-		t.Fatalf("expected missing interface index to be 0, got %d", index)
-	}
-	if err == nil {
-		t.Fatal("expected missing interface lookup to return an error")
-	}
-}

client/internal/dns/upstream_ios.go

@@ -130,3 +130,8 @@ func GetClientPrivate(iface privateClientIface, upstreamIP netip.Addr, dialTimeo
 	}
 	return client, nil
 }
+
+func getInterfaceIndex(interfaceName string) (int, error) {
+	iface, err := net.InterfaceByName(interfaceName)
+	return iface.Index, err
+}

client/ios/NetBirdSDK/client.go

@@ -158,19 +158,13 @@ func (c *Client) Run(fd int32, interfaceName string, envList *EnvList) error {
 	defer c.ctxCancel()
 	c.ctxCancelLock.Unlock()
 
-	// No login pre-flight here. The engine's own loginToManagement (connect.go) performs
-	// the authoritative Login immediately before the first Sync, so a LoginSync() call at
-	// this point only duplicated it — costing two extra Login RPCs (IsLoginRequired +
-	// Login) on every engine start, since IsLoginRequired is itself a full Login RPC.
-	//
-	// Auth failures still reach the caller through the engine path: loginToManagement
-	// returns PermissionDenied, which marks the shared status recorder
-	// (MarkManagementDisconnected) and fires ClientStop → onDisconnected, where
-	// IsLoginRequiredCached() reports login-required. The error is also returned out of Run().
-	//
-	// A pre-flight was also actively harmful when the server is unreachable: its 2-minute
-	// backoff blocked the start and then reported "login required" for what was really a
-	// timeout. The engine instead keeps retrying and recovers when the server returns.
+	auth := NewAuthWithConfig(ctx, cfg)
+	err = auth.LoginSync()
+	if err != nil {
+		return err
+	}
+
+	log.Infof("Auth successful")
 	// todo do not throw error in case of cancelled context
 	ctx = internal.CtxInitState(ctx)
 	c.onHostDnsFn = func([]string) {}

client/ios/NetBirdSDK/login.go

@@ -222,36 +222,17 @@ func (a *Auth) Login(resultListener ErrListener, urlOpener URLOpener, forceDevic
 // LoginWithDeviceName performs interactive login with device authentication support
 // The deviceName parameter allows specifying a custom device name (required for tvOS)
 func (a *Auth) LoginWithDeviceName(resultListener ErrListener, urlOpener URLOpener, forceDeviceAuth bool, deviceName string) {
-	a.startLogin(resultListener, urlOpener, forceDeviceAuth, deviceName, false)
-}
-
-// LoginInteractive performs the same interactive login as LoginWithDeviceName but skips the
-// IsLoginRequired() pre-flight and goes straight to the browser / device-code flow.
-//
-// IsLoginRequired() is itself a full Login RPC against the management server, so when the
-// caller has ALREADY established that login is required it is a pure duplicate. On iOS the
-// main app decides to show the browser based on its own isLoginRequired() check and then
-// calls straight into this method, so re-asking the server would add another Login RPC to
-// every interactive login.
-//
-// Use LoginWithDeviceName when the auth state is unknown and a silent (browser-less) login
-// must still be possible; use this when the browser is going to be shown regardless.
-func (a *Auth) LoginInteractive(resultListener ErrListener, urlOpener URLOpener, forceDeviceAuth bool, deviceName string) {
-	a.startLogin(resultListener, urlOpener, forceDeviceAuth, deviceName, true)
-}
-
-func (a *Auth) startLogin(resultListener ErrListener, urlOpener URLOpener, forceDeviceAuth bool, deviceName string, skipLoginCheck bool) {
 	if resultListener == nil {
-		log.Errorf("startLogin: resultListener is nil")
+		log.Errorf("LoginWithDeviceName: resultListener is nil")
 		return
 	}
 	if urlOpener == nil {
-		log.Errorf("startLogin: urlOpener is nil")
+		log.Errorf("LoginWithDeviceName: urlOpener is nil")
 		resultListener.OnError(fmt.Errorf("urlOpener is nil"))
 		return
 	}
 	go func() {
-		err := a.login(urlOpener, forceDeviceAuth, deviceName, skipLoginCheck)
+		err := a.login(urlOpener, forceDeviceAuth, deviceName)
 		if err != nil {
 			resultListener.OnError(err)
 		} else {
@@ -260,7 +241,7 @@ func (a *Auth) startLogin(resultListener ErrListener, urlOpener URLOpener, force
 	}()
 }
 
-func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName string, skipLoginCheck bool) error {
+func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName string) error {
 	// Create context with device name if provided
 	ctx := a.ctx
 	if deviceName != "" {
@@ -274,13 +255,10 @@ func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName strin
 	}
 	defer authClient.Close()
 
-	// check if we need to generate JWT token (skipped when the caller already knows)
-	needsLogin := true
-	if !skipLoginCheck {
-		needsLogin, err = authClient.IsLoginRequired(ctx)
-		if err != nil {
-			return fmt.Errorf("failed to check login requirement: %v", err)
-		}
+	// check if we need to generate JWT token
+	needsLogin, err := authClient.IsLoginRequired(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to check login requirement: %v", err)
 	}
 
 	jwtToken := ""

management/internals/modules/agentnetwork/manager.go

@@ -157,14 +157,14 @@ func NewManager(
 }
 
 func (m *managerImpl) GetAllProviders(ctx context.Context, accountID, userID string) ([]*types.Provider, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, accountID)
 }
 
 func (m *managerImpl) GetProvider(ctx context.Context, accountID, userID, providerID string) (*types.Provider, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAgentNetworkProviderByID(ctx, store.LockingStrengthNone, accountID, providerID)
@@ -175,7 +175,7 @@ func (m *managerImpl) GetProvider(ctx context.Context, accountID, userID, provid
 // been created yet; otherwise it is ignored (the cluster is pinned on
 // Settings and every provider in the account routes through it).
 func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provider *types.Provider, bootstrapCluster string) (*types.Provider, error) {
-	if err := m.requirePermission(ctx, provider.AccountID, userID, modules.AgentNetworkProviders, operations.Create); err != nil {
+	if err := m.requirePermission(ctx, provider.AccountID, userID, operations.Create); err != nil {
 		return nil, err
 	}
 
@@ -218,7 +218,7 @@ func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provide
 }
 
 func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provider *types.Provider) (*types.Provider, error) {
-	if err := m.requirePermission(ctx, provider.AccountID, userID, modules.AgentNetworkProviders, operations.Update); err != nil {
+	if err := m.requirePermission(ctx, provider.AccountID, userID, operations.Update); err != nil {
 		return nil, err
 	}
 
@@ -257,7 +257,7 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide
 }
 
 func (m *managerImpl) DeleteProvider(ctx context.Context, accountID, userID, providerID string) error {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Delete); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil {
 		return err
 	}
 
@@ -306,21 +306,21 @@ func pluralize(n int, singular, plural string) string {
 }
 
 func (m *managerImpl) GetAllPolicies(ctx context.Context, accountID, userID string) ([]*types.Policy, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkPolicies, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAccountAgentNetworkPolicies(ctx, store.LockingStrengthNone, accountID)
 }
 
 func (m *managerImpl) GetPolicy(ctx context.Context, accountID, userID, policyID string) (*types.Policy, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkPolicies, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAgentNetworkPolicyByID(ctx, store.LockingStrengthNone, accountID, policyID)
 }
 
 func (m *managerImpl) CreatePolicy(ctx context.Context, userID string, policy *types.Policy) (*types.Policy, error) {
-	if err := m.requirePermission(ctx, policy.AccountID, userID, modules.AgentNetworkPolicies, operations.Create); err != nil {
+	if err := m.requirePermission(ctx, policy.AccountID, userID, operations.Create); err != nil {
 		return nil, err
 	}
 
@@ -346,7 +346,7 @@ func (m *managerImpl) CreatePolicy(ctx context.Context, userID string, policy *t
 }
 
 func (m *managerImpl) UpdatePolicy(ctx context.Context, userID string, policy *types.Policy) (*types.Policy, error) {
-	if err := m.requirePermission(ctx, policy.AccountID, userID, modules.AgentNetworkPolicies, operations.Update); err != nil {
+	if err := m.requirePermission(ctx, policy.AccountID, userID, operations.Update); err != nil {
 		return nil, err
 	}
 
@@ -373,7 +373,7 @@ func (m *managerImpl) UpdatePolicy(ctx context.Context, userID string, policy *t
 }
 
 func (m *managerImpl) DeletePolicy(ctx context.Context, accountID, userID, policyID string) error {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkPolicies, operations.Delete); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil {
 		return err
 	}
 
@@ -393,21 +393,21 @@ func (m *managerImpl) DeletePolicy(ctx context.Context, accountID, userID, polic
 }
 
 func (m *managerImpl) GetAllGuardrails(ctx context.Context, accountID, userID string) ([]*types.Guardrail, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkGuardrails, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAccountAgentNetworkGuardrails(ctx, store.LockingStrengthNone, accountID)
 }
 
 func (m *managerImpl) GetGuardrail(ctx context.Context, accountID, userID, guardrailID string) (*types.Guardrail, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkGuardrails, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAgentNetworkGuardrailByID(ctx, store.LockingStrengthNone, accountID, guardrailID)
 }
 
 func (m *managerImpl) CreateGuardrail(ctx context.Context, userID string, guardrail *types.Guardrail) (*types.Guardrail, error) {
-	if err := m.requirePermission(ctx, guardrail.AccountID, userID, modules.AgentNetworkGuardrails, operations.Create); err != nil {
+	if err := m.requirePermission(ctx, guardrail.AccountID, userID, operations.Create); err != nil {
 		return nil, err
 	}
 
@@ -429,7 +429,7 @@ func (m *managerImpl) CreateGuardrail(ctx context.Context, userID string, guardr
 }
 
 func (m *managerImpl) UpdateGuardrail(ctx context.Context, userID string, guardrail *types.Guardrail) (*types.Guardrail, error) {
-	if err := m.requirePermission(ctx, guardrail.AccountID, userID, modules.AgentNetworkGuardrails, operations.Update); err != nil {
+	if err := m.requirePermission(ctx, guardrail.AccountID, userID, operations.Update); err != nil {
 		return nil, err
 	}
 
@@ -452,7 +452,7 @@ func (m *managerImpl) UpdateGuardrail(ctx context.Context, userID string, guardr
 }
 
 func (m *managerImpl) DeleteGuardrail(ctx context.Context, accountID, userID, guardrailID string) error {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkGuardrails, operations.Delete); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil {
 		return err
 	}
 
@@ -473,7 +473,7 @@ func (m *managerImpl) DeleteGuardrail(ctx context.Context, accountID, userID, gu
 
 // GetAllBudgetRules returns every account-level budget rule for the account.
 func (m *managerImpl) GetAllBudgetRules(ctx context.Context, accountID, userID string) ([]*types.AccountBudgetRule, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkBudgets, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAccountAgentNetworkBudgetRules(ctx, store.LockingStrengthNone, accountID)
@@ -481,7 +481,7 @@ func (m *managerImpl) GetAllBudgetRules(ctx context.Context, accountID, userID s
 
 // GetBudgetRule returns a single account-level budget rule.
 func (m *managerImpl) GetBudgetRule(ctx context.Context, accountID, userID, ruleID string) (*types.AccountBudgetRule, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkBudgets, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAgentNetworkBudgetRuleByID(ctx, store.LockingStrengthNone, accountID, ruleID)
@@ -491,7 +491,7 @@ func (m *managerImpl) GetBudgetRule(ctx context.Context, accountID, userID, rule
 // enforced at request time (CheckLLMPolicyLimits), not baked into the synth
 // proxy config, so no reconcile is needed.
 func (m *managerImpl) CreateBudgetRule(ctx context.Context, userID string, rule *types.AccountBudgetRule) (*types.AccountBudgetRule, error) {
-	if err := m.requirePermission(ctx, rule.AccountID, userID, modules.AgentNetworkBudgets, operations.Create); err != nil {
+	if err := m.requirePermission(ctx, rule.AccountID, userID, operations.Create); err != nil {
 		return nil, err
 	}
 
@@ -513,7 +513,7 @@ func (m *managerImpl) CreateBudgetRule(ctx context.Context, userID string, rule
 
 // UpdateBudgetRule updates an existing account-level budget rule.
 func (m *managerImpl) UpdateBudgetRule(ctx context.Context, userID string, rule *types.AccountBudgetRule) (*types.AccountBudgetRule, error) {
-	if err := m.requirePermission(ctx, rule.AccountID, userID, modules.AgentNetworkBudgets, operations.Update); err != nil {
+	if err := m.requirePermission(ctx, rule.AccountID, userID, operations.Update); err != nil {
 		return nil, err
 	}
 
@@ -536,7 +536,7 @@ func (m *managerImpl) UpdateBudgetRule(ctx context.Context, userID string, rule
 
 // DeleteBudgetRule removes an account-level budget rule.
 func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, ruleID string) error {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkBudgets, operations.Delete); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil {
 		return err
 	}
 
@@ -561,7 +561,7 @@ func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, r
 // gating, access-log emission), a reconcile is triggered so the proxy and peer
 // network maps converge on the new state.
 func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error) {
-	if err := m.requirePermission(ctx, settings.AccountID, userID, modules.AgentNetworkSettings, operations.Update); err != nil {
+	if err := m.requirePermission(ctx, settings.AccountID, userID, operations.Update); err != nil {
 		return nil, err
 	}
 
@@ -615,7 +615,7 @@ func (m *managerImpl) validateProviderRefs(ctx context.Context, accountID string
 // Returns the underlying status.NotFound when no row has been
 // bootstrapped yet (i.e. the account has no providers).
 func (m *managerImpl) GetSettings(ctx context.Context, accountID, userID string) (*types.Settings, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkSettings, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID)
@@ -685,7 +685,7 @@ func (m *managerImpl) bootstrapSettingsIfNeeded(ctx context.Context, accountID,
 // counter view; permission gate is the same Read role that gates
 // every other agent-network surface.
 func (m *managerImpl) ListConsumption(ctx context.Context, accountID, userID string) ([]*types.Consumption, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkUsage, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	return m.store.ListAgentNetworkConsumption(ctx, store.LockingStrengthNone, accountID)
@@ -694,7 +694,7 @@ func (m *managerImpl) ListConsumption(ctx context.Context, accountID, userID str
 // ListAccessLogs returns a paginated, server-side-filtered page of
 // agent-network access logs plus the total count matching the filter.
 func (m *managerImpl) ListAccessLogs(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter) ([]*types.AgentNetworkAccessLog, int64, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkLogs, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, 0, err
 	}
 	return m.store.GetAgentNetworkAccessLogs(ctx, store.LockingStrengthNone, accountID, filter)
@@ -704,7 +704,7 @@ func (m *managerImpl) ListAccessLogs(ctx context.Context, accountID, userID stri
 // agent-network access logs grouped by session, plus the total number of
 // sessions matching the filter.
 func (m *managerImpl) ListAccessLogSessions(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter) ([]*types.AgentNetworkAccessLogSession, int64, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkLogs, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, 0, err
 	}
 	return m.store.GetAgentNetworkAccessLogSessions(ctx, store.LockingStrengthNone, accountID, filter)
@@ -713,7 +713,7 @@ func (m *managerImpl) ListAccessLogSessions(ctx context.Context, accountID, user
 // GetUsageOverview returns the filtered usage rows aggregated into time buckets
 // at the requested granularity, oldest-first.
 func (m *managerImpl) GetUsageOverview(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter, granularity types.UsageGranularity) ([]*types.AgentNetworkUsageBucket, error) {
-	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkUsage, operations.Read); err != nil {
+	if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil {
 		return nil, err
 	}
 	rows, err := m.store.GetAgentNetworkUsageRows(ctx, store.LockingStrengthNone, accountID, filter)
@@ -787,8 +787,8 @@ func (m *managerImpl) RecordConsumption(ctx context.Context, accountID string, k
 	return m.store.IncrementAgentNetworkConsumption(ctx, accountID, kind, dimID, windowSeconds, windowStart, tokensIn, tokensOut, costUSD)
 }
 
-func (m *managerImpl) requirePermission(ctx context.Context, accountID, userID string, module modules.Module, op operations.Operation) error {
-	ok, _, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, module, op)
+func (m *managerImpl) requirePermission(ctx context.Context, accountID, userID string, op operations.Operation) error {
+	ok, _, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.AgentNetwork, op)
 	if err != nil {
 		return status.NewPermissionValidationError(err)
 	}

management/server/permissions/manager.go

@@ -82,9 +82,6 @@ func (m *managerImpl) ValidateUserPermissions(
 	return m.ValidateRoleModuleAccess(ctx, accountID, role, module, operation), ctxEnriched, nil
 }
 
-// ValidateRoleModuleAccess resolves an operation against the role's explicit
-// grant for the module, then the grant for its parent module when the module
-// is a dotted submodule, and finally the role's AutoAllowNew default.
 func (m *managerImpl) ValidateRoleModuleAccess(
 	ctx context.Context,
 	accountID string,
@@ -92,7 +89,7 @@ func (m *managerImpl) ValidateRoleModuleAccess(
 	module modules.Module,
 	operation operations.Operation,
 ) bool {
-	if permissions, ok := lookupModulePermissions(role, module); ok {
+	if permissions, ok := role.Permissions[module]; ok {
 		if allowed, exists := permissions[operation]; exists {
 			return allowed
 		}
@@ -103,21 +100,6 @@ func (m *managerImpl) ValidateRoleModuleAccess(
 	return role.AutoAllowNew[operation]
 }
 
-// lookupModulePermissions returns the role's explicit permission set for the
-// module, falling back to the parent module's set for dotted submodules. The
-// second return reports whether any explicit set was found.
-func lookupModulePermissions(role roles.RolePermissions, module modules.Module) (map[operations.Operation]bool, bool) {
-	if permissions, ok := role.Permissions[module]; ok {
-		return permissions, true
-	}
-	if parent, hasParent := module.Parent(); hasParent {
-		if permissions, ok := role.Permissions[parent]; ok {
-			return permissions, true
-		}
-	}
-	return nil, false
-}
-
 func (m *managerImpl) ValidateAccountAccess(ctx context.Context, accountID string, user *types.User, allowOwnerAndAdmin bool) (context.Context, error) {
 	if user.AccountID != accountID {
 		return ctx, status.NewUserNotPartOfAccountError()
@@ -137,7 +119,7 @@ func (m *managerImpl) GetPermissionsByRole(ctx context.Context, role types.UserR
 	permissions := roles.Permissions{}
 
 	for k := range modules.All {
-		if rolePermissions, ok := lookupModulePermissions(roleMap, k); ok {
+		if rolePermissions, ok := roleMap.Permissions[k]; ok {
 			permissions[k] = rolePermissions
 			continue
 		}

management/server/permissions/manager_test.go

@@ -1,139 +0,0 @@
-package permissions
-
-import (
-	"context"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"github.com/netbirdio/netbird/management/server/permissions/modules"
-	"github.com/netbirdio/netbird/management/server/permissions/operations"
-	"github.com/netbirdio/netbird/management/server/permissions/roles"
-	"github.com/netbirdio/netbird/management/server/types"
-)
-
-func TestValidateRoleModuleAccessSubmoduleCascade(t *testing.T) {
-	manager := NewManager(nil)
-	ctx := context.Background()
-
-	fullAccess := map[operations.Operation]bool{
-		operations.Read:   true,
-		operations.Create: true,
-		operations.Update: true,
-		operations.Delete: true,
-	}
-	readOnly := map[operations.Operation]bool{
-		operations.Read:   true,
-		operations.Create: false,
-		operations.Update: false,
-		operations.Delete: false,
-	}
-	denyAll := map[operations.Operation]bool{
-		operations.Read:   false,
-		operations.Create: false,
-		operations.Update: false,
-		operations.Delete: false,
-	}
-
-	t.Run("parent grant covers submodules", func(t *testing.T) {
-		role := roles.RolePermissions{
-			AutoAllowNew: denyAll,
-			Permissions:  roles.Permissions{modules.AgentNetwork: fullAccess},
-		}
-		assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkProviders, operations.Create),
-			"parent full grant should allow create on a submodule")
-		assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkLogs, operations.Read),
-			"parent full grant should allow read on a submodule")
-	})
-
-	t.Run("submodule grant does not leak to parent or siblings", func(t *testing.T) {
-		role := roles.RolePermissions{
-			AutoAllowNew: denyAll,
-			Permissions:  roles.Permissions{modules.AgentNetworkUsage: readOnly},
-		}
-		assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkUsage, operations.Read),
-			"explicit submodule read should be allowed")
-		assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkUsage, operations.Create),
-			"read-only submodule grant should not allow create")
-		assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetwork, operations.Read),
-			"submodule grant should not grant the parent module")
-		assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkProviders, operations.Read),
-			"submodule grant should not grant a sibling submodule")
-	})
-
-	t.Run("explicit submodule entry wins over parent grant", func(t *testing.T) {
-		role := roles.RolePermissions{
-			AutoAllowNew: denyAll,
-			Permissions: roles.Permissions{
-				modules.AgentNetwork:     fullAccess,
-				modules.AgentNetworkLogs: denyAll,
-			},
-		}
-		assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkLogs, operations.Read),
-			"explicit submodule deny should override the parent grant")
-		assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkUsage, operations.Read),
-			"sibling submodules should still resolve through the parent grant")
-	})
-
-	t.Run("auto allow applies when neither submodule nor parent is granted", func(t *testing.T) {
-		role := roles.RolePermissions{
-			AutoAllowNew: readOnly,
-		}
-		assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkProviders, operations.Read),
-			"auto-allow read should apply to submodules")
-		assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkProviders, operations.Delete),
-			"auto-allow should not grant unlisted operations")
-	})
-}
-
-// TestExistingRolesKeepAgentNetworkBehaviorOnSubmodules pins the behavior the
-// submodule split must not change: every built-in role resolves the new
-// submodules exactly as it resolved the agent_network module before.
-func TestExistingRolesKeepAgentNetworkBehaviorOnSubmodules(t *testing.T) {
-	manager := NewManager(nil)
-	ctx := context.Background()
-
-	submodules := []modules.Module{
-		modules.AgentNetworkProviders,
-		modules.AgentNetworkPolicies,
-		modules.AgentNetworkGuardrails,
-		modules.AgentNetworkBudgets,
-		modules.AgentNetworkUsage,
-		modules.AgentNetworkLogs,
-		modules.AgentNetworkSettings,
-	}
-	allOperations := []operations.Operation{operations.Read, operations.Create, operations.Update, operations.Delete}
-
-	for _, role := range []types.UserRole{types.UserRoleOwner, types.UserRoleAdmin, types.UserRoleAuditor, types.UserRoleNetworkAdmin, types.UserRoleUser} {
-		rolePermissions, ok := roles.RolesMap[role]
-		require.True(t, ok, "role %s must exist in RolesMap", role)
-
-		for _, sub := range submodules {
-			for _, op := range allOperations {
-				expected := manager.ValidateRoleModuleAccess(ctx, "account", rolePermissions, modules.AgentNetwork, op)
-				actual := manager.ValidateRoleModuleAccess(ctx, "account", rolePermissions, sub, op)
-				assert.Equal(t, expected, actual, "role %s: %s on %s should match the agent_network module", role, op, sub)
-			}
-		}
-	}
-}
-
-func TestGetPermissionsByRoleIncludesSubmodules(t *testing.T) {
-	manager := NewManager(nil)
-	ctx := context.Background()
-
-	permissions, err := manager.GetPermissionsByRole(ctx, types.UserRoleAuditor)
-	require.NoError(t, err, "auditor role must resolve")
-
-	usage, ok := permissions[modules.AgentNetworkUsage]
-	require.True(t, ok, "permissions map should contain the usage submodule")
-	assert.True(t, usage[operations.Read], "auditor should read the usage submodule")
-	assert.False(t, usage[operations.Update], "auditor should not update the usage submodule")
-
-	adminPermissions, err := manager.GetPermissionsByRole(ctx, types.UserRoleAdmin)
-	require.NoError(t, err, "admin role must resolve")
-	providers, ok := adminPermissions[modules.AgentNetworkProviders]
-	require.True(t, ok, "permissions map should contain the providers submodule")
-	assert.True(t, providers[operations.Delete], "admin should delete on the providers submodule")
-}

management/server/permissions/modules/module.go

@@ -1,7 +1,5 @@
 package modules
 
-import "strings"
-
 type Module string
 
 const (
@@ -22,17 +20,6 @@ const (
 	IdentityProviders Module = "identity_providers"
 	Services          Module = "services"
 	AgentNetwork      Module = "agent_network"
-
-	// Agent Network submodules. A role may grant one of these directly
-	// or grant the AgentNetwork parent, which covers all of them (see
-	// permissions.Manager cascade resolution).
-	AgentNetworkProviders  Module = "agent_network.providers"
-	AgentNetworkPolicies   Module = "agent_network.policies"
-	AgentNetworkGuardrails Module = "agent_network.guardrails"
-	AgentNetworkBudgets    Module = "agent_network.budgets"
-	AgentNetworkUsage      Module = "agent_network.usage"
-	AgentNetworkLogs       Module = "agent_network.logs"
-	AgentNetworkSettings   Module = "agent_network.settings"
 )
 
 var All = map[Module]struct{}{
@@ -53,21 +40,4 @@ var All = map[Module]struct{}{
 	IdentityProviders: {},
 	Services:          {},
 	AgentNetwork:      {},
-
-	AgentNetworkProviders:  {},
-	AgentNetworkPolicies:   {},
-	AgentNetworkGuardrails: {},
-	AgentNetworkBudgets:    {},
-	AgentNetworkUsage:      {},
-	AgentNetworkLogs:       {},
-	AgentNetworkSettings:   {},
-}
-
-// Parent returns the module owning a dotted submodule name and true, or the
-// module itself and false when it has no parent.
-func (m Module) Parent() (Module, bool) {
-	if i := strings.IndexByte(string(m), '.'); i > 0 {
-		return Module(string(m)[:i]), true
-	}
-	return m, false
 }

management/server/types/account.go

@@ -1019,7 +1019,12 @@ func (a *Account) GetPeerConnectionResources(ctx context.Context, peer *nbpeer.P
 				generateResources(rule, sourcePeers, FirewallRuleDirectionIN)
 			}
 
-			if peerInDestinations && rule.Protocol == PolicyRuleProtocolNetbirdSSH {
+			// Auth is collected when this peer serves the rule. For bidirectional
+			// rules the peer-in-sources side also serves inbound traffic, so it
+			// must be treated as a destination too.
+			peerServesAuth := peerInDestinations || (rule.Bidirectional && peerInSources)
+
+			if peerServesAuth && rule.Protocol == PolicyRuleProtocolNetbirdSSH {
 				sshEnabled = true
 				switch {
 				case len(rule.AuthorizedGroups) > 0:
@@ -1051,7 +1056,7 @@ func (a *Account) GetPeerConnectionResources(ctx context.Context, peer *nbpeer.P
 				default:
 					authorizedUsers[auth.Wildcard] = a.getAllowedUserIDs()
 				}
-			} else if peerInDestinations && PolicyRuleImpliesLegacySSH(rule) && peer.SSHEnabled {
+			} else if peerServesAuth && PolicyRuleImpliesLegacySSH(rule) && peer.SSHEnabled {
 				sshEnabled = true
 				authorizedUsers[auth.Wildcard] = a.getAllowedUserIDs()
 			}

management/server/types/account_components.go

@@ -479,7 +479,12 @@ func (a *Account) getPeersGroupsPoliciesRoutes(
 				for _, srcGroupID := range rule.Sources {
 					relevantGroupIDs[srcGroupID] = a.GetGroup(srcGroupID)
 				}
+			}
 
+			// SSH auth requirements are gathered whenever this peer serves
+			// the rule. For bidirectional rules the peer-in-sources side
+			// also serves inbound traffic and must be treated as a destination.
+			if peerInDestinations || (rule.Bidirectional && peerInSources) {
 				if rule.Protocol == PolicyRuleProtocolNetbirdSSH {
 					switch {
 					case len(rule.AuthorizedGroups) > 0:

management/server/types/networkmap_components_correctness_test.go

@@ -983,6 +983,44 @@ func TestComponents_SSHAuthorizedUsersContent(t *testing.T) {
 	assert.True(t, hasRoot || hasAdmin, "AuthorizedUsers should contain 'root' or 'admin' machine user mapping")
 }
 
+// TestComponents_SSHAuthorizedUsersBidirectionalSource verifies that a peer
+// on the sources side of a bidirectional NetbirdSSH rule receives the rule's
+// authorized users. The reverse direction (destinations -> sources) makes
+// the source-side peer a destination too, so it must be able to authorize
+// inbound SSH from the rule's destinations.
+func TestComponents_SSHAuthorizedUsersBidirectionalSource(t *testing.T) {
+	account, validatedPeers := scalableTestAccountWithoutDefaultPolicy(20, 2)
+
+	account.Users["user-dev"] = &types.User{Id: "user-dev", Role: types.UserRoleUser, AccountID: "test-account", AutoGroups: []string{"ssh-users"}}
+	account.Groups["ssh-users"] = &types.Group{ID: "ssh-users", Name: "SSH Users", Peers: []string{}}
+
+	account.Policies = append(account.Policies, &types.Policy{
+		ID: "policy-ssh-bidir", Name: "Bidirectional SSH", Enabled: true, AccountID: "test-account",
+		Rules: []*types.PolicyRule{{
+			ID: "rule-ssh-bidir", Name: "SSH both ways", Enabled: true,
+			Action: types.PolicyTrafficActionAccept, Protocol: types.PolicyRuleProtocolNetbirdSSH,
+			Bidirectional: true,
+			Sources:       []string{"group-0"}, Destinations: []string{"group-1"},
+			AuthorizedGroups: map[string][]string{"ssh-users": {"root"}},
+		}},
+	})
+
+	nmSrc := componentsNetworkMap(account, "peer-0", validatedPeers)
+	require.NotNil(t, nmSrc)
+	assert.True(t, nmSrc.EnableSSH, "source-side peer of bidirectional SSH rule should have SSH enabled")
+	require.NotEmpty(t, nmSrc.AuthorizedUsers, "source-side peer should receive authorized users from bidirectional rule")
+	rootUsers, hasRoot := nmSrc.AuthorizedUsers["root"]
+	require.True(t, hasRoot, "source-side peer should map the 'root' local user")
+	_, hasDev := rootUsers["user-dev"]
+	assert.True(t, hasDev, "source-side peer should include 'user-dev' under 'root'")
+
+	nmDst := componentsNetworkMap(account, "peer-10", validatedPeers)
+	require.NotNil(t, nmDst)
+	assert.True(t, nmDst.EnableSSH, "destination-side peer should also have SSH enabled")
+	_, hasRoot = nmDst.AuthorizedUsers["root"]
+	assert.True(t, hasRoot, "destination-side peer should also map the 'root' local user")
+}
+
 // TestComponents_SSHLegacyImpliedSSH verifies that a non-SSH ALL protocol policy with
 // SSHEnabled peer implies legacy SSH access.
 func TestComponents_SSHLegacyImpliedSSH(t *testing.T) {

shared/management/types/networkmap_components.go

@@ -256,7 +256,12 @@ func (c *NetworkMapComponents) getPeerConnectionResources(targetPeerID string) (
 				generateResources(rule, sourcePeers, FirewallRuleDirectionIN)
 			}
 
-			if peerInDestinations && rule.Protocol == PolicyRuleProtocolNetbirdSSH {
+			// Auth is collected when this peer serves the rule. For bidirectional
+			// rules the peer-in-sources side also serves inbound traffic, so it
+			// must be treated as a destination too.
+			peerServesAuth := peerInDestinations || (rule.Bidirectional && peerInSources)
+
+			if peerServesAuth && rule.Protocol == PolicyRuleProtocolNetbirdSSH {
 				sshEnabled = true
 				switch {
 				case len(rule.AuthorizedGroups) > 0:
@@ -287,7 +292,7 @@ func (c *NetworkMapComponents) getPeerConnectionResources(targetPeerID string) (
 				default:
 					authorizedUsers[auth.Wildcard] = c.getAllowedUserIDs()
 				}
-			} else if peerInDestinations && PolicyRuleImpliesLegacySSH(rule) && targetPeer.SSHEnabled {
+			} else if peerServesAuth && PolicyRuleImpliesLegacySSH(rule) && targetPeer.SSHEnabled {
 				sshEnabled = true
 				authorizedUsers[auth.Wildcard] = c.getAllowedUserIDs()
 			}