netbird/AGENTS.md

NetBird Agent Guidelines

NetBird is an open-source connectivity platform: a WireGuard®-based overlay network with a control plane. The agent (client/) runs on user machines as a privileged daemon and manages the WireGuard interface, routing, firewall, and DNS. Management (management/) is the control plane and REST/gRPC API, Signal (signal/) brokers peer handshakes, Relay (relay/) carries traffic when a direct tunnel is impossible, and Proxy (proxy/) is the identity-aware proxy behind Agent Network.

This file applies to the whole repository, and is the single source of truth for agent guidance here. CLAUDE.md is a one-line pointer to it — keep the guidance in this file, not duplicated there.

Contents

• NetBird Agent Guidelines
  • Contents
  • STOP and ask the user before
  • Quick reference
  • Structure
  • Where to look
  • Repo-wide principles
  • Error handling
  • Comments
  • Testing
  • Pitfalls
  • Commits, PRs, releases
  • After you push: CI and review bots
  • Discussion and support

STOP and ask the user before

• Opening a pull request for anything beyond a trivial fix, without an agreed ticket. Ask the user directly: "Is there a discussion or issue for this change?" NetBird is discussion-first — community reports start in Discussions, DevRel validates them, and only validated discussions become issues. A PR that changes behavior with no linked issue may be closed on arrival. If there is no ticket, offer to draft the discussion post instead of the PR, and wait for the user's call. Only typos, broken links, documentation corrections, and one-line fixes that already have an issue can skip this.
• Designing in any high-risk area (see CONTRIBUTING.md): public API and OpenAPI schema, gRPC protos, behavior existing deployments would notice after an upgrade, peer connectivity (ICE, NAT traversal, relay selection, WireGuard® or Rosenpass key handling), client system integration (routing, firewall, DNS, interface), authentication and authorization, CLI or service flags, config file format, daemon IPC, store schema and migrations, or a new feature. The design gets agreed in the ticket before code is written.
• Writing a store migration or changing a persisted model. Migrations are one-way in the field and both the GORM and pgx paths may need the change.
• Hand-editing generated code. *.pb.go, *.gen.go, and mocks are outputs. Edit the source (.proto, openapi.yml) and rerun the matching generate.sh.
• Adding, removing, or bumping a dependency, and never vendor a fork.
• Weakening a security control — authentication, authorization, certificate verification, privilege dropping, or peer identity checks — even when it is the fastest way to make a test pass.
• Force-pushing to main, force-pushing any branch that is already under review, amending pushed commits, or bypassing hooks with --no-verify.

Quick reference

# Build
go build ./...
cd client && CGO_ENABLED=0 go build .        # agent
cd management && go build .                  # management service
cd signal && go build .                      # signal service

# Verify (run before every push)
go fmt ./...
make lint            # golangci-lint on files changed vs origin/main (also the pre-push hook)
make lint-all        # full-repository lint, matches CI
make test-unit       # host-safe unit tests, -tags devcert, no sudo
make test-privileged # privileged-tagged suite in a Docker container with NET_ADMIN
make setup-hooks     # wire make lint into .githooks/pre-push

# Narrow runs
go test ./client/internal/dns/...
go test -race -run TestPeerConn ./client/internal/peer/...
PRIV_RUN=TestNftablesManager PRIV_PKGS=./client/firewall/nftables/... make test-privileged

# Code generation (never hand-edit the output)
./shared/management/http/api/generate.sh   # REST types from openapi.yml
./shared/management/proto/generate.sh
./shared/signal/proto/generate.sh
./client/proto/generate.sh
./flow/proto/generate.sh

# Run locally (lab only, never on a machine you rely on)
sudo ./client/netbird up --log-level debug --log-file console
sudo ./client/netbird down                   # teardown: restores routing, firewall, DNS
./signal/signal run --log-level debug --log-file console
./management/management management --log-level debug --log-file console --config ./management.json

netbird up needs root and rewrites the host's routing table, firewall rules, DNS configuration, and WireGuard® interface. Run it only in a disposable test environment (a VM, container, or throwaway host) that you can rebuild, never on a workstation or server whose connectivity matters. Run sudo netbird down before you stop working, before rebuilding the binary, and on every failure path, so the host's networking state is restored instead of left half-applied. See Pitfalls for why cleanup on every exit path matters.

Structure

netbird/
├── client/              NetBird agent
│   ├── cmd/             agent CLI
│   ├── internal/        agent business logic (engine, peer, dns, routemanager, ...)
│   ├── server/          daemon for background execution
│   ├── proto/           daemon gRPC protos
│   ├── iface/           WireGuard® interface management
│   ├── firewall/        nftables, iptables, pf, WFP, userspace backends
│   ├── ssh/             built-in SSH server and client
│   ├── ui/              desktop UI (Wails v3 + React)
│   ├── android/, ios/   mobile bindings
│   ├── wasm/            WebAssembly build
│   └── mdm/, system/    MDM policy, host information
├── management/          control plane
│   └── server/          account, peer, groups, networks, posture, permissions,
│                        settings, store, http (REST), idp, integrations, migration
├── signal/              handshake broker (peer/, server/)
├── relay/               relay service (protocol/, server/, healthcheck/)
├── proxy/               identity-aware proxy (llm/, acme/, accesslog/, middleware/, tcp/, udp/)
├── agent-network/       Agent Network overview
├── shared/              imported by both agent and services
│   ├── management/      proto/, client/, http/api (OpenAPI + generated types)
│   ├── signal/          proto/, client/
│   └── relay/, auth/, sshauth/, metrics/
├── e2e/                 end-to-end suites and harness
├── encryption/, dns/, route/, stun/, sharedsock/, util/, flow/
├── infrastructure_files/  docker compose and getting-started templates
└── release_files/       files packaged into releases

Where to look

Task	Location
REST API / OpenAPI	shared/management/http/api/ + management/server/http/
Management gRPC protocol	shared/management/proto/
Signal protocol	shared/signal/proto/
Daemon IPC protocol	client/proto/
Peer connection and NAT	client/internal/peer/
Network map handling	client/internal/engine.go, shared/management/networkmap/
Routing	client/internal/routemanager/, route/
Firewall backends	client/firewall/
DNS	client/internal/dns/, dns/
WireGuard® interface	client/iface/
Persistence and migrations	management/server/store/, management/server/migration/
IdP integrations	management/server/idp/
Permissions model	management/server/permissions/
LLM routing / Agent Network	proxy/internal/llm/, agent-network/
End-to-end tests	e2e/

Repo-wide principles

1. Run go fmt on every modified Go file. Formatting is not optional.
2. Zero unaddressed diagnostics. Fix IDE and linter warnings on code you touch, and delete imports, helpers, and parameters your refactor orphaned. Exception: unused parameters in shared code may be consumed by builds outside this repository — do not remove them, ask instead.
3. Function comments are mandatory for exported functions, written as full sentences with a period, starting with the identifier name.
4. Prefer private functions and constants. Export only what a caller outside the package genuinely needs.
5. Early returns and guard clauses. Handle errors and edge cases first instead of nesting if/else chains.
6. Split complex functions. If a function trips a complexity warning, break it into named helpers rather than silencing the warning.
7. Avoid LLM-slop tells: em dashes, hedging narration, restating the diff in prose, trailing summaries. Defaults, not absolute bans. Applies to code, comments, commit messages, and PR descriptions alike.
8. Concurrency: do a two-pass race analysis after every change that adds shared state. Guard maps and slices with a mutex, keep critical sections short, and run go test -race on the touched packages.
9. Cross-platform builds must keep working. The agent targets Linux, macOS, Windows, FreeBSD, Android, and iOS. When you add a platform-specific file, add the counterpart or a build-tagged fallback for the others.
10. Never hand-edit generated files. Change the source and regenerate.
11. Never log secrets — private keys, setup keys, tokens, PAT values — and keep peer IPs and hostnames out of logs above debug level.

Error handling

Use single-assignment form when the error is only needed inside the if:

// Good
if err := someCall(); err != nil {
    return fmt.Errorf("context: %w", err)
}

// Bad - unnecessary split
err := someCall()
if err != nil {
    return fmt.Errorf("context: %w", err)
}

Use multiple assignment when the value is needed after the block:

result, err := someCall()
if err != nil {
    return fmt.Errorf("context: %w", err)
}

Add short, meaningful context, and do not start fmt.Errorf messages with obvious words like "failed to" or "error":

// Good
return fmt.Errorf("parse remote address: %w", err)
return fmt.Errorf("listen on %s: %w", addr, err)

// Bad
return fmt.Errorf("failed to parse remote address: %w", err)
return fmt.Errorf("error listening on %s: %w", addr, err)

// "failed" is fine in log messages
log.Debugf("failed to parse remote address: %v", err)

Skip the wrapping when a function only extracts or delegates and the wrap would add nothing:

func parseAddr(addr string) (string, int, error) {
    host, portStr, err := net.SplitHostPort(addr)
    if err != nil {
        return "", 0, err
    }
    // ...
}

Log the errors you choose not to act on:

• log.Debugf() for errors that do not affect program flow but help debugging.
• log.Tracef() for very verbose errors that would otherwise spam logs.
• Never ignore errors from writes, network sends, or critical cleanup.
• Close errors may be ignored for read-only operations; log them at debug for writes.

Comments

Comment the why, never the what. Default to no comment, and add one only when a hidden constraint or workaround would surprise a future reader. Never reference the current task, PR, or your own changes in a comment.

// Bad - trailing comments explaining the obvious
defer localConn.Close() // Close the connection
if err != nil {         // Check if error occurred

// Good
defer localConn.Close()

// Good - explains a non-obvious constraint
// Use incremental checksum update per RFC 1624 for performance.
checksum = updateChecksum(checksum, oldPort, newPort)

Length budget

• 90 characters per line. Wrap the comment, do not run past it.
• 250 characters per comment, roughly three wrapped lines. Doc comments on exported identifiers may exceed it when the API genuinely needs the explanation; inline comments inside a function body may not.

The budget is a smell detector, not a rule to game. Do not compress a needed explanation into cryptic shorthand to fit — if a block of code needs more than 250 characters of prose, the code is doing too much. Fix the code:

• Extract a named function. A well-named function replaces the comment: the name says what, the body shows how, and the comment you no longer write was the what anyway. Clean Code calls this "explain yourself in code".
• Extract a named constant or predicate. if isExpiredSetupKey(key) needs no comment; if key.ExpiresAt.Before(now) && !key.Revoked && key.UsageLimit > 0 does.
• Keep the surviving comment for the why — the RFC, the kernel quirk, the ordering constraint. That part is usually one or two lines.

Long switch and if/else chains

A switch whose cases carry multi-line explanations is the usual place this budget is breached, and the comment is a symptom. In order of preference:

1. Extract each case body into a named function. The case becomes one line, the name carries the meaning, and the switch reads as a table of contents.
2. Replace the switch with a lookup table — map[Kind]handlerFunc — when the branches are uniform. Adding a case stops meaning editing a growing function.
3. Replace conditional with polymorphism when branches vary by type and the same switch shape starts appearing in more than one place. Clean Code's rule of thumb: tolerate a switch statement if it appears once, is buried in a factory that returns an interface, and no other switch dispatches on the same type. A second switch over the same enum is the signal to introduce the interface.

Do not restructure a switch purely to satisfy the budget when the cases are one line each and self-evident — a flat, boring switch over an enum is fine and needs no comments at all.

Explanatory comments in tests are welcome — they document the scenario being set up, and the 250-character budget does not apply to them.

Testing

• Unit tests live beside the code as _test.go. make test-unit runs the host-safe set with -tags devcert and no sudo.
• Privileged tests carry the privileged build tag and mutate host networking. They run through make test-privileged, inside a Docker container with NET_ADMIN. Never bypass that harness by running them directly on the host.
• End-to-end suites live in e2e/ with a shared harness.
• Test real behavior, not API existence. Assert on the observable end state a consumer would see — bytes that arrived, the packet after translation, the row after the write — not merely that a method exists or returns an error.
• Avoid mocks for code we own. Exercise the real store, manager, or controller and assert what the caller actually receives.
• require for setup and preconditions, assert for the conditions under test. Use require whenever a later line would panic or be meaningless otherwise.
• Message guidance: optional for NoError/Error; always give context for comparison, boolean, and collection assertions.

server, err := StartTestServer()
require.NoError(t, err, "Test server setup must succeed")
defer server.Close()

result, err := client.DoOperation()
assert.NoError(t, err)
assert.Equal(t, expectedResult, result, "Result should match expected")

Pitfalls

• The agent runs as root. Anything touching routing, firewall, DNS, or the interface can take a user's machine off the network. Prefer a reversible change and make sure cleanup runs on every exit path.
• Management has two account loaders (GORM and pgx). Adding a relation to an account often means updating both, or it silently comes back empty in production.
• go test ./... without -tags devcert skips tests that need the development certificate. Use make test-unit.
• make lint only checks the diff against origin/main. CI runs make lint-all; run it too before pushing a large change.
• Protos are consumed by released clients. An old agent must keep working against a new Management, so fields are added, never renumbered or removed.
• Windows requires the wintun driver, and the daemon serves a named pipe (npipe://netbird) rather than loopback TCP. Loopback TCP carries no caller identity, so privileged operations are refused over it.

Commits, PRs, releases

• PR titles must start with a bracketed tag. Before you propose a title, read .github/workflows/pr-title-check.yml and take the allowed tags from the allowedTags array in that file. It is the only source of truth, it changes as components are added, and the check runs on every title edit — a tag that is not in that array is a red build. Do not rely on a list memorized from anywhere else, including this file.

  [client] Authorize daemon IPC callers by their local identity
  [management,client] Add MDM policy support
  

  Multiple tags are comma-separated inside one pair of brackets. Match the tag to the component you actually changed, not to the one you read the most.

• Use the repository's PR template. Fill in .github/pull_request_template.md rather than replacing it with your own summary: describe the change, link the issue, tick the checklist honestly (including "ran locally" and "single purpose"), and complete the documentation section. Do not tick a box you have not verified, and do not delete rows that do not apply.

• Keep the PR description short. Under 1000 words on top of the template's own text, and usually far less — a few paragraphs. Reviewers read the diff; the description exists to explain what the diff cannot say for itself. This is well below what an agent will produce by default, so cut before you post.

• Body: why before what. Lead with the problem and the reason for this approach, then the shape of the change. No bullet list of files changed, no per-function walkthrough, no restating the diff in prose, no trailing summary section, no self-congratulatory closing line.

• No Co-Authored-By or tool-attribution trailers in the PR description, and none in commits either. Contributors own their contributions. Whatever tooling produced the diff, the person opening the PR is its author: they have read every line, they can explain why it works, they can answer review questions without going back to a model, and they are accountable for the consequences of merging it. Do not add a trailer, footer, or description line that spreads that ownership onto a tool.

• Commit subjects follow the same [scope] Subject convention. Keep the subject short, and use the body for why before what. No bullet lists of files changed.

• Push review fixes as separate commits. The PR is squashed on merge, so there is no reason to rewrite history mid-review; many small commits make the re-review readable.

• Do not force-push a branch that is under review. A force-push detaches existing review comments from the lines they were written against, destroys the "changes since your last review" diff a reviewer relies on, and discards the CI history that showed which commit broke what. Add commits instead — including for fixups and reverts. Force-push only when there is no alternative: a rebase to clear a genuine conflict, or removing a secret or a large binary that was committed by mistake. When you must, ask the user first, then say so in a PR comment so reviewers know their anchors moved. Never force-push main, and never force-push a branch you do not own.

• One PR, one purpose. Split refactors out of fixes and fixes out of features.

• Keep the PR small. Size is the single strongest predictor of how long a PR waits. Aim for under ~400 changed lines across under ~20 files; past roughly 1000 lines or 50 files a community PR is likely to be sent back to be split, or left unreviewed until it is. Large PRs from outside the core team may be blocked outright when the size was never agreed in the ticket — reviewing a sprawling change against a privileged networking daemon is a security risk in itself, not just a time cost.

  Judge the size by hand-written code: exclude generated output, go.sum, vendored files, and test fixtures from the estimate, but do not use their presence to argue a 3000-line PR is small.

  When a change genuinely cannot be small — a protocol migration, a cross-component rename — agree the split in the ticket before writing code, and land it as a sequence of PRs that each build, test, and make sense on their own. Propose that split to the user rather than opening one large PR and hoping.

• User-facing changes need a docs PR in netbirdio/docs, linked from the PR description.

After you push: CI and review bots

Opening the PR is not the end of the task. Watch the run, read what the bots say, and drive the PR to green before you report the work as done.

gh pr checks <pr>            --watch    # all checks, live
gh run view <run-id> --log-failed       # only the failing steps
gh pr view <pr> --comments              # bot and human review comments

Never report a change as finished while checks are pending or red, and never describe a red PR as passing. If you ran out of turn before CI finished, say which checks were still running.

The checks

• Go tests — golang-test-{linux,darwin,windows,freebsd}.yml, sharded per component. A failure in a component you did not touch is usually a real interaction, not noise; read the log before assuming flake.
• golangci-lint — golangci-lint.yml runs the full repository, while make lint only checks your diff. A clean local lint does not guarantee green CI on a large change.
• PR Title Check — pr-title-check.yml, see above.
• Codecov — uploaded from the Linux test workflow with per-component flags (unit,client, unit,management, unit,relay, unit,proxy, unit,signal, integration,management). Coverage on new code should not go backwards. Add tests for the paths you introduced; do not adjust thresholds or exclude files to clear the report.
• CodeRabbit — configured in .coderabbit.yaml: chill profile, auto-review on every non-draft PR, TypeScript/JavaScript/SVG paths filtered out. Chat auto-reply is on, so @coderabbitai in a comment reaches it.
• SonarCloud — project netbirdio_netbird, quality gate on new code (bugs, vulnerabilities, code smells, duplication, coverage).
• Snyk — dependency and code scanning.

Sonar and Snyk report as GitHub App checks rather than workflows in this repository, so their detail lives on the PR check, not in the Actions logs.

Handling bot findings

• Read every comment and act on it. Either fix it, or reply with the reason it does not apply. Do not bulk-resolve threads to clear the count, and do not silently ignore a finding because the check is advisory.
• Bots are frequently wrong here. NetBird has privileged, platform-specific, and concurrency-heavy code that static analysis reads poorly. A confident CodeRabbit or Sonar comment can still be nonsense. Verify the claim against the code before you change anything — never edit correct code just to silence a bot.
• Security findings get the opposite default. For a Snyk or Sonar vulnerability, or a CodeRabbit comment about authentication, authorization, certificate verification, or key handling, assume it is real until you have disproved it. Surface it to the user rather than dismissing it yourself.
• A new vulnerable dependency is a stop. Bumping or replacing dependencies needs the user's decision, as above.
• Never change a workflow, threshold, lint exclusion, or bot config to make a check pass. If a check is genuinely wrong, say so and let the user decide.
• Do not paper over flakes with blind re-runs. Identify the failure first. If it is a known flake, name it; if you cannot tell, report it as unresolved rather than re-running until it goes green.

Discussion and support

• Discussions: https://github.com/netbirdio/netbird/discussions
• Slack: https://docs.netbird.io/slack-url
• Docs: https://docs.netbird.io
• Security: https://github.com/netbirdio/netbird/security/policy — never in public
• Contribution process: CONTRIBUTING.md