Compare commits

..

3 Commits

Author SHA1 Message Date
Viktor Liu
e44d96cc9c Merge remote-tracking branch 'origin/main' into nrpt-cleanup-enumerate
# Conflicts:
#	client/internal/dns/host_windows.go
2026-08-14 09:42:01 +02:00
Viktor Liu
8e3b3c3150 Propagate registry errors other than a missing policy store root
Claude-Session: https://claude.ai/code/session_01Y4KM6AL3cDzUsx3HEHCoxs
2026-08-13 17:15:19 +02:00
Viktor Liu
1da09fcf60 Delete NRPT rules by enumerating the registry instead of a rule count
Claude-Session: https://claude.ai/code/session_01Y4KM6AL3cDzUsx3HEHCoxs
2026-08-13 14:07:38 +02:00
11 changed files with 143 additions and 226 deletions

View File

@@ -1,42 +0,0 @@
name: UI Translations
on:
pull_request:
paths:
- "client/ui/i18n/locales/**"
- "client/ui/i18n/check-translations.mjs"
- ".github/workflows/ui-translations.yml"
push:
branches:
- main
paths:
- "client/ui/i18n/locales/**"
- "client/ui/i18n/check-translations.mjs"
permissions:
contents: read
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.head_ref || github.actor_id }}
cancel-in-progress: true
jobs:
check-translations:
name: Check translation key parity
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- name: Checkout repository
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: "22"
# English (en) is the source of truth for translation keys; every other
# locale declared in _index.json must carry the exact same key set.
- name: Check translation key parity
run: node client/ui/i18n/check-translations.mjs

View File

@@ -204,9 +204,8 @@ func (a *Auth) foregroundGetTokenInfo(authClient *auth.Auth, urlOpener URLOpener
return nil, fmt.Errorf("failed to get OAuth flow: %v", err)
}
// An empty hint is deliberate, not a fallback: a fresh profile leaves the
// choice to the IdP. Switching accounts is done by switching or removing
// profiles, not by logging out — logout keeps the email.
// An empty hint is deliberate, not a fallback: a fresh or logged-out profile
// leaves the choice to the IdP, which is how accounts get switched.
if a.cfgPath != "" {
if hint := readProfileEmail(a.cfgPath); hint != "" {
if setter, ok := oAuthFlow.(loginHintSetter); ok {

View File

@@ -22,8 +22,7 @@ type Profile struct {
ID string
Name string
// Email is the account this profile last logged in with, "" if it never
// completed an SSO login. Kept across logouts; cleared when the profile is
// removed. See profile_state.go.
// completed an SSO login or was logged out. See profile_state.go.
Email string
IsActive bool
}
@@ -201,9 +200,11 @@ func (pm *ProfileManager) LogoutProfile(id string) error {
return fmt.Errorf("failed to save config: %w", err)
}
// The stored account email is kept on purpose, matching the desktop and CLI
// logout semantics: the next login passes it as the login_hint so the IdP
// preselects the account. Removing the profile is what deletes it.
// Not fatal: a stale hint costs an account switch, not the logout itself.
if err := removeProfileEmail(configPath); err != nil {
log.Warnf("failed to clear stored account email for profile %s: %v", id, err)
}
log.Infof("logged out from profile: %s", id)
return nil
}
@@ -223,24 +224,11 @@ func (pm *ProfileManager) RenameProfile(id string, newName string) error {
// RemoveProfile deletes a profile
func (pm *ProfileManager) RemoveProfile(id string) error {
configPath, err := pm.getProfileConfigPath(id)
if err != nil {
return err
}
// Use ServiceManager (removes profile from profiles/ directory)
if err := pm.serviceMgr.RemoveProfile(profilemanager.ID(id), androidUsername); err != nil {
return fmt.Errorf("failed to remove profile: %w", err)
}
// The account file is this package's, not the ServiceManager's, so it must
// go here. The default profile has a fixed filename, so a recreated one
// would otherwise inherit the deleted profile's email as its login_hint.
// Not fatal: the profile itself is gone.
if err := removeProfileEmail(configPath); err != nil {
log.Warnf("failed to remove stored account email for profile %s: %v", id, err)
}
log.Infof("removed profile: %s", id)
return nil
}

View File

@@ -90,10 +90,10 @@ func writeProfileEmail(configPath string, email string) error {
return nil
}
// removeProfileEmail drops the stored account email. Called on profile removal,
// not on logout: a logged-out profile keeps its email so the next login passes
// it as the login_hint, matching the desktop and CLI semantics. Mirrors the
// desktop UI's RemoveProfileState call.
// removeProfileEmail drops the stored account email. Called on logout: while the
// email is on disk it goes out as a login_hint, which would steer the next login
// straight back into the account just logged out of. Mirrors the desktop UI's
// RemoveProfileState call.
func removeProfileEmail(configPath string) error {
accountPath, err := profileAccountPathFor(configPath)
if err != nil {

View File

@@ -127,10 +127,10 @@ func TestWriteThenReadProfileEmail(t *testing.T) {
t.Fatalf("remove: %v", err)
}
if got := readProfileEmail(configPath); got != "" {
t.Errorf("expected no email after removal, got %q", got)
t.Errorf("expected no email after logout, got %q", got)
}
// Removal may run on a never-logged-in profile, so a second remove must pass.
// Logout may run on a never-logged-in profile, so a second remove must pass.
if err := removeProfileEmail(configPath); err != nil {
t.Fatalf("second remove should be a no-op: %v", err)
}

View File

@@ -35,6 +35,8 @@ var (
// exported so a diagnostic reader reports the same locations that are written.
const (
// NRPTKeyPrefix starts the name of every NRPT rule key this client creates.
// Older versions used different layouts under the same prefix: a single
// unsuffixed key, then one key per domain, now one key per batch of domains.
NRPTKeyPrefix = "NetBird-Match"
// DNSPolicyConfigRoot holds the NRPT rules of the local policy store.
@@ -89,7 +91,6 @@ type registryConfigurator struct {
guid string
routingAll bool
gpo bool
nrptEntryCount int
origNameservers []netip.Addr
}
@@ -322,14 +323,9 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager
}
if len(matchDomains) != 0 {
count, err := r.addDNSMatchPolicy(matchDomains, config.ServerIP)
// Update count even on error to ensure cleanup covers partially created rules
r.nrptEntryCount = count
if err != nil {
if err := r.addDNSMatchPolicy(matchDomains, config.ServerIP); err != nil {
return fmt.Errorf("add dns match policy: %w", err)
}
} else {
r.nrptEntryCount = 0
}
r.updateState(stateManager)
@@ -345,9 +341,8 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager
func (r *registryConfigurator) updateState(stateManager *statemanager.Manager) {
if err := stateManager.UpdateState(&ShutdownState{
Guid: r.guid,
GPO: r.gpo,
NRPTEntryCount: r.nrptEntryCount,
Guid: r.guid,
GPO: r.gpo,
}); err != nil {
log.Errorf("failed to update shutdown state: %s", err)
}
@@ -362,7 +357,7 @@ func (r *registryConfigurator) addDNSSetupForAll(ip netip.Addr) error {
return nil
}
func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr) (int, error) {
func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr) error {
// if the gpo key is present, we need to put our DNS settings there, otherwise our config might be ignored
// see https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-gpnrpt/8cc31cb9-20cb-4140-9e85-3e08703b4745
@@ -379,19 +374,17 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr
gpoPath := fmt.Sprintf("%s-%d", gpoDnsPolicyConfigMatchPath, ruleIndex)
if err := r.configureDNSPolicy(localPath, batchDomains, ip); err != nil {
return ruleIndex, fmt.Errorf("configure DNS Local policy for rule %d: %w", ruleIndex, err)
return fmt.Errorf("configure DNS Local policy for rule %d: %w", ruleIndex, err)
}
// Increment immediately so the caller's cleanup path knows about this rule
ruleIndex++
if r.gpo {
if err := r.configureDNSPolicy(gpoPath, batchDomains, ip); err != nil {
return ruleIndex, fmt.Errorf("configure gpo DNS policy for rule %d: %w", ruleIndex-1, err)
return fmt.Errorf("configure gpo DNS policy for rule %d: %w", ruleIndex, err)
}
}
log.Debugf("added NRPT rule %d with %d domains", ruleIndex-1, len(batchDomains))
log.Debugf("added NRPT rule %d with %d domains", ruleIndex, len(batchDomains))
ruleIndex++
}
if r.gpo {
@@ -401,7 +394,7 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr
}
log.Infof("added %d NRPT rules for %d domains", ruleIndex, len(domains))
return ruleIndex, nil
return nil
}
func (r *registryConfigurator) configureDNSPolicy(policyPath string, domains []string, ip netip.Addr) error {
@@ -534,28 +527,28 @@ func (r *registryConfigurator) restoreHostDNS() error {
return nil
}
// removeDNSMatchPolicies deletes every NRPT rule this client may have created,
// from the local and the GPO policy store. The rules are found by enumerating
// the registry, the only authoritative record of what was written. Cleanup must
// not depend on a rule count: the in-memory one is scoped to a single
// registryConfigurator and the persisted one is deleted on every clean
// disconnect, and a rule left behind keeps resolving names over an interface
// that is gone, until reboot discards the volatile key.
func (r *registryConfigurator) removeDNSMatchPolicies() error {
var merr *multierror.Error
// Try to remove the base entries (for backward compatibility)
if err := removeRegistryKeyFromDNSPolicyConfig(dnsPolicyConfigMatchPath); err != nil {
merr = multierror.Append(merr, fmt.Errorf("remove local base entry: %w", err))
}
if err := removeRegistryKeyFromDNSPolicyConfig(gpoDnsPolicyConfigMatchPath); err != nil {
merr = multierror.Append(merr, fmt.Errorf("remove GPO base entry: %w", err))
}
for i := 0; i < r.nrptEntryCount; i++ {
localPath := fmt.Sprintf("%s-%d", dnsPolicyConfigMatchPath, i)
gpoPath := fmt.Sprintf("%s-%d", gpoDnsPolicyConfigMatchPath, i)
if err := removeRegistryKeyFromDNSPolicyConfig(localPath); err != nil {
merr = multierror.Append(merr, fmt.Errorf("remove local entry %d: %w", i, err))
for _, root := range []string{DNSPolicyConfigRoot, GPODNSPolicyConfigRoot} {
names, err := listNRPTRuleKeys(root)
if err != nil {
merr = multierror.Append(merr, fmt.Errorf("list rule keys under %s: %w", root, err))
continue
}
if err := removeRegistryKeyFromDNSPolicyConfig(gpoPath); err != nil {
merr = multierror.Append(merr, fmt.Errorf("remove GPO entry %d: %w", i, err))
for _, name := range names {
path := root + `\` + name
if err := removeRegistryKeyFromDNSPolicyConfig(path); err != nil {
merr = multierror.Append(merr, fmt.Errorf("remove entry %s: %w", path, err))
}
}
}
@@ -570,6 +563,39 @@ func (r *registryConfigurator) restoreUncleanShutdownDNS() error {
return r.restoreHostDNS()
}
// listNRPTRuleKeys returns the names of our NRPT rule keys under a policy store
// root. An absent root holds nothing to clean up, which is the normal state of
// the GPO store on a machine without DNS Client policy.
func listNRPTRuleKeys(root string) ([]string, error) {
k, err := registry.OpenKey(registry.LOCAL_MACHINE, root, registry.ENUMERATE_SUB_KEYS)
switch {
case errors.Is(err, registry.ErrNotExist), errors.Is(err, syscall.ERROR_PATH_NOT_FOUND):
// the GPO store is absent on a machine without DNS client policy
log.Debugf("HKEY_LOCAL_MACHINE\\%s does not exist", root)
return nil, nil
case err != nil:
// any other failure has to reach the caller: reporting no rules would
// report a successful cleanup while leaving the rules in place
return nil, fmt.Errorf("open HKEY_LOCAL_MACHINE\\%s: %w", root, err)
}
defer closer(k)
names, err := k.ReadSubKeyNames(-1)
if err != nil {
return nil, fmt.Errorf("read subkey names: %w", err)
}
var ruleKeys []string
for _, name := range names {
// registry key names are case insensitive
if strings.HasPrefix(strings.ToLower(name), strings.ToLower(NRPTKeyPrefix)) {
ruleKeys = append(ruleKeys, name)
}
}
return ruleKeys, nil
}
func removeRegistryKeyFromDNSPolicyConfig(regKeyPath string) error {
k, err := registry.OpenKey(registry.LOCAL_MACHINE, regKeyPath, registry.QUERY_VALUE)
if err != nil {

View File

@@ -25,7 +25,7 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) {
// Create a test interface registry key so updateSearchDomains doesn't fail
testGUID := "{12345678-1234-1234-1234-123456789ABC}"
interfacePath := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + testGUID
interfacePath := InterfaceConfigPath + `\` + testGUID
testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE)
require.NoError(t, err, "Should create test interface registry key")
testKey.Close()
@@ -56,7 +56,7 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) {
require.NoError(t, err)
// Verify 3 NRPT rules exist
assert.Equal(t, 3, cfg.nrptEntryCount, "Should create 3 NRPT rules for 125 domains")
assert.Equal(t, 3, countNRPTRuleKeys(t), "Should create 3 NRPT rules for 125 domains")
for i := 0; i < 3; i++ {
exists, err := registryKeyExists(fmt.Sprintf("%s-%d", dnsPolicyConfigMatchPath, i))
require.NoError(t, err)
@@ -81,7 +81,7 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) {
require.NoError(t, err)
// Verify first 2 NRPT rules exist
assert.Equal(t, 2, cfg.nrptEntryCount, "Should create 2 NRPT rules for 75 domains")
assert.Equal(t, 2, countNRPTRuleKeys(t), "Should create 2 NRPT rules for 75 domains")
for i := 0; i < 2; i++ {
exists, err := registryKeyExists(fmt.Sprintf("%s-%d", dnsPolicyConfigMatchPath, i))
require.NoError(t, err)
@@ -106,9 +106,65 @@ func registryKeyExists(path string) (bool, error) {
return true, nil
}
// TestNRPTCleanupWithoutRuleCount verifies that rules written by a previous run
// are removed by a configurator that has no record of how many there are: an
// unclean exit loses the in-memory count and a clean disconnect deletes the
// persisted one, so cleanup cannot depend on either.
func TestNRPTCleanupWithoutRuleCount(t *testing.T) {
if testing.Short() {
t.Skip("skipping registry integration test in short mode")
}
defer cleanupRegistryKeys(t)
cleanupRegistryKeys(t)
testIP := netip.MustParseAddr("100.64.0.1")
// 75 domains produce two indexed rules, as the current layout does
domains := make([]string, 75)
for i := range domains {
domains[i] = fmt.Sprintf(".domain%d.com", i+1)
}
previousRun := &registryConfigurator{}
require.NoError(t, previousRun.addDNSMatchPolicy(domains, testIP))
// the unsuffixed key an older version would have written
require.NoError(t, previousRun.configureDNSPolicy(dnsPolicyConfigMatchPath, []string{".legacy.example.com"}, testIP))
// a policy owned by someone else, which cleanup must not touch
foreignPath := DNSPolicyConfigRoot + `\DnsPolicyConfigTestForeign`
foreignKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, foreignPath, registry.SET_VALUE)
require.NoError(t, err, "Should create foreign policy key")
foreignKey.Close()
defer func() {
_ = registry.DeleteKey(registry.LOCAL_MACHINE, foreignPath)
}()
require.Equal(t, 3, countNRPTRuleKeys(t), "Should have two indexed rules and the legacy one")
// a configurator that never applied a DNS config, as one built after a
// restart or from a shutdown state without a count is
freshRun := &registryConfigurator{}
require.NoError(t, freshRun.removeDNSMatchPolicies())
assert.Equal(t, 0, countNRPTRuleKeys(t), "Should remove every rule left by the previous run")
exists, err := registryKeyExists(foreignPath)
require.NoError(t, err)
assert.True(t, exists, "Should not remove a policy that is not ours")
}
func countNRPTRuleKeys(t *testing.T) int {
t.Helper()
names, err := listNRPTRuleKeys(DNSPolicyConfigRoot)
require.NoError(t, err, "Should list NRPT rule keys")
return len(names)
}
func cleanupRegistryKeys(*testing.T) {
// Clean up more entries to account for batching tests with many domains
cfg := &registryConfigurator{nrptEntryCount: 20}
cfg := &registryConfigurator{}
_ = cfg.removeDNSMatchPolicies()
}
@@ -125,7 +181,7 @@ func TestNRPTDomainBatching(t *testing.T) {
// Create a test interface registry key so updateSearchDomains doesn't fail
testGUID := "{12345678-1234-1234-1234-123456789ABC}"
interfacePath := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + testGUID
interfacePath := InterfaceConfigPath + `\` + testGUID
testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE)
require.NoError(t, err, "Should create test interface registry key")
testKey.Close()
@@ -193,7 +249,7 @@ func TestNRPTDomainBatching(t *testing.T) {
require.NoError(t, err)
// Verify that exactly expectedRuleCount rules were created
assert.Equal(t, tc.expectedRuleCount, cfg.nrptEntryCount,
assert.Equal(t, tc.expectedRuleCount, countNRPTRuleKeys(t),
"Should create %d NRPT rules for %d domains", tc.expectedRuleCount, tc.domainCount)
// Verify all expected rules exist

View File

@@ -5,9 +5,8 @@ import (
)
type ShutdownState struct {
Guid string
GPO bool
NRPTEntryCount int
Guid string
GPO bool
}
func (s *ShutdownState) Name() string {
@@ -16,9 +15,8 @@ func (s *ShutdownState) Name() string {
func (s *ShutdownState) Cleanup() error {
manager := &registryConfigurator{
guid: s.Guid,
gpo: s.GPO,
nrptEntryCount: s.NRPTEntryCount,
guid: s.Guid,
gpo: s.GPO,
}
if err := manager.restoreUncleanShutdownDNS(); err != nil {

View File

@@ -15,8 +15,7 @@
"lint": "eslint \"src/**/*.{ts,tsx}\"",
"lint:fix": "eslint \"src/**/*.{ts,tsx}\" --fix",
"check": "pnpm lint && pnpm typecheck && pnpm format:check",
"check:fix": "pnpm lint:fix && pnpm format && pnpm typecheck",
"i18n:check": "node ../i18n/check-translations.mjs"
"check:fix": "pnpm lint:fix && pnpm format && pnpm typecheck"
},
"dependencies": {
"@radix-ui/react-dialog": "^1.1.15",

View File

@@ -1,104 +0,0 @@
#!/usr/bin/env node
// Validates that every shipped translation bundle carries exactly the same set
// of keys as the English source of truth. English (en) defines the keys; every
// other locale declared in _index.json must match it 1:1:
//
// - no missing keys — a missing key silently falls back to English at runtime
// (see i18n bundle fallback), so the gap never surfaces to users or CI
// without this check;
// - no orphaned keys — keys left behind after an English key is renamed or
// removed are dead weight and a sign the locale is drifting.
//
// Pure Node, no dependencies, so it runs without installing the frontend
// toolchain.
//
// Local: node client/ui/i18n/check-translations.mjs (or: pnpm i18n:check)
// CI: .github/workflows/ui-translations.yml
import { readdirSync, readFileSync } from "node:fs";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
const SOURCE = "en";
const localesDir = join(dirname(fileURLToPath(import.meta.url)), "locales");
const isCI = Boolean(process.env.GITHUB_ACTIONS);
function readJSON(path) {
return JSON.parse(readFileSync(path, "utf8"));
}
function keysOf(langCode) {
return Object.keys(readJSON(join(localesDir, langCode, "common.json")));
}
// Emit a GitHub Actions annotation so failures render inline on the PR diff.
function annotate(file, message) {
if (isCI) console.log(`::error file=${file}::${message}`);
}
const index = readJSON(join(localesDir, "_index.json"));
const declared = index.languages.map((l) => l.code);
if (!declared.includes(SOURCE)) {
console.error(`FATAL: source language "${SOURCE}" is not declared in _index.json`);
process.exit(1);
}
const sourceKeys = keysOf(SOURCE);
const sourceSet = new Set(sourceKeys);
console.log(`Source of truth: ${SOURCE}/common.json — ${sourceKeys.length} keys\n`);
let failed = false;
for (const code of declared) {
if (code === SOURCE) continue;
const file = `client/ui/i18n/locales/${code}/common.json`;
let keys;
try {
keys = keysOf(code);
} catch (e) {
failed = true;
const msg = `bundle is declared in _index.json but common.json is missing or invalid (${e.message})`;
console.error(`${code}: ${msg}`);
annotate("client/ui/i18n/locales/_index.json", `${code}: ${msg}`);
continue;
}
const set = new Set(keys);
const missing = sourceKeys.filter((k) => !set.has(k));
const extra = keys.filter((k) => !sourceSet.has(k));
if (missing.length === 0 && extra.length === 0) {
console.log(`${code}: ${keys.length} keys`);
continue;
}
failed = true;
console.error(`${code}: ${keys.length} keys (expected ${sourceKeys.length})`);
if (missing.length) {
console.error(` missing ${missing.length}: ${missing.join(", ")}`);
annotate(file, `Missing ${missing.length} key(s) present in ${SOURCE}: ${missing.join(", ")}`);
}
if (extra.length) {
console.error(` extra ${extra.length}: ${extra.join(", ")}`);
annotate(file, `Has ${extra.length} key(s) not present in ${SOURCE}: ${extra.join(", ")}`);
}
}
// Locale directories present on disk but not declared in _index.json are never
// loaded by the app — surface them so dead translation files don't rot silently.
const onDisk = readdirSync(localesDir, { withFileTypes: true })
.filter((e) => e.isDirectory())
.map((e) => e.name);
const undeclared = onDisk.filter((d) => !declared.includes(d));
if (undeclared.length) {
console.warn(`\n⚠ locale directories not declared in _index.json (not shipped): ${undeclared.join(", ")}`);
}
console.log();
if (failed) {
console.error("Translation check FAILED — every locale must match the English key set.");
process.exit(1);
}
console.log("Translation check passed — all locales match the English key set.");

View File

@@ -1312,9 +1312,6 @@
"daemon.outdated.description": {
"message": "このアプリを使用するには NetBird サービスを更新してください。"
},
"daemon.outdated.download": {
"message": "最新版をダウンロード"
},
"error.jwt_clock_skew": {
"message": "サインインに失敗しました: このデバイスの時計がサーバーと同期していません。システムの時計を同期してからもう一度お試しください。"
},