{"owner":"netbirdio","repo":"netbird","hasSkills":true,"hasMcp":false,"mcpConfig":null,"found":["CLAUDE.md","AGENTS.md"],"skills":{"CLAUDE.md":"See [AGENTS.md](AGENTS.md) for the agent guidelines in this repository.\n","AGENTS.md":"# NetBird Agent Guidelines\n\n**NetBird** is an open source connectivity platform: a WireGuard®-based overlay\nnetwork with a control plane. The **agent** (`client/`) runs on user machines as\na privileged daemon and manages the WireGuard interface, routing, firewall, and\nDNS. **Management** (`management/`) is the control plane and REST/gRPC API,\n**Signal** (`signal/`) brokers peer handshakes, **Relay** (`relay/`) carries\ntraffic when a direct tunnel is impossible, and **Proxy** (`proxy/`) is the\nidentity-aware proxy behind Agent Network.\n\nThis file applies to the whole repository, and is the single source of truth for\nagent guidance here. `CLAUDE.md` is a one-line pointer to it — keep the guidance\nin this file, not duplicated there.\n\n## Contents\n\n- [STOP and ask the user before](#stop-and-ask-the-user-before)\n- [Quick reference](#quick-reference)\n- [Structure](#structure)\n- [Where to look](#where-to-look)\n- [Security](#security)\n- [Agent conventions](#agent-conventions)\n- [Repo-wide principles](#repo-wide-principles)\n- [Type safety](#type-safety)\n- [Concurrency and lifecycle](#concurrency-and-lifecycle)\n- [Error handling](#error-handling)\n- [Comments](#comments)\n- [Testing](#testing)\n- [Pitfalls](#pitfalls)\n- [Commits, PRs, releases](#commits-prs-releases)\n- [After you push: CI and review bots](#after-you-push-ci-and-review-bots)\n- [Discussion and support](#discussion-and-support)\n\n## STOP and ask the user before\n\n- **Opening a pull request for anything beyond a trivial fix, without an agreed\n  ticket.** Ask the user directly: *\"Is there a discussion or issue for this\n  change?\"* NetBird is discussion-first — community reports start in\n  [Discussions](https://github.com/netbirdio/netbird/discussions), DevRel\n  validates them, and only validated discussions become issues. A PR that\n  changes behavior with no linked issue may be closed on arrival. If there is no\n  ticket, offer to draft the discussion post **instead of** the PR, and wait for\n  the user's call. Only typos, broken links, documentation corrections, and\n  one-line fixes that already have an issue can skip this.\n- **Designing in any high-risk area** (see\n  [CONTRIBUTING.md](CONTRIBUTING.md#high-risk-areas)): public API and OpenAPI\n  schema, gRPC protos, behavior existing deployments would notice after an\n  upgrade, peer connectivity (ICE, NAT traversal, relay selection, WireGuard® or\n  Rosenpass key handling), client system integration (routing, firewall, DNS,\n  interface), authentication and authorization, CLI or service flags, config\n  file format, daemon IPC, store schema and migrations, or a new feature. The\n  design gets agreed in the ticket before code is written.\n- **Writing a store migration or changing a persisted model.** Migrations are\n  one-way in the field and both the GORM and pgx paths may need the change.\n- **Hand-editing generated code.** `*.pb.go`, `*.gen.go`, and mocks are outputs.\n  Edit the source (`.proto`, `openapi.yml`) and rerun the matching\n  `generate.sh`.\n- **Adding, removing, or bumping a dependency**, and never vendor a fork.\n- **Weakening a security control** — authentication, authorization, certificate\n  verification, privilege dropping, or peer identity checks — even when it is\n  the fastest way to make a test pass.\n- **Force-pushing to `main`**, force-pushing any branch that is already under\n  review, amending pushed commits, or bypassing hooks with `--no-verify`.\n\n## Quick reference\n\n```bash\n# Build\ngo build ./...\ncd client && CGO_ENABLED=0 go build .        # agent\ncd management && go build .                  # management service\ncd signal && go build .                      # signal service\n\n# Verify (run before every push)\ngo fmt ./...\nmake lint            # golangci-lint on files changed vs origin/main (also the pre-push hook)\nmake lint-all        # full-repository lint, matches CI\nmake test-unit       # host-safe unit tests, -tags devcert, no sudo\nmake test-privileged # privileged-tagged suite in a Docker container with NET_ADMIN\nmake setup-hooks     # wire make lint into .githooks/pre-push\n\n# Narrow runs\ngo test ./client/internal/dns/...\ngo test -race -run TestPeerConn ./client/internal/peer/...\nPRIV_RUN=TestNftablesManager PRIV_PKGS=./client/firewall/nftables/... make test-privileged\n\n# Code generation (never hand-edit the output)\n./shared/management/http/api/generate.sh   # REST types from openapi.yml\n./shared/management/proto/generate.sh\n./shared/signal/proto/generate.sh\n./client/proto/generate.sh\n./flow/proto/generate.sh\n\n# Run locally (lab only, never on a machine you rely on)\nsudo ./client/netbird up --log-level debug --log-file console\nsudo ./client/netbird down                   # teardown: restores routing, firewall, DNS\n./signal/signal run --log-level debug --log-file console\n./management/management management --log-level debug --log-file console --config ./management.json\n```\n\n`netbird up` needs root and rewrites the host's routing table, firewall rules,\nDNS configuration, and WireGuard® interface. Run it only in a disposable test\nenvironment (a VM, container, or throwaway host) that you can rebuild, never on\na workstation or server whose connectivity matters. Run `sudo netbird down`\nbefore you stop working, before rebuilding the binary, and on every failure\npath, so the host's networking state is restored instead of left half-applied.\nSee [Pitfalls](#pitfalls) for why cleanup on every exit path matters.\n\n## Structure\n\n```text\nnetbird/\n├── client/              NetBird agent\n│   ├── cmd/             agent CLI\n│   ├── internal/        agent business logic (engine, peer, dns, routemanager, ...)\n│   ├── server/          daemon for background execution\n│   ├── proto/           daemon gRPC protos\n│   ├── iface/           WireGuard® interface management\n│   ├── firewall/        nftables, iptables, pf, WFP, userspace backends\n│   ├── ssh/             built-in SSH server and client\n│   ├── ui/              desktop UI (Wails v3 + React)\n│   ├── android/, ios/   mobile bindings\n│   ├── wasm/            WebAssembly build\n│   └── mdm/, system/    MDM policy, host information\n├── management/          control plane\n│   └── server/          account, peer, groups, networks, posture, permissions,\n│                        settings, store, http (REST), idp, integrations, migration\n├── signal/              handshake broker (peer/, server/)\n├── relay/               relay service (protocol/, server/, healthcheck/)\n├── proxy/               identity-aware proxy (llm/, acme/, accesslog/, middleware/, tcp/, udp/)\n├── agent-network/       Agent Network overview\n├── shared/              imported by both agent and services\n│   ├── management/      proto/, client/, http/api (OpenAPI + generated types)\n│   ├── signal/          proto/, client/\n│   └── relay/, auth/, sshauth/, metrics/\n├── e2e/                 end-to-end suites and harness\n├── encryption/, dns/, route/, stun/, sharedsock/, util/, flow/\n├── infrastructure_files/  docker compose and getting-started templates\n└── release_files/       files packaged into releases\n```\n\n## Where to look\n\n| Task                        | Location                                                     |\n| --------------------------- | ------------------------------------------------------------ |\n| REST API / OpenAPI          | `shared/management/http/api/` + `management/server/http/`    |\n| Management gRPC protocol    | `shared/management/proto/`                                   |\n| Signal protocol             | `shared/signal/proto/`                                       |\n| Daemon IPC protocol         | `client/proto/`                                              |\n| Peer connection and NAT     | `client/internal/peer/`                                      |\n| Network map handling        | `client/internal/engine.go`, `shared/management/networkmap/` |\n| Routing                     | `client/internal/routemanager/`, `route/`                    |\n| Firewall backends           | `client/firewall/`                                           |\n| DNS                         | `client/internal/dns/`, `dns/`                               |\n| WireGuard® interface        | `client/iface/`                                              |\n| Persistence and migrations  | `management/server/store/`, `management/server/migration/`   |\n| IdP integrations            | `management/server/idp/`                                     |\n| Permissions model           | `management/server/permissions/`                             |\n| LLM routing / Agent Network | `proxy/internal/llm/`, `agent-network/`                      |\n| End-to-end tests            | `e2e/`                                                       |\n\n## Security\n\n### Never fail open\n\nWhen a security check — access control, an IP restriction, an auth decision —\nhits an error such as an unparseable value, an unavailable lookup, or a state it\ndoes not recognize, it must **deny**. Never skip the check or allow the request\nthrough because the check itself failed, and make the `default` and unknown cases\nof a security-related `switch` deny rather than fall through.\n\n### Daemon RPC input is untrusted\n\nThe agent runs as root (LocalSystem on Windows), so a daemon RPC crosses a\nprivilege boundary: treat every field as untrusted input rather than as something\nthe UI or CLI validated on the way in.\n\nWhen you add or change an RPC, ask what the handler does with caller input while\nrunning as root. If the answer touches a filesystem path, a URL or host, or a\nprivileged state change, it needs a gate **in the handler** — a check in the client\nthat normally calls it is not a check at all.\n\n- **A caller-supplied path the daemon opens.** Never `os.Open` it as root.\n  Constrain it, then open it *as the caller* with `ipcauth.OpenOwnedFile`, which\n  opens `O_NOFOLLOW`, requires a regular file, and refuses a file the caller does\n  not own — so a symlink or hardlink aimed at a root-only file is rejected.\n- **A caller-supplied URL or host the daemon fetches.** Restrict the scheme and\n  allow only known hosts for unprivileged callers. Prefer a lexical host\n  allowlist plus TLS verification over \"resolve the host, then reject private\n  IPs\": the resolve-then-trust pattern has a DNS-rebinding race (public IP at\n  check time, attacker IP at connect time), while a name allowlist has no IP\n  check to race. Never accept `http://` where `https://` is expected.\n- **A privileged state change** (SSH root login, management URL, deregistration)\n  gates on the caller identity from `ipcauth.CallerIdentity(ctx)`.\n\nCaller identity comes from the kernel — `SO_PEERCRED`, `LOCAL_PEERCRED`, or the\nnamed-pipe client token — and never from an RPC field. When\n`ipcauth.CallerIdentity` reports that it could not determine an identity, **deny**;\ndo not fall back to treating the caller as the transport peer.\n\n## Agent conventions\n\n### Three networking modes\n\nWhere packets actually flow depends on the mode the agent is running in. The\nthree are not interchangeable, so establish which one a change applies to — and\nwhat it should do in the other two — before you write it.\n\n- **kernel mode** (Linux only): in-kernel WireGuard®. The kernel handles both\n  peer-to-peer and routed traffic, and ACLs are iptables or nftables rules. The\n  client programs kernel facilities but never sees the traffic itself.\n- **userspace mode** (wireguard-go with a TUN): wireguard-go runs in-process. The\n  kernel handles peer-to-peer traffic once it leaves the TUN, while routed traffic\n  — exit nodes and network routes — goes through the userspace forwarder, which\n  terminates the connection and re-establishes it over OS sockets. Used on\n  platforms without kernel WireGuard® or when the user opts out.\n- **netstack mode**: wireguard-go in-process with no TUN and no kernel\n  networking. The forwarder does all routing by stitching userspace sockets, and\n  listeners such as the embedded SSH and DNS servers bind on a gVisor netstack.\n  Used where the process cannot create a TUN device, such as the embedded client\n  (`client/embed/`) and the WASM build.\n\n### The overlay interface is not \"WireGuard\"\n\nDo not put \"WireGuard\" in identifiers or comments unless the code is genuinely\ncoupled to WireGuard® specifically — a wireguard-go call, a handshake field, a\nkernel WireGuard® netlink attribute. For the interface, the host, peers, or\ntraffic in general, say \"the NetBird interface\", \"the interface\", or \"the overlay\".\nMost firewall, routing, and DNS code is transport-agnostic, so a WireGuard®\nreference there is simply inaccurate and rots as the transports change.\n\n### IPv6 is a soft feature\n\nThe IPv6 overlay is opt-in dual-stack, and capability can change at runtime. Treat\nit as soft rather than a requirement:\n\n- Gate local v6 paths on the interface accessor (`wgIface.Address().HasIPv6()`),\n  not on raw state fields, and skip the v6 path when the host has no v6 rather\n  than returning an error.\n- Treat an empty or unparseable peer v6 address as \"no v6 for that peer\" and skip\n  it, keeping the v4 path working.\n- Never let a missing v6 break v4. Fail-closed is for security checks; a\n  capability mismatch skips the v6 work and carries on.\n\n### Environment variables\n\nName the variable in a constant and parse booleans with `strconv.ParseBool` rather\nthan comparing strings inline, so an unexpected value is logged instead of\nsilently meaning false:\n\n```go\nconst EnvDisableFeature = \"NB_DISABLE_FEATURE\"\n\nfunc isDisabledByEnv() bool {\n    val := os.Getenv(EnvDisableFeature)\n    if val == \"\" {\n        return false\n    }\n    disabled, err := strconv.ParseBool(val)\n    if err != nil {\n        log.Warnf(\"failed to parse %s: %v\", EnvDisableFeature, err)\n        return false\n    }\n    return disabled\n}\n```\n\n### Validating against protocol specs\n\nWhen a change depends on what a protocol actually mandates, read the specification\ntext from the [IETF datatracker](https://datatracker.ietf.org/) rather than a\nsummary, and check that you have the current RFC — the widely cited one for a\nprotocol is often superseded. Cite the section, not just the document, so a\nreviewer can jump straight to the rule.\n\n## Repo-wide principles\n\n1. **Run `go fmt` on every modified Go file.** Formatting is not optional.\n2. **Zero unaddressed linter warnings.** Fix what `golangci-lint` reports on code\n   you touch, and delete imports, helpers, and parameters your refactor orphaned.\n   Exception: unused parameters in shared code may be consumed by builds outside\n   this repository — do not remove them, ask instead.\n3. **Function comments are mandatory for exported functions**, written as full\n   sentences with a period, starting with the identifier name.\n4. **Prefer private functions and constants.** Export only what a caller outside\n   the package genuinely needs.\n5. **Early returns and guard clauses.** Handle errors and edge cases first\n   instead of nesting `if`/`else` chains.\n6. **Split complex functions.** If a function trips a complexity warning, break\n   it into named helpers rather than silencing the warning.\n7. **Avoid LLM-slop tells:** em dashes, hedging narration, restating the diff in\n   prose, trailing summaries. Defaults, not absolute bans. Applies to code,\n   comments, commit messages, and PR descriptions alike.\n8. **Concurrency: do a two-pass race analysis after every change** that touches\n   shared state, including reads of existing maps and slices. Guard them with a\n   mutex (or an atomic or channel where that fits better), keep critical\n   sections short, and run `go test -race` on the touched packages. See\n   [Concurrency and lifecycle](#concurrency-and-lifecycle) for the failure modes\n   to check for.\n9. **Cross-platform builds must keep working.** The agent targets Linux, macOS,\n   Windows, FreeBSD, Android, and iOS. When you add a platform-specific file,\n   add the counterpart or a build-tagged fallback for the others.\n10. **Never hand-edit generated files.** Change the source and regenerate.\n11. **Never log secrets** — private keys, setup keys, tokens, PAT values — and\n    keep peer IPs and hostnames out of logs above debug level.\n\n## Type safety\n\n**No bare primitives for domain concepts.** A `string` parameter for an account\nID next to a `string` parameter for a peer ID is two bugs waiting to happen,\nbecause the compiler cannot catch the swap. Declare the type once and use it\nthroughout, converting only at the boundaries where data enters or leaves —\nprotobuf, gRPC, HTTP, an external library.\n\n```go\ntype ServiceID string\ntype AccountID string\n\n// Internal: typed all the way through\nfunc (r *Router) RemoveRoute(host SNIHost, svcID ServiceID) { ... }\n\n// Proto boundary: convert once, on the way in and on the way out\nsvcID := ServiceID(mapping.GetId())\nreq.ServiceId = string(svcID)\n```\n\n- **IP addresses are `netip.Addr`**, not `string` and not `net.IP`. Parse at the\n  boundary and pass the typed value inward.\n- **Always `Unmap()`** after parsing an address, after converting from `net.IP`,\n  and after extracting one from `RemoteAddr()`. This normalizes a v4-mapped v6\n  address (`::ffff:10.1.2.3`) to plain v4 so IPv4 rules match it. A stored or\n  compared mapped address silently fails to match those rules.\n- **Ports are `uint16`** internally; use `int` only where a library forces it and\n  convert immediately.\n- **Enums are a typed string with constants**, so the valid set is discoverable\n  and a typo fails to compile.\n- **Map keys follow the same rule**, and must be a real type (`type ServiceID\n  string`) rather than an alias (`type serviceID = string`) — an alias silently\n  accepts bare strings.\n\n## Concurrency and lifecycle\n\nBeyond the mutex hygiene in the principles above, check for these failure\nmodes.\n\n- **Never read a struct field inside a goroutine** when another goroutine may nil\n  or reassign it. Pass the value as a parameter, or capture it into a local before\n  launching. This matters most when `Stop()` nils a field without waiting for the\n  goroutine to finish.\n\n  ```go\n  go func(ifaceName string) {   // good: passed in, cannot be nilled underneath\n      m.Start(ctx, ifaceName)\n  }(iface.Name())\n  ```\n\n- **Never wait on a channel while holding a lock the sender needs.** Copy what you\n  need out from under the lock, release it, then wait.\n\n  ```go\n  func (m *Manager) Stop() {\n      m.mu.Lock()\n      cancel, done := m.cancel, m.done\n      m.mu.Unlock()\n      if cancel != nil {\n          cancel()\n          <-done\n      }\n  }\n  ```\n\n- **`Stop`/`Close` must be idempotent** — guard on an already-stopped flag or a\n  nil cancel — and must release the state they guarded. Clear maps and caches;\n  a cancelled goroutine holding a live map still pins that memory. Note that a\n  nil map only panics on writes; reads and iteration behave like an empty map,\n  so where post-close use must be rejected, check the stopped flag explicitly.\n- **Publish coupled state only after every fallible step succeeds.** When several\n  fields form an invariant, build them into locals and assign them to the receiver\n  at the end. Assigning as you go leaves the object half-initialized when a later\n  step fails, so a readiness predicate reports ready while a coupled field is nil.\n  If an earlier step already had an external side effect — a created chain, an\n  opened handle, an inserted rule — roll it back before returning the error.\n- **Clean up what you own on constructor error paths.** Once a constructor has\n  started something, every later error path must undo it: cancel a goroutine and\n  wait for it to exit, stop a ticker, close a watcher. The object is never\n  returned, so its `Close` will never run.\n- **A failed `Start` must undo everything it started.** When a component brings up\n  several subsystems in sequence — connection manager, watchers, routing, DNS,\n  flow, persisted state — a failure partway through has to tear down the ones\n  already running, not just close the handle the error came from. Put the\n  already-started guard *before* that teardown path, so a rejected second `Start`\n  cannot dismantle the one that is running.\n\n## Error handling\n\nUse single-assignment form when the error is only needed inside the `if`:\n\n```go\n// Good\nif err := someCall(); err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n\n// Bad - unnecessary split\nerr := someCall()\nif err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n```\n\nUse multiple assignment when the value is needed after the block:\n\n```go\nresult, err := someCall()\nif err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n```\n\nAdd short, meaningful context, and **do not** start `fmt.Errorf` messages with\nobvious words like \"failed to\" or \"error\":\n\n```go\n// Good\nreturn fmt.Errorf(\"parse remote address: %w\", err)\nreturn fmt.Errorf(\"listen on %s: %w\", addr, err)\n\n// Bad\nreturn fmt.Errorf(\"failed to parse remote address: %w\", err)\nreturn fmt.Errorf(\"error listening on %s: %w\", addr, err)\n\n// \"failed\" is fine in log messages\nlog.Debugf(\"failed to parse remote address: %v\", err)\n```\n\nSkip the wrapping when a function only extracts or delegates and the wrap would\nadd nothing:\n\n```go\nfunc parseAddr(addr string) (string, int, error) {\n    host, portStr, err := net.SplitHostPort(addr)\n    if err != nil {\n        return \"\", 0, err\n    }\n    // ...\n}\n```\n\nLog the errors you choose not to act on:\n\n- `log.Debugf()` for errors that do not affect program flow but help debugging.\n- `log.Tracef()` for very verbose errors that would otherwise spam logs.\n- **Never ignore** errors from writes, network sends, or critical cleanup.\n- Close errors may be ignored for read-only operations; log them at debug for\n  writes.\n\n**Do not log and return the same error.** It gets reported twice, from two places,\nand the second reader cannot tell whether it happened once or twice. Return it and\nlet the caller decide. The exception is an API handler that has already written a\nresponse. Internal helpers return errors rather than logging and swallowing them.\n\n**Never return a typed nil as an error.** A nil `*MyError` stored in an `error`\ninterface is not nil, so `err != nil` is true and callers take the failure path on\nsuccess. Return the error only where it is actually set:\n\n```go\nif _, err := conn.Write(buf); err != nil {   // good\n    return err\n}\nreturn nil\n```\n\n**Accumulate with `multierror` when an operation should continue past individual\nfailures** — teardown, cleanup, or setup where partial success is acceptable.\n`client/errors.FormatErrorOrNil` returns nil for an empty accumulator, so callers\nstill see a plain nil on full success:\n\n```go\nfunc (m *Manager) Cleanup() error {\n    var merr *multierror.Error\n    for _, r := range m.resources {\n        if err := r.Close(); err != nil {\n            merr = multierror.Append(merr, fmt.Errorf(\"close %s: %w\", r.Name, err))\n        }\n    }\n    return nberrors.FormatErrorOrNil(merr)\n}\n```\n\n| Scenario              | Approach              | Why                                       |\n| --------------------- | --------------------- | ----------------------------------------- |\n| Cleanup / teardown    | Accumulate            | Clean up as much as possible              |\n| Setup with rollback   | Abort on first error  | Partial state is invalid; undo what stuck |\n| Setup with partial OK | Accumulate            | Degraded operation is still useful        |\n\n## Comments\n\nComment the **why**, never the **what**. Default to no comment, and add one only\nwhen a hidden constraint or workaround would surprise a future reader. Never\nreference the current task, PR, or your own changes in a comment.\n\n```go\n// Bad - trailing comments explaining the obvious\ndefer localConn.Close() // Close the connection\nif err != nil {         // Check if error occurred\n\n// Good\ndefer localConn.Close()\n\n// Good - explains a non-obvious constraint\n// Use incremental checksum update per RFC 1624 for performance.\nchecksum = updateChecksum(checksum, oldPort, newPort)\n```\n\n### Length budget\n\nNeither of these is linter-enforced, so they are conventions the surrounding code\nmostly follows rather than hard limits:\n\n- **Around 90 characters per line.** Wrap the comment rather than running well past\n  it.\n- **Roughly 250 characters per comment**, about three wrapped lines. Doc comments\n  on exported identifiers may exceed it when the API genuinely needs the\n  explanation; inline comments inside a function body rarely should.\n\nThe budget is a smell detector, not a rule to game. Do not compress a needed\nexplanation into cryptic shorthand to fit — if a block of code needs more than\n250 characters of prose, the code is doing too much. Fix the code:\n\n- **Extract a named function.** A well-named function replaces the comment: the\n  name says *what*, the body shows *how*, and the comment you no longer write\n  was the *what* anyway. Clean Code calls this \"explain yourself in code\".\n- **Extract a named constant or predicate.** `if isExpiredSetupKey(key)` needs\n  no comment; `if key.ExpiresAt.Before(now) && !key.Revoked && key.UsageLimit > 0`\n  does.\n- **Keep the surviving comment for the why** — the RFC, the kernel quirk, the\n  ordering constraint. That part is usually one or two lines.\n\n### Long switch and if/else chains\n\nA `switch` whose cases carry multi-line explanations is the usual place this\nbudget is breached, and the comment is a symptom. In order of preference:\n\n1. **Extract each case body into a named function.** The case becomes one line,\n   the name carries the meaning, and the switch reads as a table of contents.\n2. **Replace the switch with a lookup table** — `map[Kind]handlerFunc` — when the\n   branches are uniform. Adding a case stops meaning editing a growing function.\n3. **Replace conditional with polymorphism** when branches vary by type and the\n   same switch shape starts appearing in more than one place. Clean Code's rule\n   of thumb: tolerate a switch statement if it appears **once**, is buried in a\n   factory that returns an interface, and no other switch dispatches on the same\n   type. A second switch over the same enum is the signal to introduce the\n   interface.\n\nDo not restructure a switch purely to satisfy the budget when the cases are one\nline each and self-evident — a flat, boring `switch` over an enum is fine and\nneeds no comments at all.\n\nExplanatory comments in tests are welcome — they document the scenario being set\nup, and the 250-character budget does not apply to them.\n\n## Testing\n\n- **Unit tests** live beside the code as `_test.go`. `make test-unit` runs the\n  host-safe set with `-tags devcert` and no sudo.\n- **Privileged tests** carry the `privileged` build tag and mutate host\n  networking. They run through `make test-privileged`, inside a Docker container\n  with `NET_ADMIN`. Never bypass that harness by running them directly on the\n  host.\n- **End-to-end suites** live in `e2e/` with a shared harness.\n- **Test real behavior, not API existence.** Assert on the observable end state\n  a consumer would see — bytes that arrived, the packet after translation, the\n  row after the write — not merely that a method exists or returns an error.\n- **Avoid mocks for code we own.** Exercise the real store, manager, or\n  controller and assert what the caller actually receives.\n- **`require` for setup and preconditions, `assert` for the conditions under\n  test.** Use `require` whenever a later line would panic or be meaningless\n  otherwise.\n- **Message guidance:** optional for `NoError`/`Error`; always give context for\n  comparison, boolean, and collection assertions.\n- **Reproduce a bug before fixing it.** Write the test, watch it fail *for the\n  reason you expect* — a test that fails for an unrelated reason proves nothing —\n  then apply the fix and confirm it passes. Add the thin surrounding cases while\n  you are there.\n- **Use `t.Setenv`** rather than `os.Setenv` so the previous value is restored on\n  cleanup. To test the unset case, call `t.Setenv` first to register the restore,\n  then `os.Unsetenv`.\n- **Prefer `t.Cleanup` over `defer`** in any test with parallel subtests: the\n  parent function returns, running its `defer`s, while parallel subtests are\n  still suspended. Sequential subtests finish inside `t.Run`, so `defer` is safe\n  there, but `t.Cleanup` works in both cases.\n- **Explanatory comments in tests are welcome.** Describe the scenario being set\n  up; the comment budget below does not apply to them.\n\n```go\nserver, err := StartTestServer()\nrequire.NoError(t, err, \"Test server setup must succeed\")\ndefer server.Close()\n\nresult, err := client.DoOperation()\nassert.NoError(t, err)\nassert.Equal(t, expectedResult, result, \"Result should match expected\")\n```\n\n## Pitfalls\n\n- **The agent runs as root.** Anything touching routing, firewall, DNS, or the\n  interface can take a user's machine off the network. Prefer a reversible\n  change and make sure cleanup runs on every exit path.\n- **Management has two account loaders** (GORM and pgx). Adding a relation to an\n  account often means updating both, or it silently comes back empty in\n  production.\n- **`go test ./...` without `-tags devcert` skips tests** that need the\n  development certificate. Use `make test-unit`.\n- **`make lint` only checks the diff against `origin/main`.** CI runs\n  `make lint-all`; run it too before pushing a large change.\n- **Protos are consumed by released clients.** An old agent must keep working\n  against a new Management, so fields are added, never renumbered or removed.\n- **Windows requires the wintun driver**, and the daemon serves a named pipe\n  (`npipe://netbird`) rather than loopback TCP. Loopback TCP carries no caller\n  identity, so privileged operations are refused over it.\n\n## Commits, PRs, releases\n\n- **PR titles must start with a bracketed tag.** Before you propose a title,\n  **read [`.github/workflows/pr-title-check.yml`](.github/workflows/pr-title-check.yml)\n  and take the allowed tags from the `allowedTags` array in that file.** It is\n  the only source of truth, it changes as components are added, and the check\n  runs on every title edit — a tag that is not in that array is a red build. Do\n  not rely on a list memorized from anywhere else, including this file.\n\n  ```text\n  [client] Authorize daemon IPC callers by their local identity\n  [management,client] Add MDM policy support\n  ```\n\n  Multiple tags are comma-separated inside one pair of brackets. Match the tag\n  to the component you actually changed, not to the one you read the most.\n\n- **Use the repository's PR template.** Fill in\n  [`.github/pull_request_template.md`](.github/pull_request_template.md) rather\n  than replacing it with your own summary: describe the change, link the issue,\n  tick the checklist honestly (including \"ran locally\" and \"single purpose\"),\n  and complete the documentation section. Do not tick a box you have not\n  verified, and do not delete rows that do not apply — the docs gate in CI reads\n  that section and fails when it is missing.\n\n- **Keep the PR description short.** Under 1000 words on top of the template's\n  own text, and usually far less — a few paragraphs. Reviewers read the diff;\n  the description exists to explain what the diff cannot say for itself. This is\n  well below what an agent will produce by default, so cut before you post.\n\n- **Body: why before what.** Lead with the problem and the reason for this\n  approach, then the shape of the change. No bullet list of files changed, no\n  per-function walkthrough, no restating the diff in prose, no trailing summary\n  section, no self-congratulatory closing line.\n\n- **No `Co-Authored-By` or tool-attribution trailers in the PR description**,\n  and none in commits either. Contributors own their contributions. Whatever\n  tooling produced the diff, the person opening the PR is its author: they have\n  read every line, they can explain why it works, they can answer review\n  questions without going back to a model, and they are accountable for the\n  consequences of merging it. Do not add a trailer, footer, or description line\n  that spreads that ownership onto a tool.\n\n- **Commit subjects follow the same `[scope] Subject` convention.** Keep the\n  subject short, and use the body for why before what. No bullet lists of files\n  changed.\n\n- **Push review fixes as separate commits.** The PR is squashed on merge, so\n  there is no reason to rewrite history mid-review; many small commits make the\n  re-review readable.\n\n- **Do not force-push a branch that is under review.** A force-push detaches\n  existing review comments from the lines they were written against, destroys\n  the \"changes since your last review\" diff a reviewer relies on, and discards\n  the CI history that showed which commit broke what. Add commits instead —\n  including for fixups and reverts. Force-push only when there is no\n  alternative: a rebase to clear a genuine conflict, or removing a secret or a\n  large binary that was committed by mistake. When you must, ask the user first,\n  then say so in a PR comment so reviewers know their anchors moved. Never\n  force-push `main`, and never force-push a branch you do not own.\n\n- **One PR, one purpose.** Split refactors out of fixes and fixes out of\n  features.\n\n- **Keep the PR small.** Size is the single strongest predictor of how long a PR\n  waits. Aim for **under ~400 changed lines across under ~20 files**; past\n  roughly **1000 lines or 50 files** a community PR is likely to be sent back to\n  be split, or left unreviewed until it is. Large PRs from outside the core team\n  may be blocked outright when the size was never agreed in the ticket —\n  reviewing a sprawling change against a privileged networking daemon is a\n  security risk in itself, not just a time cost.\n\n  Judge the size by hand-written code: exclude generated output, `go.sum`,\n  vendored files, and test fixtures from the estimate, but do not use their\n  presence to argue a 3000-line PR is small.\n\n  When a change genuinely cannot be small — a protocol migration, a\n  cross-component rename — agree the split in the ticket **before** writing\n  code, and land it as a sequence of PRs that each build, test, and make sense\n  on their own. Propose that split to the user rather than opening one large PR\n  and hoping.\n\n  Prefer GitHub's stacked pull requests for such a sequence, rather than\n  hand-managing base branches: open each PR against the branch below it instead of\n  `main`, so every PR's diff shows only its own change. Merging a layer retargets\n  the PRs above it, and branch protections and required checks on the base branch\n  still apply to each one.\n\n- **User-facing changes need a docs PR** in\n  [netbirdio/docs](https://github.com/netbirdio/docs), linked from the PR\n  description.\n\n## After you push: CI and review bots\n\nOpening the PR is not the end of the task. Watch the run, read what the bots\nsay, and drive the PR to green before you report the work as done.\n\n```bash\ngh pr checks <pr>            --watch    # all checks, live\ngh run view <run-id> --log-failed       # only the failing steps\ngh pr view <pr> --comments              # bot and human review comments\n```\n\n**Never report a change as finished while checks are pending or red**, and never\ndescribe a red PR as passing. If you ran out of turn before CI finished, say\nwhich checks were still running.\n\n### The checks\n\n- **Go tests** — `golang-test-{linux,darwin,windows,freebsd}.yml`, sharded per\n  component. A failure in a component you did not touch is usually a real\n  interaction, not noise; read the log before assuming flake.\n- **golangci-lint** — `golangci-lint.yml` runs the full repository, while\n  `make lint` only checks your diff. A clean local lint does not guarantee green\n  CI on a large change.\n- **PR Title Check** — `pr-title-check.yml`, see above.\n- **Codecov** — uploaded from the Linux test workflow with per-component flags\n  (`unit,client`, `unit,management`, `unit,relay`, `unit,proxy`, `unit,signal`,\n  `integration,management`). Coverage on new code should not go backwards. Add\n  tests for the paths you introduced; do not adjust thresholds or exclude files\n  to clear the report.\n- **CodeRabbit** — configured in [`.coderabbit.yaml`](.coderabbit.yaml): `chill`\n  profile, auto-review on every non-draft PR, TypeScript/JavaScript/SVG paths\n  filtered out. Chat auto-reply is on, so `@coderabbitai` in a comment reaches\n  it.\n- **SonarCloud** — project `netbirdio_netbird`, quality gate on new code (bugs,\n  vulnerabilities, code smells, duplication, coverage).\n- **Snyk** — dependency and code scanning.\n\nSonar and Snyk report as GitHub App checks rather than workflows in this\nrepository, so their detail lives on the PR check, not in the Actions logs.\n\n### Handling bot findings\n\n- **Read every comment and act on it.** Either fix it, or reply with the reason\n  it does not apply. Do not bulk-resolve threads to clear the count, and do not\n  silently ignore a finding because the check is advisory.\n- **Bots are frequently wrong here.** NetBird has privileged, platform-specific,\n  and concurrency-heavy code that static analysis reads poorly. A confident\n  CodeRabbit or Sonar comment can still be nonsense. Verify the claim against\n  the code before you change anything — never edit correct code just to silence\n  a bot.\n- **Security findings get the opposite default.** For a Snyk or Sonar\n  vulnerability, or a CodeRabbit comment about authentication, authorization,\n  certificate verification, or key handling, assume it is real until you have\n  disproved it. Surface it to the user rather than dismissing it yourself.\n- **A new vulnerable dependency is a stop.** Bumping or replacing dependencies\n  needs the user's decision, as above.\n- **Never change a workflow, threshold, lint exclusion, or bot config to make a\n  check pass.** If a check is genuinely wrong, say so and let the user decide.\n- **Do not paper over flakes with blind re-runs.** Identify the failure first. If\n  it is a known flake, name it; if you cannot tell, report it as unresolved\n  rather than re-running until it goes green.\n\n## Discussion and support\n\n- Discussions: <https://github.com/netbirdio/netbird/discussions>\n- Slack: <https://docs.netbird.io/slack-url>\n- Docs: <https://docs.netbird.io>\n- Security: <https://github.com/netbirdio/netbird/security/policy> — never in public\n- Contribution process: [CONTRIBUTING.md](CONTRIBUTING.md)\n"},"files":{"CLAUDE.md":"See [AGENTS.md](AGENTS.md) for the agent guidelines in this repository.\n","AGENTS.md":"# NetBird Agent Guidelines\n\n**NetBird** is an open source connectivity platform: a WireGuard®-based overlay\nnetwork with a control plane. The **agent** (`client/`) runs on user machines as\na privileged daemon and manages the WireGuard interface, routing, firewall, and\nDNS. **Management** (`management/`) is the control plane and REST/gRPC API,\n**Signal** (`signal/`) brokers peer handshakes, **Relay** (`relay/`) carries\ntraffic when a direct tunnel is impossible, and **Proxy** (`proxy/`) is the\nidentity-aware proxy behind Agent Network.\n\nThis file applies to the whole repository, and is the single source of truth for\nagent guidance here. `CLAUDE.md` is a one-line pointer to it — keep the guidance\nin this file, not duplicated there.\n\n## Contents\n\n- [STOP and ask the user before](#stop-and-ask-the-user-before)\n- [Quick reference](#quick-reference)\n- [Structure](#structure)\n- [Where to look](#where-to-look)\n- [Security](#security)\n- [Agent conventions](#agent-conventions)\n- [Repo-wide principles](#repo-wide-principles)\n- [Type safety](#type-safety)\n- [Concurrency and lifecycle](#concurrency-and-lifecycle)\n- [Error handling](#error-handling)\n- [Comments](#comments)\n- [Testing](#testing)\n- [Pitfalls](#pitfalls)\n- [Commits, PRs, releases](#commits-prs-releases)\n- [After you push: CI and review bots](#after-you-push-ci-and-review-bots)\n- [Discussion and support](#discussion-and-support)\n\n## STOP and ask the user before\n\n- **Opening a pull request for anything beyond a trivial fix, without an agreed\n  ticket.** Ask the user directly: *\"Is there a discussion or issue for this\n  change?\"* NetBird is discussion-first — community reports start in\n  [Discussions](https://github.com/netbirdio/netbird/discussions), DevRel\n  validates them, and only validated discussions become issues. A PR that\n  changes behavior with no linked issue may be closed on arrival. If there is no\n  ticket, offer to draft the discussion post **instead of** the PR, and wait for\n  the user's call. Only typos, broken links, documentation corrections, and\n  one-line fixes that already have an issue can skip this.\n- **Designing in any high-risk area** (see\n  [CONTRIBUTING.md](CONTRIBUTING.md#high-risk-areas)): public API and OpenAPI\n  schema, gRPC protos, behavior existing deployments would notice after an\n  upgrade, peer connectivity (ICE, NAT traversal, relay selection, WireGuard® or\n  Rosenpass key handling), client system integration (routing, firewall, DNS,\n  interface), authentication and authorization, CLI or service flags, config\n  file format, daemon IPC, store schema and migrations, or a new feature. The\n  design gets agreed in the ticket before code is written.\n- **Writing a store migration or changing a persisted model.** Migrations are\n  one-way in the field and both the GORM and pgx paths may need the change.\n- **Hand-editing generated code.** `*.pb.go`, `*.gen.go`, and mocks are outputs.\n  Edit the source (`.proto`, `openapi.yml`) and rerun the matching\n  `generate.sh`.\n- **Adding, removing, or bumping a dependency**, and never vendor a fork.\n- **Weakening a security control** — authentication, authorization, certificate\n  verification, privilege dropping, or peer identity checks — even when it is\n  the fastest way to make a test pass.\n- **Force-pushing to `main`**, force-pushing any branch that is already under\n  review, amending pushed commits, or bypassing hooks with `--no-verify`.\n\n## Quick reference\n\n```bash\n# Build\ngo build ./...\ncd client && CGO_ENABLED=0 go build .        # agent\ncd management && go build .                  # management service\ncd signal && go build .                      # signal service\n\n# Verify (run before every push)\ngo fmt ./...\nmake lint            # golangci-lint on files changed vs origin/main (also the pre-push hook)\nmake lint-all        # full-repository lint, matches CI\nmake test-unit       # host-safe unit tests, -tags devcert, no sudo\nmake test-privileged # privileged-tagged suite in a Docker container with NET_ADMIN\nmake setup-hooks     # wire make lint into .githooks/pre-push\n\n# Narrow runs\ngo test ./client/internal/dns/...\ngo test -race -run TestPeerConn ./client/internal/peer/...\nPRIV_RUN=TestNftablesManager PRIV_PKGS=./client/firewall/nftables/... make test-privileged\n\n# Code generation (never hand-edit the output)\n./shared/management/http/api/generate.sh   # REST types from openapi.yml\n./shared/management/proto/generate.sh\n./shared/signal/proto/generate.sh\n./client/proto/generate.sh\n./flow/proto/generate.sh\n\n# Run locally (lab only, never on a machine you rely on)\nsudo ./client/netbird up --log-level debug --log-file console\nsudo ./client/netbird down                   # teardown: restores routing, firewall, DNS\n./signal/signal run --log-level debug --log-file console\n./management/management management --log-level debug --log-file console --config ./management.json\n```\n\n`netbird up` needs root and rewrites the host's routing table, firewall rules,\nDNS configuration, and WireGuard® interface. Run it only in a disposable test\nenvironment (a VM, container, or throwaway host) that you can rebuild, never on\na workstation or server whose connectivity matters. Run `sudo netbird down`\nbefore you stop working, before rebuilding the binary, and on every failure\npath, so the host's networking state is restored instead of left half-applied.\nSee [Pitfalls](#pitfalls) for why cleanup on every exit path matters.\n\n## Structure\n\n```text\nnetbird/\n├── client/              NetBird agent\n│   ├── cmd/             agent CLI\n│   ├── internal/        agent business logic (engine, peer, dns, routemanager, ...)\n│   ├── server/          daemon for background execution\n│   ├── proto/           daemon gRPC protos\n│   ├── iface/           WireGuard® interface management\n│   ├── firewall/        nftables, iptables, pf, WFP, userspace backends\n│   ├── ssh/             built-in SSH server and client\n│   ├── ui/              desktop UI (Wails v3 + React)\n│   ├── android/, ios/   mobile bindings\n│   ├── wasm/            WebAssembly build\n│   └── mdm/, system/    MDM policy, host information\n├── management/          control plane\n│   └── server/          account, peer, groups, networks, posture, permissions,\n│                        settings, store, http (REST), idp, integrations, migration\n├── signal/              handshake broker (peer/, server/)\n├── relay/               relay service (protocol/, server/, healthcheck/)\n├── proxy/               identity-aware proxy (llm/, acme/, accesslog/, middleware/, tcp/, udp/)\n├── agent-network/       Agent Network overview\n├── shared/              imported by both agent and services\n│   ├── management/      proto/, client/, http/api (OpenAPI + generated types)\n│   ├── signal/          proto/, client/\n│   └── relay/, auth/, sshauth/, metrics/\n├── e2e/                 end-to-end suites and harness\n├── encryption/, dns/, route/, stun/, sharedsock/, util/, flow/\n├── infrastructure_files/  docker compose and getting-started templates\n└── release_files/       files packaged into releases\n```\n\n## Where to look\n\n| Task                        | Location                                                     |\n| --------------------------- | ------------------------------------------------------------ |\n| REST API / OpenAPI          | `shared/management/http/api/` + `management/server/http/`    |\n| Management gRPC protocol    | `shared/management/proto/`                                   |\n| Signal protocol             | `shared/signal/proto/`                                       |\n| Daemon IPC protocol         | `client/proto/`                                              |\n| Peer connection and NAT     | `client/internal/peer/`                                      |\n| Network map handling        | `client/internal/engine.go`, `shared/management/networkmap/` |\n| Routing                     | `client/internal/routemanager/`, `route/`                    |\n| Firewall backends           | `client/firewall/`                                           |\n| DNS                         | `client/internal/dns/`, `dns/`                               |\n| WireGuard® interface        | `client/iface/`                                              |\n| Persistence and migrations  | `management/server/store/`, `management/server/migration/`   |\n| IdP integrations            | `management/server/idp/`                                     |\n| Permissions model           | `management/server/permissions/`                             |\n| LLM routing / Agent Network | `proxy/internal/llm/`, `agent-network/`                      |\n| End-to-end tests            | `e2e/`                                                       |\n\n## Security\n\n### Never fail open\n\nWhen a security check — access control, an IP restriction, an auth decision —\nhits an error such as an unparseable value, an unavailable lookup, or a state it\ndoes not recognize, it must **deny**. Never skip the check or allow the request\nthrough because the check itself failed, and make the `default` and unknown cases\nof a security-related `switch` deny rather than fall through.\n\n### Daemon RPC input is untrusted\n\nThe agent runs as root (LocalSystem on Windows), so a daemon RPC crosses a\nprivilege boundary: treat every field as untrusted input rather than as something\nthe UI or CLI validated on the way in.\n\nWhen you add or change an RPC, ask what the handler does with caller input while\nrunning as root. If the answer touches a filesystem path, a URL or host, or a\nprivileged state change, it needs a gate **in the handler** — a check in the client\nthat normally calls it is not a check at all.\n\n- **A caller-supplied path the daemon opens.** Never `os.Open` it as root.\n  Constrain it, then open it *as the caller* with `ipcauth.OpenOwnedFile`, which\n  opens `O_NOFOLLOW`, requires a regular file, and refuses a file the caller does\n  not own — so a symlink or hardlink aimed at a root-only file is rejected.\n- **A caller-supplied URL or host the daemon fetches.** Restrict the scheme and\n  allow only known hosts for unprivileged callers. Prefer a lexical host\n  allowlist plus TLS verification over \"resolve the host, then reject private\n  IPs\": the resolve-then-trust pattern has a DNS-rebinding race (public IP at\n  check time, attacker IP at connect time), while a name allowlist has no IP\n  check to race. Never accept `http://` where `https://` is expected.\n- **A privileged state change** (SSH root login, management URL, deregistration)\n  gates on the caller identity from `ipcauth.CallerIdentity(ctx)`.\n\nCaller identity comes from the kernel — `SO_PEERCRED`, `LOCAL_PEERCRED`, or the\nnamed-pipe client token — and never from an RPC field. When\n`ipcauth.CallerIdentity` reports that it could not determine an identity, **deny**;\ndo not fall back to treating the caller as the transport peer.\n\n## Agent conventions\n\n### Three networking modes\n\nWhere packets actually flow depends on the mode the agent is running in. The\nthree are not interchangeable, so establish which one a change applies to — and\nwhat it should do in the other two — before you write it.\n\n- **kernel mode** (Linux only): in-kernel WireGuard®. The kernel handles both\n  peer-to-peer and routed traffic, and ACLs are iptables or nftables rules. The\n  client programs kernel facilities but never sees the traffic itself.\n- **userspace mode** (wireguard-go with a TUN): wireguard-go runs in-process. The\n  kernel handles peer-to-peer traffic once it leaves the TUN, while routed traffic\n  — exit nodes and network routes — goes through the userspace forwarder, which\n  terminates the connection and re-establishes it over OS sockets. Used on\n  platforms without kernel WireGuard® or when the user opts out.\n- **netstack mode**: wireguard-go in-process with no TUN and no kernel\n  networking. The forwarder does all routing by stitching userspace sockets, and\n  listeners such as the embedded SSH and DNS servers bind on a gVisor netstack.\n  Used where the process cannot create a TUN device, such as the embedded client\n  (`client/embed/`) and the WASM build.\n\n### The overlay interface is not \"WireGuard\"\n\nDo not put \"WireGuard\" in identifiers or comments unless the code is genuinely\ncoupled to WireGuard® specifically — a wireguard-go call, a handshake field, a\nkernel WireGuard® netlink attribute. For the interface, the host, peers, or\ntraffic in general, say \"the NetBird interface\", \"the interface\", or \"the overlay\".\nMost firewall, routing, and DNS code is transport-agnostic, so a WireGuard®\nreference there is simply inaccurate and rots as the transports change.\n\n### IPv6 is a soft feature\n\nThe IPv6 overlay is opt-in dual-stack, and capability can change at runtime. Treat\nit as soft rather than a requirement:\n\n- Gate local v6 paths on the interface accessor (`wgIface.Address().HasIPv6()`),\n  not on raw state fields, and skip the v6 path when the host has no v6 rather\n  than returning an error.\n- Treat an empty or unparseable peer v6 address as \"no v6 for that peer\" and skip\n  it, keeping the v4 path working.\n- Never let a missing v6 break v4. Fail-closed is for security checks; a\n  capability mismatch skips the v6 work and carries on.\n\n### Environment variables\n\nName the variable in a constant and parse booleans with `strconv.ParseBool` rather\nthan comparing strings inline, so an unexpected value is logged instead of\nsilently meaning false:\n\n```go\nconst EnvDisableFeature = \"NB_DISABLE_FEATURE\"\n\nfunc isDisabledByEnv() bool {\n    val := os.Getenv(EnvDisableFeature)\n    if val == \"\" {\n        return false\n    }\n    disabled, err := strconv.ParseBool(val)\n    if err != nil {\n        log.Warnf(\"failed to parse %s: %v\", EnvDisableFeature, err)\n        return false\n    }\n    return disabled\n}\n```\n\n### Validating against protocol specs\n\nWhen a change depends on what a protocol actually mandates, read the specification\ntext from the [IETF datatracker](https://datatracker.ietf.org/) rather than a\nsummary, and check that you have the current RFC — the widely cited one for a\nprotocol is often superseded. Cite the section, not just the document, so a\nreviewer can jump straight to the rule.\n\n## Repo-wide principles\n\n1. **Run `go fmt` on every modified Go file.** Formatting is not optional.\n2. **Zero unaddressed linter warnings.** Fix what `golangci-lint` reports on code\n   you touch, and delete imports, helpers, and parameters your refactor orphaned.\n   Exception: unused parameters in shared code may be consumed by builds outside\n   this repository — do not remove them, ask instead.\n3. **Function comments are mandatory for exported functions**, written as full\n   sentences with a period, starting with the identifier name.\n4. **Prefer private functions and constants.** Export only what a caller outside\n   the package genuinely needs.\n5. **Early returns and guard clauses.** Handle errors and edge cases first\n   instead of nesting `if`/`else` chains.\n6. **Split complex functions.** If a function trips a complexity warning, break\n   it into named helpers rather than silencing the warning.\n7. **Avoid LLM-slop tells:** em dashes, hedging narration, restating the diff in\n   prose, trailing summaries. Defaults, not absolute bans. Applies to code,\n   comments, commit messages, and PR descriptions alike.\n8. **Concurrency: do a two-pass race analysis after every change** that touches\n   shared state, including reads of existing maps and slices. Guard them with a\n   mutex (or an atomic or channel where that fits better), keep critical\n   sections short, and run `go test -race` on the touched packages. See\n   [Concurrency and lifecycle](#concurrency-and-lifecycle) for the failure modes\n   to check for.\n9. **Cross-platform builds must keep working.** The agent targets Linux, macOS,\n   Windows, FreeBSD, Android, and iOS. When you add a platform-specific file,\n   add the counterpart or a build-tagged fallback for the others.\n10. **Never hand-edit generated files.** Change the source and regenerate.\n11. **Never log secrets** — private keys, setup keys, tokens, PAT values — and\n    keep peer IPs and hostnames out of logs above debug level.\n\n## Type safety\n\n**No bare primitives for domain concepts.** A `string` parameter for an account\nID next to a `string` parameter for a peer ID is two bugs waiting to happen,\nbecause the compiler cannot catch the swap. Declare the type once and use it\nthroughout, converting only at the boundaries where data enters or leaves —\nprotobuf, gRPC, HTTP, an external library.\n\n```go\ntype ServiceID string\ntype AccountID string\n\n// Internal: typed all the way through\nfunc (r *Router) RemoveRoute(host SNIHost, svcID ServiceID) { ... }\n\n// Proto boundary: convert once, on the way in and on the way out\nsvcID := ServiceID(mapping.GetId())\nreq.ServiceId = string(svcID)\n```\n\n- **IP addresses are `netip.Addr`**, not `string` and not `net.IP`. Parse at the\n  boundary and pass the typed value inward.\n- **Always `Unmap()`** after parsing an address, after converting from `net.IP`,\n  and after extracting one from `RemoteAddr()`. This normalizes a v4-mapped v6\n  address (`::ffff:10.1.2.3`) to plain v4 so IPv4 rules match it. A stored or\n  compared mapped address silently fails to match those rules.\n- **Ports are `uint16`** internally; use `int` only where a library forces it and\n  convert immediately.\n- **Enums are a typed string with constants**, so the valid set is discoverable\n  and a typo fails to compile.\n- **Map keys follow the same rule**, and must be a real type (`type ServiceID\n  string`) rather than an alias (`type serviceID = string`) — an alias silently\n  accepts bare strings.\n\n## Concurrency and lifecycle\n\nBeyond the mutex hygiene in the principles above, check for these failure\nmodes.\n\n- **Never read a struct field inside a goroutine** when another goroutine may nil\n  or reassign it. Pass the value as a parameter, or capture it into a local before\n  launching. This matters most when `Stop()` nils a field without waiting for the\n  goroutine to finish.\n\n  ```go\n  go func(ifaceName string) {   // good: passed in, cannot be nilled underneath\n      m.Start(ctx, ifaceName)\n  }(iface.Name())\n  ```\n\n- **Never wait on a channel while holding a lock the sender needs.** Copy what you\n  need out from under the lock, release it, then wait.\n\n  ```go\n  func (m *Manager) Stop() {\n      m.mu.Lock()\n      cancel, done := m.cancel, m.done\n      m.mu.Unlock()\n      if cancel != nil {\n          cancel()\n          <-done\n      }\n  }\n  ```\n\n- **`Stop`/`Close` must be idempotent** — guard on an already-stopped flag or a\n  nil cancel — and must release the state they guarded. Clear maps and caches;\n  a cancelled goroutine holding a live map still pins that memory. Note that a\n  nil map only panics on writes; reads and iteration behave like an empty map,\n  so where post-close use must be rejected, check the stopped flag explicitly.\n- **Publish coupled state only after every fallible step succeeds.** When several\n  fields form an invariant, build them into locals and assign them to the receiver\n  at the end. Assigning as you go leaves the object half-initialized when a later\n  step fails, so a readiness predicate reports ready while a coupled field is nil.\n  If an earlier step already had an external side effect — a created chain, an\n  opened handle, an inserted rule — roll it back before returning the error.\n- **Clean up what you own on constructor error paths.** Once a constructor has\n  started something, every later error path must undo it: cancel a goroutine and\n  wait for it to exit, stop a ticker, close a watcher. The object is never\n  returned, so its `Close` will never run.\n- **A failed `Start` must undo everything it started.** When a component brings up\n  several subsystems in sequence — connection manager, watchers, routing, DNS,\n  flow, persisted state — a failure partway through has to tear down the ones\n  already running, not just close the handle the error came from. Put the\n  already-started guard *before* that teardown path, so a rejected second `Start`\n  cannot dismantle the one that is running.\n\n## Error handling\n\nUse single-assignment form when the error is only needed inside the `if`:\n\n```go\n// Good\nif err := someCall(); err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n\n// Bad - unnecessary split\nerr := someCall()\nif err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n```\n\nUse multiple assignment when the value is needed after the block:\n\n```go\nresult, err := someCall()\nif err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n```\n\nAdd short, meaningful context, and **do not** start `fmt.Errorf` messages with\nobvious words like \"failed to\" or \"error\":\n\n```go\n// Good\nreturn fmt.Errorf(\"parse remote address: %w\", err)\nreturn fmt.Errorf(\"listen on %s: %w\", addr, err)\n\n// Bad\nreturn fmt.Errorf(\"failed to parse remote address: %w\", err)\nreturn fmt.Errorf(\"error listening on %s: %w\", addr, err)\n\n// \"failed\" is fine in log messages\nlog.Debugf(\"failed to parse remote address: %v\", err)\n```\n\nSkip the wrapping when a function only extracts or delegates and the wrap would\nadd nothing:\n\n```go\nfunc parseAddr(addr string) (string, int, error) {\n    host, portStr, err := net.SplitHostPort(addr)\n    if err != nil {\n        return \"\", 0, err\n    }\n    // ...\n}\n```\n\nLog the errors you choose not to act on:\n\n- `log.Debugf()` for errors that do not affect program flow but help debugging.\n- `log.Tracef()` for very verbose errors that would otherwise spam logs.\n- **Never ignore** errors from writes, network sends, or critical cleanup.\n- Close errors may be ignored for read-only operations; log them at debug for\n  writes.\n\n**Do not log and return the same error.** It gets reported twice, from two places,\nand the second reader cannot tell whether it happened once or twice. Return it and\nlet the caller decide. The exception is an API handler that has already written a\nresponse. Internal helpers return errors rather than logging and swallowing them.\n\n**Never return a typed nil as an error.** A nil `*MyError` stored in an `error`\ninterface is not nil, so `err != nil` is true and callers take the failure path on\nsuccess. Return the error only where it is actually set:\n\n```go\nif _, err := conn.Write(buf); err != nil {   // good\n    return err\n}\nreturn nil\n```\n\n**Accumulate with `multierror` when an operation should continue past individual\nfailures** — teardown, cleanup, or setup where partial success is acceptable.\n`client/errors.FormatErrorOrNil` returns nil for an empty accumulator, so callers\nstill see a plain nil on full success:\n\n```go\nfunc (m *Manager) Cleanup() error {\n    var merr *multierror.Error\n    for _, r := range m.resources {\n        if err := r.Close(); err != nil {\n            merr = multierror.Append(merr, fmt.Errorf(\"close %s: %w\", r.Name, err))\n        }\n    }\n    return nberrors.FormatErrorOrNil(merr)\n}\n```\n\n| Scenario              | Approach              | Why                                       |\n| --------------------- | --------------------- | ----------------------------------------- |\n| Cleanup / teardown    | Accumulate            | Clean up as much as possible              |\n| Setup with rollback   | Abort on first error  | Partial state is invalid; undo what stuck |\n| Setup with partial OK | Accumulate            | Degraded operation is still useful        |\n\n## Comments\n\nComment the **why**, never the **what**. Default to no comment, and add one only\nwhen a hidden constraint or workaround would surprise a future reader. Never\nreference the current task, PR, or your own changes in a comment.\n\n```go\n// Bad - trailing comments explaining the obvious\ndefer localConn.Close() // Close the connection\nif err != nil {         // Check if error occurred\n\n// Good\ndefer localConn.Close()\n\n// Good - explains a non-obvious constraint\n// Use incremental checksum update per RFC 1624 for performance.\nchecksum = updateChecksum(checksum, oldPort, newPort)\n```\n\n### Length budget\n\nNeither of these is linter-enforced, so they are conventions the surrounding code\nmostly follows rather than hard limits:\n\n- **Around 90 characters per line.** Wrap the comment rather than running well past\n  it.\n- **Roughly 250 characters per comment**, about three wrapped lines. Doc comments\n  on exported identifiers may exceed it when the API genuinely needs the\n  explanation; inline comments inside a function body rarely should.\n\nThe budget is a smell detector, not a rule to game. Do not compress a needed\nexplanation into cryptic shorthand to fit — if a block of code needs more than\n250 characters of prose, the code is doing too much. Fix the code:\n\n- **Extract a named function.** A well-named function replaces the comment: the\n  name says *what*, the body shows *how*, and the comment you no longer write\n  was the *what* anyway. Clean Code calls this \"explain yourself in code\".\n- **Extract a named constant or predicate.** `if isExpiredSetupKey(key)` needs\n  no comment; `if key.ExpiresAt.Before(now) && !key.Revoked && key.UsageLimit > 0`\n  does.\n- **Keep the surviving comment for the why** — the RFC, the kernel quirk, the\n  ordering constraint. That part is usually one or two lines.\n\n### Long switch and if/else chains\n\nA `switch` whose cases carry multi-line explanations is the usual place this\nbudget is breached, and the comment is a symptom. In order of preference:\n\n1. **Extract each case body into a named function.** The case becomes one line,\n   the name carries the meaning, and the switch reads as a table of contents.\n2. **Replace the switch with a lookup table** — `map[Kind]handlerFunc` — when the\n   branches are uniform. Adding a case stops meaning editing a growing function.\n3. **Replace conditional with polymorphism** when branches vary by type and the\n   same switch shape starts appearing in more than one place. Clean Code's rule\n   of thumb: tolerate a switch statement if it appears **once**, is buried in a\n   factory that returns an interface, and no other switch dispatches on the same\n   type. A second switch over the same enum is the signal to introduce the\n   interface.\n\nDo not restructure a switch purely to satisfy the budget when the cases are one\nline each and self-evident — a flat, boring `switch` over an enum is fine and\nneeds no comments at all.\n\nExplanatory comments in tests are welcome — they document the scenario being set\nup, and the 250-character budget does not apply to them.\n\n## Testing\n\n- **Unit tests** live beside the code as `_test.go`. `make test-unit` runs the\n  host-safe set with `-tags devcert` and no sudo.\n- **Privileged tests** carry the `privileged` build tag and mutate host\n  networking. They run through `make test-privileged`, inside a Docker container\n  with `NET_ADMIN`. Never bypass that harness by running them directly on the\n  host.\n- **End-to-end suites** live in `e2e/` with a shared harness.\n- **Test real behavior, not API existence.** Assert on the observable end state\n  a consumer would see — bytes that arrived, the packet after translation, the\n  row after the write — not merely that a method exists or returns an error.\n- **Avoid mocks for code we own.** Exercise the real store, manager, or\n  controller and assert what the caller actually receives.\n- **`require` for setup and preconditions, `assert` for the conditions under\n  test.** Use `require` whenever a later line would panic or be meaningless\n  otherwise.\n- **Message guidance:** optional for `NoError`/`Error`; always give context for\n  comparison, boolean, and collection assertions.\n- **Reproduce a bug before fixing it.** Write the test, watch it fail *for the\n  reason you expect* — a test that fails for an unrelated reason proves nothing —\n  then apply the fix and confirm it passes. Add the thin surrounding cases while\n  you are there.\n- **Use `t.Setenv`** rather than `os.Setenv` so the previous value is restored on\n  cleanup. To test the unset case, call `t.Setenv` first to register the restore,\n  then `os.Unsetenv`.\n- **Prefer `t.Cleanup` over `defer`** in any test with parallel subtests: the\n  parent function returns, running its `defer`s, while parallel subtests are\n  still suspended. Sequential subtests finish inside `t.Run`, so `defer` is safe\n  there, but `t.Cleanup` works in both cases.\n- **Explanatory comments in tests are welcome.** Describe the scenario being set\n  up; the comment budget below does not apply to them.\n\n```go\nserver, err := StartTestServer()\nrequire.NoError(t, err, \"Test server setup must succeed\")\ndefer server.Close()\n\nresult, err := client.DoOperation()\nassert.NoError(t, err)\nassert.Equal(t, expectedResult, result, \"Result should match expected\")\n```\n\n## Pitfalls\n\n- **The agent runs as root.** Anything touching routing, firewall, DNS, or the\n  interface can take a user's machine off the network. Prefer a reversible\n  change and make sure cleanup runs on every exit path.\n- **Management has two account loaders** (GORM and pgx). Adding a relation to an\n  account often means updating both, or it silently comes back empty in\n  production.\n- **`go test ./...` without `-tags devcert` skips tests** that need the\n  development certificate. Use `make test-unit`.\n- **`make lint` only checks the diff against `origin/main`.** CI runs\n  `make lint-all`; run it too before pushing a large change.\n- **Protos are consumed by released clients.** An old agent must keep working\n  against a new Management, so fields are added, never renumbered or removed.\n- **Windows requires the wintun driver**, and the daemon serves a named pipe\n  (`npipe://netbird`) rather than loopback TCP. Loopback TCP carries no caller\n  identity, so privileged operations are refused over it.\n\n## Commits, PRs, releases\n\n- **PR titles must start with a bracketed tag.** Before you propose a title,\n  **read [`.github/workflows/pr-title-check.yml`](.github/workflows/pr-title-check.yml)\n  and take the allowed tags from the `allowedTags` array in that file.** It is\n  the only source of truth, it changes as components are added, and the check\n  runs on every title edit — a tag that is not in that array is a red build. Do\n  not rely on a list memorized from anywhere else, including this file.\n\n  ```text\n  [client] Authorize daemon IPC callers by their local identity\n  [management,client] Add MDM policy support\n  ```\n\n  Multiple tags are comma-separated inside one pair of brackets. Match the tag\n  to the component you actually changed, not to the one you read the most.\n\n- **Use the repository's PR template.** Fill in\n  [`.github/pull_request_template.md`](.github/pull_request_template.md) rather\n  than replacing it with your own summary: describe the change, link the issue,\n  tick the checklist honestly (including \"ran locally\" and \"single purpose\"),\n  and complete the documentation section. Do not tick a box you have not\n  verified, and do not delete rows that do not apply — the docs gate in CI reads\n  that section and fails when it is missing.\n\n- **Keep the PR description short.** Under 1000 words on top of the template's\n  own text, and usually far less — a few paragraphs. Reviewers read the diff;\n  the description exists to explain what the diff cannot say for itself. This is\n  well below what an agent will produce by default, so cut before you post.\n\n- **Body: why before what.** Lead with the problem and the reason for this\n  approach, then the shape of the change. No bullet list of files changed, no\n  per-function walkthrough, no restating the diff in prose, no trailing summary\n  section, no self-congratulatory closing line.\n\n- **No `Co-Authored-By` or tool-attribution trailers in the PR description**,\n  and none in commits either. Contributors own their contributions. Whatever\n  tooling produced the diff, the person opening the PR is its author: they have\n  read every line, they can explain why it works, they can answer review\n  questions without going back to a model, and they are accountable for the\n  consequences of merging it. Do not add a trailer, footer, or description line\n  that spreads that ownership onto a tool.\n\n- **Commit subjects follow the same `[scope] Subject` convention.** Keep the\n  subject short, and use the body for why before what. No bullet lists of files\n  changed.\n\n- **Push review fixes as separate commits.** The PR is squashed on merge, so\n  there is no reason to rewrite history mid-review; many small commits make the\n  re-review readable.\n\n- **Do not force-push a branch that is under review.** A force-push detaches\n  existing review comments from the lines they were written against, destroys\n  the \"changes since your last review\" diff a reviewer relies on, and discards\n  the CI history that showed which commit broke what. Add commits instead —\n  including for fixups and reverts. Force-push only when there is no\n  alternative: a rebase to clear a genuine conflict, or removing a secret or a\n  large binary that was committed by mistake. When you must, ask the user first,\n  then say so in a PR comment so reviewers know their anchors moved. Never\n  force-push `main`, and never force-push a branch you do not own.\n\n- **One PR, one purpose.** Split refactors out of fixes and fixes out of\n  features.\n\n- **Keep the PR small.** Size is the single strongest predictor of how long a PR\n  waits. Aim for **under ~400 changed lines across under ~20 files**; past\n  roughly **1000 lines or 50 files** a community PR is likely to be sent back to\n  be split, or left unreviewed until it is. Large PRs from outside the core team\n  may be blocked outright when the size was never agreed in the ticket —\n  reviewing a sprawling change against a privileged networking daemon is a\n  security risk in itself, not just a time cost.\n\n  Judge the size by hand-written code: exclude generated output, `go.sum`,\n  vendored files, and test fixtures from the estimate, but do not use their\n  presence to argue a 3000-line PR is small.\n\n  When a change genuinely cannot be small — a protocol migration, a\n  cross-component rename — agree the split in the ticket **before** writing\n  code, and land it as a sequence of PRs that each build, test, and make sense\n  on their own. Propose that split to the user rather than opening one large PR\n  and hoping.\n\n  Prefer GitHub's stacked pull requests for such a sequence, rather than\n  hand-managing base branches: open each PR against the branch below it instead of\n  `main`, so every PR's diff shows only its own change. Merging a layer retargets\n  the PRs above it, and branch protections and required checks on the base branch\n  still apply to each one.\n\n- **User-facing changes need a docs PR** in\n  [netbirdio/docs](https://github.com/netbirdio/docs), linked from the PR\n  description.\n\n## After you push: CI and review bots\n\nOpening the PR is not the end of the task. Watch the run, read what the bots\nsay, and drive the PR to green before you report the work as done.\n\n```bash\ngh pr checks <pr>            --watch    # all checks, live\ngh run view <run-id> --log-failed       # only the failing steps\ngh pr view <pr> --comments              # bot and human review comments\n```\n\n**Never report a change as finished while checks are pending or red**, and never\ndescribe a red PR as passing. If you ran out of turn before CI finished, say\nwhich checks were still running.\n\n### The checks\n\n- **Go tests** — `golang-test-{linux,darwin,windows,freebsd}.yml`, sharded per\n  component. A failure in a component you did not touch is usually a real\n  interaction, not noise; read the log before assuming flake.\n- **golangci-lint** — `golangci-lint.yml` runs the full repository, while\n  `make lint` only checks your diff. A clean local lint does not guarantee green\n  CI on a large change.\n- **PR Title Check** — `pr-title-check.yml`, see above.\n- **Codecov** — uploaded from the Linux test workflow with per-component flags\n  (`unit,client`, `unit,management`, `unit,relay`, `unit,proxy`, `unit,signal`,\n  `integration,management`). Coverage on new code should not go backwards. Add\n  tests for the paths you introduced; do not adjust thresholds or exclude files\n  to clear the report.\n- **CodeRabbit** — configured in [`.coderabbit.yaml`](.coderabbit.yaml): `chill`\n  profile, auto-review on every non-draft PR, TypeScript/JavaScript/SVG paths\n  filtered out. Chat auto-reply is on, so `@coderabbitai` in a comment reaches\n  it.\n- **SonarCloud** — project `netbirdio_netbird`, quality gate on new code (bugs,\n  vulnerabilities, code smells, duplication, coverage).\n- **Snyk** — dependency and code scanning.\n\nSonar and Snyk report as GitHub App checks rather than workflows in this\nrepository, so their detail lives on the PR check, not in the Actions logs.\n\n### Handling bot findings\n\n- **Read every comment and act on it.** Either fix it, or reply with the reason\n  it does not apply. Do not bulk-resolve threads to clear the count, and do not\n  silently ignore a finding because the check is advisory.\n- **Bots are frequently wrong here.** NetBird has privileged, platform-specific,\n  and concurrency-heavy code that static analysis reads poorly. A confident\n  CodeRabbit or Sonar comment can still be nonsense. Verify the claim against\n  the code before you change anything — never edit correct code just to silence\n  a bot.\n- **Security findings get the opposite default.** For a Snyk or Sonar\n  vulnerability, or a CodeRabbit comment about authentication, authorization,\n  certificate verification, or key handling, assume it is real until you have\n  disproved it. Surface it to the user rather than dismissing it yourself.\n- **A new vulnerable dependency is a stop.** Bumping or replacing dependencies\n  needs the user's decision, as above.\n- **Never change a workflow, threshold, lint exclusion, or bot config to make a\n  check pass.** If a check is genuinely wrong, say so and let the user decide.\n- **Do not paper over flakes with blind re-runs.** Identify the failure first. If\n  it is a known flake, name it; if you cannot tell, report it as unresolved\n  rather than re-running until it goes green.\n\n## Discussion and support\n\n- Discussions: <https://github.com/netbirdio/netbird/discussions>\n- Slack: <https://docs.netbird.io/slack-url>\n- Docs: <https://docs.netbird.io>\n- Security: <https://github.com/netbirdio/netbird/security/policy> — never in public\n- Contribution process: [CONTRIBUTING.md](CONTRIBUTING.md)\n"},"items":[{"name":"CLAUDE.md","path":"CLAUDE.md","title":"CLAUDE.md","content":"See [AGENTS.md](AGENTS.md) for the agent guidelines in this repository.\n","category":"root","tokens":18},{"name":"AGENTS.md","path":"AGENTS.md","title":"AGENTS.md","content":"# NetBird Agent Guidelines\n\n**NetBird** is an open source connectivity platform: a WireGuard®-based overlay\nnetwork with a control plane. The **agent** (`client/`) runs on user machines as\na privileged daemon and manages the WireGuard interface, routing, firewall, and\nDNS. **Management** (`management/`) is the control plane and REST/gRPC API,\n**Signal** (`signal/`) brokers peer handshakes, **Relay** (`relay/`) carries\ntraffic when a direct tunnel is impossible, and **Proxy** (`proxy/`) is the\nidentity-aware proxy behind Agent Network.\n\nThis file applies to the whole repository, and is the single source of truth for\nagent guidance here. `CLAUDE.md` is a one-line pointer to it — keep the guidance\nin this file, not duplicated there.\n\n## Contents\n\n- [STOP and ask the user before](#stop-and-ask-the-user-before)\n- [Quick reference](#quick-reference)\n- [Structure](#structure)\n- [Where to look](#where-to-look)\n- [Security](#security)\n- [Agent conventions](#agent-conventions)\n- [Repo-wide principles](#repo-wide-principles)\n- [Type safety](#type-safety)\n- [Concurrency and lifecycle](#concurrency-and-lifecycle)\n- [Error handling](#error-handling)\n- [Comments](#comments)\n- [Testing](#testing)\n- [Pitfalls](#pitfalls)\n- [Commits, PRs, releases](#commits-prs-releases)\n- [After you push: CI and review bots](#after-you-push-ci-and-review-bots)\n- [Discussion and support](#discussion-and-support)\n\n## STOP and ask the user before\n\n- **Opening a pull request for anything beyond a trivial fix, without an agreed\n  ticket.** Ask the user directly: *\"Is there a discussion or issue for this\n  change?\"* NetBird is discussion-first — community reports start in\n  [Discussions](https://github.com/netbirdio/netbird/discussions), DevRel\n  validates them, and only validated discussions become issues. A PR that\n  changes behavior with no linked issue may be closed on arrival. If there is no\n  ticket, offer to draft the discussion post **instead of** the PR, and wait for\n  the user's call. Only typos, broken links, documentation corrections, and\n  one-line fixes that already have an issue can skip this.\n- **Designing in any high-risk area** (see\n  [CONTRIBUTING.md](CONTRIBUTING.md#high-risk-areas)): public API and OpenAPI\n  schema, gRPC protos, behavior existing deployments would notice after an\n  upgrade, peer connectivity (ICE, NAT traversal, relay selection, WireGuard® or\n  Rosenpass key handling), client system integration (routing, firewall, DNS,\n  interface), authentication and authorization, CLI or service flags, config\n  file format, daemon IPC, store schema and migrations, or a new feature. The\n  design gets agreed in the ticket before code is written.\n- **Writing a store migration or changing a persisted model.** Migrations are\n  one-way in the field and both the GORM and pgx paths may need the change.\n- **Hand-editing generated code.** `*.pb.go`, `*.gen.go`, and mocks are outputs.\n  Edit the source (`.proto`, `openapi.yml`) and rerun the matching\n  `generate.sh`.\n- **Adding, removing, or bumping a dependency**, and never vendor a fork.\n- **Weakening a security control** — authentication, authorization, certificate\n  verification, privilege dropping, or peer identity checks — even when it is\n  the fastest way to make a test pass.\n- **Force-pushing to `main`**, force-pushing any branch that is already under\n  review, amending pushed commits, or bypassing hooks with `--no-verify`.\n\n## Quick reference\n\n```bash\n# Build\ngo build ./...\ncd client && CGO_ENABLED=0 go build .        # agent\ncd management && go build .                  # management service\ncd signal && go build .                      # signal service\n\n# Verify (run before every push)\ngo fmt ./...\nmake lint            # golangci-lint on files changed vs origin/main (also the pre-push hook)\nmake lint-all        # full-repository lint, matches CI\nmake test-unit       # host-safe unit tests, -tags devcert, no sudo\nmake test-privileged # privileged-tagged suite in a Docker container with NET_ADMIN\nmake setup-hooks     # wire make lint into .githooks/pre-push\n\n# Narrow runs\ngo test ./client/internal/dns/...\ngo test -race -run TestPeerConn ./client/internal/peer/...\nPRIV_RUN=TestNftablesManager PRIV_PKGS=./client/firewall/nftables/... make test-privileged\n\n# Code generation (never hand-edit the output)\n./shared/management/http/api/generate.sh   # REST types from openapi.yml\n./shared/management/proto/generate.sh\n./shared/signal/proto/generate.sh\n./client/proto/generate.sh\n./flow/proto/generate.sh\n\n# Run locally (lab only, never on a machine you rely on)\nsudo ./client/netbird up --log-level debug --log-file console\nsudo ./client/netbird down                   # teardown: restores routing, firewall, DNS\n./signal/signal run --log-level debug --log-file console\n./management/management management --log-level debug --log-file console --config ./management.json\n```\n\n`netbird up` needs root and rewrites the host's routing table, firewall rules,\nDNS configuration, and WireGuard® interface. Run it only in a disposable test\nenvironment (a VM, container, or throwaway host) that you can rebuild, never on\na workstation or server whose connectivity matters. Run `sudo netbird down`\nbefore you stop working, before rebuilding the binary, and on every failure\npath, so the host's networking state is restored instead of left half-applied.\nSee [Pitfalls](#pitfalls) for why cleanup on every exit path matters.\n\n## Structure\n\n```text\nnetbird/\n├── client/              NetBird agent\n│   ├── cmd/             agent CLI\n│   ├── internal/        agent business logic (engine, peer, dns, routemanager, ...)\n│   ├── server/          daemon for background execution\n│   ├── proto/           daemon gRPC protos\n│   ├── iface/           WireGuard® interface management\n│   ├── firewall/        nftables, iptables, pf, WFP, userspace backends\n│   ├── ssh/             built-in SSH server and client\n│   ├── ui/              desktop UI (Wails v3 + React)\n│   ├── android/, ios/   mobile bindings\n│   ├── wasm/            WebAssembly build\n│   └── mdm/, system/    MDM policy, host information\n├── management/          control plane\n│   └── server/          account, peer, groups, networks, posture, permissions,\n│                        settings, store, http (REST), idp, integrations, migration\n├── signal/              handshake broker (peer/, server/)\n├── relay/               relay service (protocol/, server/, healthcheck/)\n├── proxy/               identity-aware proxy (llm/, acme/, accesslog/, middleware/, tcp/, udp/)\n├── agent-network/       Agent Network overview\n├── shared/              imported by both agent and services\n│   ├── management/      proto/, client/, http/api (OpenAPI + generated types)\n│   ├── signal/          proto/, client/\n│   └── relay/, auth/, sshauth/, metrics/\n├── e2e/                 end-to-end suites and harness\n├── encryption/, dns/, route/, stun/, sharedsock/, util/, flow/\n├── infrastructure_files/  docker compose and getting-started templates\n└── release_files/       files packaged into releases\n```\n\n## Where to look\n\n| Task                        | Location                                                     |\n| --------------------------- | ------------------------------------------------------------ |\n| REST API / OpenAPI          | `shared/management/http/api/` + `management/server/http/`    |\n| Management gRPC protocol    | `shared/management/proto/`                                   |\n| Signal protocol             | `shared/signal/proto/`                                       |\n| Daemon IPC protocol         | `client/proto/`                                              |\n| Peer connection and NAT     | `client/internal/peer/`                                      |\n| Network map handling        | `client/internal/engine.go`, `shared/management/networkmap/` |\n| Routing                     | `client/internal/routemanager/`, `route/`                    |\n| Firewall backends           | `client/firewall/`                                           |\n| DNS                         | `client/internal/dns/`, `dns/`                               |\n| WireGuard® interface        | `client/iface/`                                              |\n| Persistence and migrations  | `management/server/store/`, `management/server/migration/`   |\n| IdP integrations            | `management/server/idp/`                                     |\n| Permissions model           | `management/server/permissions/`                             |\n| LLM routing / Agent Network | `proxy/internal/llm/`, `agent-network/`                      |\n| End-to-end tests            | `e2e/`                                                       |\n\n## Security\n\n### Never fail open\n\nWhen a security check — access control, an IP restriction, an auth decision —\nhits an error such as an unparseable value, an unavailable lookup, or a state it\ndoes not recognize, it must **deny**. Never skip the check or allow the request\nthrough because the check itself failed, and make the `default` and unknown cases\nof a security-related `switch` deny rather than fall through.\n\n### Daemon RPC input is untrusted\n\nThe agent runs as root (LocalSystem on Windows), so a daemon RPC crosses a\nprivilege boundary: treat every field as untrusted input rather than as something\nthe UI or CLI validated on the way in.\n\nWhen you add or change an RPC, ask what the handler does with caller input while\nrunning as root. If the answer touches a filesystem path, a URL or host, or a\nprivileged state change, it needs a gate **in the handler** — a check in the client\nthat normally calls it is not a check at all.\n\n- **A caller-supplied path the daemon opens.** Never `os.Open` it as root.\n  Constrain it, then open it *as the caller* with `ipcauth.OpenOwnedFile`, which\n  opens `O_NOFOLLOW`, requires a regular file, and refuses a file the caller does\n  not own — so a symlink or hardlink aimed at a root-only file is rejected.\n- **A caller-supplied URL or host the daemon fetches.** Restrict the scheme and\n  allow only known hosts for unprivileged callers. Prefer a lexical host\n  allowlist plus TLS verification over \"resolve the host, then reject private\n  IPs\": the resolve-then-trust pattern has a DNS-rebinding race (public IP at\n  check time, attacker IP at connect time), while a name allowlist has no IP\n  check to race. Never accept `http://` where `https://` is expected.\n- **A privileged state change** (SSH root login, management URL, deregistration)\n  gates on the caller identity from `ipcauth.CallerIdentity(ctx)`.\n\nCaller identity comes from the kernel — `SO_PEERCRED`, `LOCAL_PEERCRED`, or the\nnamed-pipe client token — and never from an RPC field. When\n`ipcauth.CallerIdentity` reports that it could not determine an identity, **deny**;\ndo not fall back to treating the caller as the transport peer.\n\n## Agent conventions\n\n### Three networking modes\n\nWhere packets actually flow depends on the mode the agent is running in. The\nthree are not interchangeable, so establish which one a change applies to — and\nwhat it should do in the other two — before you write it.\n\n- **kernel mode** (Linux only): in-kernel WireGuard®. The kernel handles both\n  peer-to-peer and routed traffic, and ACLs are iptables or nftables rules. The\n  client programs kernel facilities but never sees the traffic itself.\n- **userspace mode** (wireguard-go with a TUN): wireguard-go runs in-process. The\n  kernel handles peer-to-peer traffic once it leaves the TUN, while routed traffic\n  — exit nodes and network routes — goes through the userspace forwarder, which\n  terminates the connection and re-establishes it over OS sockets. Used on\n  platforms without kernel WireGuard® or when the user opts out.\n- **netstack mode**: wireguard-go in-process with no TUN and no kernel\n  networking. The forwarder does all routing by stitching userspace sockets, and\n  listeners such as the embedded SSH and DNS servers bind on a gVisor netstack.\n  Used where the process cannot create a TUN device, such as the embedded client\n  (`client/embed/`) and the WASM build.\n\n### The overlay interface is not \"WireGuard\"\n\nDo not put \"WireGuard\" in identifiers or comments unless the code is genuinely\ncoupled to WireGuard® specifically — a wireguard-go call, a handshake field, a\nkernel WireGuard® netlink attribute. For the interface, the host, peers, or\ntraffic in general, say \"the NetBird interface\", \"the interface\", or \"the overlay\".\nMost firewall, routing, and DNS code is transport-agnostic, so a WireGuard®\nreference there is simply inaccurate and rots as the transports change.\n\n### IPv6 is a soft feature\n\nThe IPv6 overlay is opt-in dual-stack, and capability can change at runtime. Treat\nit as soft rather than a requirement:\n\n- Gate local v6 paths on the interface accessor (`wgIface.Address().HasIPv6()`),\n  not on raw state fields, and skip the v6 path when the host has no v6 rather\n  than returning an error.\n- Treat an empty or unparseable peer v6 address as \"no v6 for that peer\" and skip\n  it, keeping the v4 path working.\n- Never let a missing v6 break v4. Fail-closed is for security checks; a\n  capability mismatch skips the v6 work and carries on.\n\n### Environment variables\n\nName the variable in a constant and parse booleans with `strconv.ParseBool` rather\nthan comparing strings inline, so an unexpected value is logged instead of\nsilently meaning false:\n\n```go\nconst EnvDisableFeature = \"NB_DISABLE_FEATURE\"\n\nfunc isDisabledByEnv() bool {\n    val := os.Getenv(EnvDisableFeature)\n    if val == \"\" {\n        return false\n    }\n    disabled, err := strconv.ParseBool(val)\n    if err != nil {\n        log.Warnf(\"failed to parse %s: %v\", EnvDisableFeature, err)\n        return false\n    }\n    return disabled\n}\n```\n\n### Validating against protocol specs\n\nWhen a change depends on what a protocol actually mandates, read the specification\ntext from the [IETF datatracker](https://datatracker.ietf.org/) rather than a\nsummary, and check that you have the current RFC — the widely cited one for a\nprotocol is often superseded. Cite the section, not just the document, so a\nreviewer can jump straight to the rule.\n\n## Repo-wide principles\n\n1. **Run `go fmt` on every modified Go file.** Formatting is not optional.\n2. **Zero unaddressed linter warnings.** Fix what `golangci-lint` reports on code\n   you touch, and delete imports, helpers, and parameters your refactor orphaned.\n   Exception: unused parameters in shared code may be consumed by builds outside\n   this repository — do not remove them, ask instead.\n3. **Function comments are mandatory for exported functions**, written as full\n   sentences with a period, starting with the identifier name.\n4. **Prefer private functions and constants.** Export only what a caller outside\n   the package genuinely needs.\n5. **Early returns and guard clauses.** Handle errors and edge cases first\n   instead of nesting `if`/`else` chains.\n6. **Split complex functions.** If a function trips a complexity warning, break\n   it into named helpers rather than silencing the warning.\n7. **Avoid LLM-slop tells:** em dashes, hedging narration, restating the diff in\n   prose, trailing summaries. Defaults, not absolute bans. Applies to code,\n   comments, commit messages, and PR descriptions alike.\n8. **Concurrency: do a two-pass race analysis after every change** that touches\n   shared state, including reads of existing maps and slices. Guard them with a\n   mutex (or an atomic or channel where that fits better), keep critical\n   sections short, and run `go test -race` on the touched packages. See\n   [Concurrency and lifecycle](#concurrency-and-lifecycle) for the failure modes\n   to check for.\n9. **Cross-platform builds must keep working.** The agent targets Linux, macOS,\n   Windows, FreeBSD, Android, and iOS. When you add a platform-specific file,\n   add the counterpart or a build-tagged fallback for the others.\n10. **Never hand-edit generated files.** Change the source and regenerate.\n11. **Never log secrets** — private keys, setup keys, tokens, PAT values — and\n    keep peer IPs and hostnames out of logs above debug level.\n\n## Type safety\n\n**No bare primitives for domain concepts.** A `string` parameter for an account\nID next to a `string` parameter for a peer ID is two bugs waiting to happen,\nbecause the compiler cannot catch the swap. Declare the type once and use it\nthroughout, converting only at the boundaries where data enters or leaves —\nprotobuf, gRPC, HTTP, an external library.\n\n```go\ntype ServiceID string\ntype AccountID string\n\n// Internal: typed all the way through\nfunc (r *Router) RemoveRoute(host SNIHost, svcID ServiceID) { ... }\n\n// Proto boundary: convert once, on the way in and on the way out\nsvcID := ServiceID(mapping.GetId())\nreq.ServiceId = string(svcID)\n```\n\n- **IP addresses are `netip.Addr`**, not `string` and not `net.IP`. Parse at the\n  boundary and pass the typed value inward.\n- **Always `Unmap()`** after parsing an address, after converting from `net.IP`,\n  and after extracting one from `RemoteAddr()`. This normalizes a v4-mapped v6\n  address (`::ffff:10.1.2.3`) to plain v4 so IPv4 rules match it. A stored or\n  compared mapped address silently fails to match those rules.\n- **Ports are `uint16`** internally; use `int` only where a library forces it and\n  convert immediately.\n- **Enums are a typed string with constants**, so the valid set is discoverable\n  and a typo fails to compile.\n- **Map keys follow the same rule**, and must be a real type (`type ServiceID\n  string`) rather than an alias (`type serviceID = string`) — an alias silently\n  accepts bare strings.\n\n## Concurrency and lifecycle\n\nBeyond the mutex hygiene in the principles above, check for these failure\nmodes.\n\n- **Never read a struct field inside a goroutine** when another goroutine may nil\n  or reassign it. Pass the value as a parameter, or capture it into a local before\n  launching. This matters most when `Stop()` nils a field without waiting for the\n  goroutine to finish.\n\n  ```go\n  go func(ifaceName string) {   // good: passed in, cannot be nilled underneath\n      m.Start(ctx, ifaceName)\n  }(iface.Name())\n  ```\n\n- **Never wait on a channel while holding a lock the sender needs.** Copy what you\n  need out from under the lock, release it, then wait.\n\n  ```go\n  func (m *Manager) Stop() {\n      m.mu.Lock()\n      cancel, done := m.cancel, m.done\n      m.mu.Unlock()\n      if cancel != nil {\n          cancel()\n          <-done\n      }\n  }\n  ```\n\n- **`Stop`/`Close` must be idempotent** — guard on an already-stopped flag or a\n  nil cancel — and must release the state they guarded. Clear maps and caches;\n  a cancelled goroutine holding a live map still pins that memory. Note that a\n  nil map only panics on writes; reads and iteration behave like an empty map,\n  so where post-close use must be rejected, check the stopped flag explicitly.\n- **Publish coupled state only after every fallible step succeeds.** When several\n  fields form an invariant, build them into locals and assign them to the receiver\n  at the end. Assigning as you go leaves the object half-initialized when a later\n  step fails, so a readiness predicate reports ready while a coupled field is nil.\n  If an earlier step already had an external side effect — a created chain, an\n  opened handle, an inserted rule — roll it back before returning the error.\n- **Clean up what you own on constructor error paths.** Once a constructor has\n  started something, every later error path must undo it: cancel a goroutine and\n  wait for it to exit, stop a ticker, close a watcher. The object is never\n  returned, so its `Close` will never run.\n- **A failed `Start` must undo everything it started.** When a component brings up\n  several subsystems in sequence — connection manager, watchers, routing, DNS,\n  flow, persisted state — a failure partway through has to tear down the ones\n  already running, not just close the handle the error came from. Put the\n  already-started guard *before* that teardown path, so a rejected second `Start`\n  cannot dismantle the one that is running.\n\n## Error handling\n\nUse single-assignment form when the error is only needed inside the `if`:\n\n```go\n// Good\nif err := someCall(); err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n\n// Bad - unnecessary split\nerr := someCall()\nif err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n```\n\nUse multiple assignment when the value is needed after the block:\n\n```go\nresult, err := someCall()\nif err != nil {\n    return fmt.Errorf(\"context: %w\", err)\n}\n```\n\nAdd short, meaningful context, and **do not** start `fmt.Errorf` messages with\nobvious words like \"failed to\" or \"error\":\n\n```go\n// Good\nreturn fmt.Errorf(\"parse remote address: %w\", err)\nreturn fmt.Errorf(\"listen on %s: %w\", addr, err)\n\n// Bad\nreturn fmt.Errorf(\"failed to parse remote address: %w\", err)\nreturn fmt.Errorf(\"error listening on %s: %w\", addr, err)\n\n// \"failed\" is fine in log messages\nlog.Debugf(\"failed to parse remote address: %v\", err)\n```\n\nSkip the wrapping when a function only extracts or delegates and the wrap would\nadd nothing:\n\n```go\nfunc parseAddr(addr string) (string, int, error) {\n    host, portStr, err := net.SplitHostPort(addr)\n    if err != nil {\n        return \"\", 0, err\n    }\n    // ...\n}\n```\n\nLog the errors you choose not to act on:\n\n- `log.Debugf()` for errors that do not affect program flow but help debugging.\n- `log.Tracef()` for very verbose errors that would otherwise spam logs.\n- **Never ignore** errors from writes, network sends, or critical cleanup.\n- Close errors may be ignored for read-only operations; log them at debug for\n  writes.\n\n**Do not log and return the same error.** It gets reported twice, from two places,\nand the second reader cannot tell whether it happened once or twice. Return it and\nlet the caller decide. The exception is an API handler that has already written a\nresponse. Internal helpers return errors rather than logging and swallowing them.\n\n**Never return a typed nil as an error.** A nil `*MyError` stored in an `error`\ninterface is not nil, so `err != nil` is true and callers take the failure path on\nsuccess. Return the error only where it is actually set:\n\n```go\nif _, err := conn.Write(buf); err != nil {   // good\n    return err\n}\nreturn nil\n```\n\n**Accumulate with `multierror` when an operation should continue past individual\nfailures** — teardown, cleanup, or setup where partial success is acceptable.\n`client/errors.FormatErrorOrNil` returns nil for an empty accumulator, so callers\nstill see a plain nil on full success:\n\n```go\nfunc (m *Manager) Cleanup() error {\n    var merr *multierror.Error\n    for _, r := range m.resources {\n        if err := r.Close(); err != nil {\n            merr = multierror.Append(merr, fmt.Errorf(\"close %s: %w\", r.Name, err))\n        }\n    }\n    return nberrors.FormatErrorOrNil(merr)\n}\n```\n\n| Scenario              | Approach              | Why                                       |\n| --------------------- | --------------------- | ----------------------------------------- |\n| Cleanup / teardown    | Accumulate            | Clean up as much as possible              |\n| Setup with rollback   | Abort on first error  | Partial state is invalid; undo what stuck |\n| Setup with partial OK | Accumulate            | Degraded operation is still useful        |\n\n## Comments\n\nComment the **why**, never the **what**. Default to no comment, and add one only\nwhen a hidden constraint or workaround would surprise a future reader. Never\nreference the current task, PR, or your own changes in a comment.\n\n```go\n// Bad - trailing comments explaining the obvious\ndefer localConn.Close() // Close the connection\nif err != nil {         // Check if error occurred\n\n// Good\ndefer localConn.Close()\n\n// Good - explains a non-obvious constraint\n// Use incremental checksum update per RFC 1624 for performance.\nchecksum = updateChecksum(checksum, oldPort, newPort)\n```\n\n### Length budget\n\nNeither of these is linter-enforced, so they are conventions the surrounding code\nmostly follows rather than hard limits:\n\n- **Around 90 characters per line.** Wrap the comment rather than running well past\n  it.\n- **Roughly 250 characters per comment**, about three wrapped lines. Doc comments\n  on exported identifiers may exceed it when the API genuinely needs the\n  explanation; inline comments inside a function body rarely should.\n\nThe budget is a smell detector, not a rule to game. Do not compress a needed\nexplanation into cryptic shorthand to fit — if a block of code needs more than\n250 characters of prose, the code is doing too much. Fix the code:\n\n- **Extract a named function.** A well-named function replaces the comment: the\n  name says *what*, the body shows *how*, and the comment you no longer write\n  was the *what* anyway. Clean Code calls this \"explain yourself in code\".\n- **Extract a named constant or predicate.** `if isExpiredSetupKey(key)` needs\n  no comment; `if key.ExpiresAt.Before(now) && !key.Revoked && key.UsageLimit > 0`\n  does.\n- **Keep the surviving comment for the why** — the RFC, the kernel quirk, the\n  ordering constraint. That part is usually one or two lines.\n\n### Long switch and if/else chains\n\nA `switch` whose cases carry multi-line explanations is the usual place this\nbudget is breached, and the comment is a symptom. In order of preference:\n\n1. **Extract each case body into a named function.** The case becomes one line,\n   the name carries the meaning, and the switch reads as a table of contents.\n2. **Replace the switch with a lookup table** — `map[Kind]handlerFunc` — when the\n   branches are uniform. Adding a case stops meaning editing a growing function.\n3. **Replace conditional with polymorphism** when branches vary by type and the\n   same switch shape starts appearing in more than one place. Clean Code's rule\n   of thumb: tolerate a switch statement if it appears **once**, is buried in a\n   factory that returns an interface, and no other switch dispatches on the same\n   type. A second switch over the same enum is the signal to introduce the\n   interface.\n\nDo not restructure a switch purely to satisfy the budget when the cases are one\nline each and self-evident — a flat, boring `switch` over an enum is fine and\nneeds no comments at all.\n\nExplanatory comments in tests are welcome — they document the scenario being set\nup, and the 250-character budget does not apply to them.\n\n## Testing\n\n- **Unit tests** live beside the code as `_test.go`. `make test-unit` runs the\n  host-safe set with `-tags devcert` and no sudo.\n- **Privileged tests** carry the `privileged` build tag and mutate host\n  networking. They run through `make test-privileged`, inside a Docker container\n  with `NET_ADMIN`. Never bypass that harness by running them directly on the\n  host.\n- **End-to-end suites** live in `e2e/` with a shared harness.\n- **Test real behavior, not API existence.** Assert on the observable end state\n  a consumer would see — bytes that arrived, the packet after translation, the\n  row after the write — not merely that a method exists or returns an error.\n- **Avoid mocks for code we own.** Exercise the real store, manager, or\n  controller and assert what the caller actually receives.\n- **`require` for setup and preconditions, `assert` for the conditions under\n  test.** Use `require` whenever a later line would panic or be meaningless\n  otherwise.\n- **Message guidance:** optional for `NoError`/`Error`; always give context for\n  comparison, boolean, and collection assertions.\n- **Reproduce a bug before fixing it.** Write the test, watch it fail *for the\n  reason you expect* — a test that fails for an unrelated reason proves nothing —\n  then apply the fix and confirm it passes. Add the thin surrounding cases while\n  you are there.\n- **Use `t.Setenv`** rather than `os.Setenv` so the previous value is restored on\n  cleanup. To test the unset case, call `t.Setenv` first to register the restore,\n  then `os.Unsetenv`.\n- **Prefer `t.Cleanup` over `defer`** in any test with parallel subtests: the\n  parent function returns, running its `defer`s, while parallel subtests are\n  still suspended. Sequential subtests finish inside `t.Run`, so `defer` is safe\n  there, but `t.Cleanup` works in both cases.\n- **Explanatory comments in tests are welcome.** Describe the scenario being set\n  up; the comment budget below does not apply to them.\n\n```go\nserver, err := StartTestServer()\nrequire.NoError(t, err, \"Test server setup must succeed\")\ndefer server.Close()\n\nresult, err := client.DoOperation()\nassert.NoError(t, err)\nassert.Equal(t, expectedResult, result, \"Result should match expected\")\n```\n\n## Pitfalls\n\n- **The agent runs as root.** Anything touching routing, firewall, DNS, or the\n  interface can take a user's machine off the network. Prefer a reversible\n  change and make sure cleanup runs on every exit path.\n- **Management has two account loaders** (GORM and pgx). Adding a relation to an\n  account often means updating both, or it silently comes back empty in\n  production.\n- **`go test ./...` without `-tags devcert` skips tests** that need the\n  development certificate. Use `make test-unit`.\n- **`make lint` only checks the diff against `origin/main`.** CI runs\n  `make lint-all`; run it too before pushing a large change.\n- **Protos are consumed by released clients.** An old agent must keep working\n  against a new Management, so fields are added, never renumbered or removed.\n- **Windows requires the wintun driver**, and the daemon serves a named pipe\n  (`npipe://netbird`) rather than loopback TCP. Loopback TCP carries no caller\n  identity, so privileged operations are refused over it.\n\n## Commits, PRs, releases\n\n- **PR titles must start with a bracketed tag.** Before you propose a title,\n  **read [`.github/workflows/pr-title-check.yml`](.github/workflows/pr-title-check.yml)\n  and take the allowed tags from the `allowedTags` array in that file.** It is\n  the only source of truth, it changes as components are added, and the check\n  runs on every title edit — a tag that is not in that array is a red build. Do\n  not rely on a list memorized from anywhere else, including this file.\n\n  ```text\n  [client] Authorize daemon IPC callers by their local identity\n  [management,client] Add MDM policy support\n  ```\n\n  Multiple tags are comma-separated inside one pair of brackets. Match the tag\n  to the component you actually changed, not to the one you read the most.\n\n- **Use the repository's PR template.** Fill in\n  [`.github/pull_request_template.md`](.github/pull_request_template.md) rather\n  than replacing it with your own summary: describe the change, link the issue,\n  tick the checklist honestly (including \"ran locally\" and \"single purpose\"),\n  and complete the documentation section. Do not tick a box you have not\n  verified, and do not delete rows that do not apply — the docs gate in CI reads\n  that section and fails when it is missing.\n\n- **Keep the PR description short.** Under 1000 words on top of the template's\n  own text, and usually far less — a few paragraphs. Reviewers read the diff;\n  the description exists to explain what the diff cannot say for itself. This is\n  well below what an agent will produce by default, so cut before you post.\n\n- **Body: why before what.** Lead with the problem and the reason for this\n  approach, then the shape of the change. No bullet list of files changed, no\n  per-function walkthrough, no restating the diff in prose, no trailing summary\n  section, no self-congratulatory closing line.\n\n- **No `Co-Authored-By` or tool-attribution trailers in the PR description**,\n  and none in commits either. Contributors own their contributions. Whatever\n  tooling produced the diff, the person opening the PR is its author: they have\n  read every line, they can explain why it works, they can answer review\n  questions without going back to a model, and they are accountable for the\n  consequences of merging it. Do not add a trailer, footer, or description line\n  that spreads that ownership onto a tool.\n\n- **Commit subjects follow the same `[scope] Subject` convention.** Keep the\n  subject short, and use the body for why before what. No bullet lists of files\n  changed.\n\n- **Push review fixes as separate commits.** The PR is squashed on merge, so\n  there is no reason to rewrite history mid-review; many small commits make the\n  re-review readable.\n\n- **Do not force-push a branch that is under review.** A force-push detaches\n  existing review comments from the lines they were written against, destroys\n  the \"changes since your last review\" diff a reviewer relies on, and discards\n  the CI history that showed which commit broke what. Add commits instead —\n  including for fixups and reverts. Force-push only when there is no\n  alternative: a rebase to clear a genuine conflict, or removing a secret or a\n  large binary that was committed by mistake. When you must, ask the user first,\n  then say so in a PR comment so reviewers know their anchors moved. Never\n  force-push `main`, and never force-push a branch you do not own.\n\n- **One PR, one purpose.** Split refactors out of fixes and fixes out of\n  features.\n\n- **Keep the PR small.** Size is the single strongest predictor of how long a PR\n  waits. Aim for **under ~400 changed lines across under ~20 files**; past\n  roughly **1000 lines or 50 files** a community PR is likely to be sent back to\n  be split, or left unreviewed until it is. Large PRs from outside the core team\n  may be blocked outright when the size was never agreed in the ticket —\n  reviewing a sprawling change against a privileged networking daemon is a\n  security risk in itself, not just a time cost.\n\n  Judge the size by hand-written code: exclude generated output, `go.sum`,\n  vendored files, and test fixtures from the estimate, but do not use their\n  presence to argue a 3000-line PR is small.\n\n  When a change genuinely cannot be small — a protocol migration, a\n  cross-component rename — agree the split in the ticket **before** writing\n  code, and land it as a sequence of PRs that each build, test, and make sense\n  on their own. Propose that split to the user rather than opening one large PR\n  and hoping.\n\n  Prefer GitHub's stacked pull requests for such a sequence, rather than\n  hand-managing base branches: open each PR against the branch below it instead of\n  `main`, so every PR's diff shows only its own change. Merging a layer retargets\n  the PRs above it, and branch protections and required checks on the base branch\n  still apply to each one.\n\n- **User-facing changes need a docs PR** in\n  [netbirdio/docs](https://github.com/netbirdio/docs), linked from the PR\n  description.\n\n## After you push: CI and review bots\n\nOpening the PR is not the end of the task. Watch the run, read what the bots\nsay, and drive the PR to green before you report the work as done.\n\n```bash\ngh pr checks <pr>            --watch    # all checks, live\ngh run view <run-id> --log-failed       # only the failing steps\ngh pr view <pr> --comments              # bot and human review comments\n```\n\n**Never report a change as finished while checks are pending or red**, and never\ndescribe a red PR as passing. If you ran out of turn before CI finished, say\nwhich checks were still running.\n\n### The checks\n\n- **Go tests** — `golang-test-{linux,darwin,windows,freebsd}.yml`, sharded per\n  component. A failure in a component you did not touch is usually a real\n  interaction, not noise; read the log before assuming flake.\n- **golangci-lint** — `golangci-lint.yml` runs the full repository, while\n  `make lint` only checks your diff. A clean local lint does not guarantee green\n  CI on a large change.\n- **PR Title Check** — `pr-title-check.yml`, see above.\n- **Codecov** — uploaded from the Linux test workflow with per-component flags\n  (`unit,client`, `unit,management`, `unit,relay`, `unit,proxy`, `unit,signal`,\n  `integration,management`). Coverage on new code should not go backwards. Add\n  tests for the paths you introduced; do not adjust thresholds or exclude files\n  to clear the report.\n- **CodeRabbit** — configured in [`.coderabbit.yaml`](.coderabbit.yaml): `chill`\n  profile, auto-review on every non-draft PR, TypeScript/JavaScript/SVG paths\n  filtered out. Chat auto-reply is on, so `@coderabbitai` in a comment reaches\n  it.\n- **SonarCloud** — project `netbirdio_netbird`, quality gate on new code (bugs,\n  vulnerabilities, code smells, duplication, coverage).\n- **Snyk** — dependency and code scanning.\n\nSonar and Snyk report as GitHub App checks rather than workflows in this\nrepository, so their detail lives on the PR check, not in the Actions logs.\n\n### Handling bot findings\n\n- **Read every comment and act on it.** Either fix it, or reply with the reason\n  it does not apply. Do not bulk-resolve threads to clear the count, and do not\n  silently ignore a finding because the check is advisory.\n- **Bots are frequently wrong here.** NetBird has privileged, platform-specific,\n  and concurrency-heavy code that static analysis reads poorly. A confident\n  CodeRabbit or Sonar comment can still be nonsense. Verify the claim against\n  the code before you change anything — never edit correct code just to silence\n  a bot.\n- **Security findings get the opposite default.** For a Snyk or Sonar\n  vulnerability, or a CodeRabbit comment about authentication, authorization,\n  certificate verification, or key handling, assume it is real until you have\n  disproved it. Surface it to the user rather than dismissing it yourself.\n- **A new vulnerable dependency is a stop.** Bumping or replacing dependencies\n  needs the user's decision, as above.\n- **Never change a workflow, threshold, lint exclusion, or bot config to make a\n  check pass.** If a check is genuinely wrong, say so and let the user decide.\n- **Do not paper over flakes with blind re-runs.** Identify the failure first. If\n  it is a known flake, name it; if you cannot tell, report it as unresolved\n  rather than re-running until it goes green.\n\n## Discussion and support\n\n- Discussions: <https://github.com/netbirdio/netbird/discussions>\n- Slack: <https://docs.netbird.io/slack-url>\n- Docs: <https://docs.netbird.io>\n- Security: <https://github.com/netbirdio/netbird/security/policy> — never in public\n- Contribution process: [CONTRIBUTING.md](CONTRIBUTING.md)\n","category":"root","tokens":9556}]}