feat(security): transition from static policies.yaml to dynamic relat…

[aojea]

…ional SQL policy API

• Replaced local file-based policies.yaml with centralized SQL tables (roles and bindings) in Postgres/SQLite on the Control Plane.
• Exposed a REST API (/policies) to dynamically retrieve and update policies on the Hub.
• Refactored the policy compiler to build Biscuit Datalog rules from relational DB snapshots locally on the node.
• Added support for periodic policy sync interval configuration flag (--policy-sync-interval) on the node.
• Gracefully handled OIDC callback local server shutdown to prevent TCP reset issues during enrollment.
• Removed obsolete --policy-file flags, volumes, and ConfigMaps from all GKE template and kind development configurations.
• Integrated automated policy seeding via background port-forward curl commands post-rollout in GitHub actions and local kind runner scripts.
• Updated policy and control plane website guides to document the new JSON API and database model.
• Fixed all golangci-lint issues across the codebase.
• Verified that all unit, integration, and E2E test suites pass successfully.

[gemini-code-assist]

Code Review

This pull request refactors the mesh policy configuration, moving from a static file-based bootstrap (policies.yaml) to a dynamic REST API and database-backed model where nodes periodically sync policies from the Hub and compile them locally. The review feedback highlights several critical issues: a break statement in VerifyBiscuitAndGetKey that breaks key rotation, multiple logging statements with missing arguments, and several bugs in the node's policy compiler, including unhandled sam:system:authenticated members, missing target_restricted() facts for specific allowed targets, and ignored CustomDatalog configurations.

[gemini-code-assist]

This break statement completely breaks key rotation and multi-key verification.

If there are multiple trusted keys (e.g., during key rotation), and the token is signed by a key other than the first one in the list, the verification with the first key will fail (due to an invalid signature). Because of this break, the loop will abort immediately without trying the other trusted keys, causing valid tokens to be incorrectly rejected.

Please remove this break so that the loop continues to try other trusted keys if verification fails.

Suggested change

	lastErr = err
	break // Signature is valid, but authorization failed. No need to try other keys.
	lastErr = err

[gemini-code-assist]

The log message contains format specifiers %s and %v but the corresponding arguments nodeRecord.PeerID and err were removed. This will result in missing arguments in the log output.

Suggested change

	logger.Errorf("Failed to mint refreshed token for node %s: %v")
	logger.Errorf("Failed to mint refreshed token for node %s: %v", nodeRecord.PeerID, err)

[gemini-code-assist]

The log message contains a %v format specifier but the err argument was removed. This will result in a missing argument in the log output.

Suggested change

	logger.Errorf("Failed to mint bootstrap biscuit: %v")
	logger.Errorf("Failed to mint bootstrap biscuit: %v", err)

[gemini-code-assist]

The log message contains a %v format specifier but the err argument was removed. This will result in a missing argument in the log output.

Suggested change

	logger.Errorf("Failed to mint bootstrap biscuit: %v")
	logger.Errorf("Failed to mint bootstrap biscuit: %v", err)

[gemini-code-assist]

The policy compiler does not handle the special sam:system:authenticated member (defined as api.SystemAuthenticated). Previously, any authenticated identity was granted the corresponding role. Now, "sam:system:authenticated" is split into a predicate named "sam" with value "system:authenticated". Since there is no "sam" predicate/fact in Biscuit, this binding will never match and is completely broken.

Please add explicit handling for api.SystemAuthenticated to grant the role to any authenticated user or node (for example, by generating rules with a wildcard/variable body for user or node facts).

[gemini-code-assist]

When a role has specific allowed_targets (other than *), the policy compiler should generate a rule concluding api.FactTargetRestricted (i.e., target_restricted()) for that role. Without this, the node's authorizer won't know that targets are restricted for the role, which will cause authorization checks to fail or behave incorrectly.

[gemini-code-assist]

The policy compiler completely ignores CustomDatalog (defined in api.PolicyRole). Custom Datalog facts are saved in the database and sent to the node, but they are never compiled into Biscuit rules on the node.

Please add support for parsing and compiling CustomDatalog facts into Biscuit rules/facts in BuildPolicyRules.

[aojea]

/gemini review

[gemini-code-assist]

Code Review

This pull request transitions the Sovereign Agent Mesh (SAM) from static file-based policy bootstrapping to a dynamic, database-backed policy API managed via the Control Plane. It introduces structured protobuf messages for roles and bindings, updates the storage layer to use relational tables, adds a Helm chart for deployment, and implements periodic policy synchronization on the nodes. The review feedback highlights several critical issues: the role resolution logic is missing support for email and node prefixes, the policy update endpoint lacks validation, and updates are not immediately broadcasted to the mesh. Additionally, the node's initial policy sync needs a retry loop with exponential backoff, the OIDC server shutdown should use a timeout context, and the Helm chart's database port and SSL mode should be configurable.

[gemini-code-assist]

The resolveRoles function is missing support for api.FactEmail ("email") and api.FactNode ("node") prefixes. Additionally, the check for oidcEmail == value is incorrectly nested under api.FactUser, which means any binding defined with "email:xxx" will fail to resolve because its prefix is "email", not "user".

Furthermore, node-specific bindings (e.g., "node:12D3KooW...") cannot be resolved because the "node" prefix is not handled, and the peer ID is not passed to resolveRoles. We should update the signature to accept peerID string and handle both prefixes correctly. Note that the callers of resolveRoles at lines 345 and 562 must also be updated to pass pID.String().

func resolveRoles(claims jwt.MapClaims, bindings []*api.PolicyBinding, peerID string) []string {
	oidcRoles := toStringSlice(claims["roles"])
	oidcGroups := toStringSlice(claims["groups"])
	oidcSub, _ := claims["sub"].(string)
	oidcEmail, _ := claims["email"].(string)

	resolvedRoles := make(map[string]bool)
	for _, b := range bindings {
		if b == nil {
			continue
		}
		for _, m := range b.Members {
			if m == api.SystemAuthenticated {
				resolvedRoles[b.Role] = true
				continue
			}
			parts := strings.SplitN(m, ":", 2)
			if len(parts) != 2 {
				continue
			}
			prefix, value := parts[0], parts[1]
			switch prefix {
			case api.FactGroup:
				for _, g := range oidcGroups {
					if g == value {
						resolvedRoles[b.Role] = true
					}
				}
			case api.FactUser:
				if oidcSub == value {
					resolvedRoles[b.Role] = true
				}
			case api.FactEmail:
				if oidcEmail == value {
					resolvedRoles[b.Role] = true
				}
			case api.FactNode:
				if peerID == value {
					resolvedRoles[b.Role] = true
				}
			}
		}
	}
	for _, r := range oidcRoles {
		resolvedRoles[r] = true
	}

	var res []string
	for r := range resolvedRoles {
		res = append(res, r)
	}
	return res
}

[gemini-code-assist]

Update the call to resolveRoles to pass pID.String() to support node-specific bindings and correct prefix resolution.

Suggested change

	resolvedRoles := resolveRoles(claims, bindings)
	resolvedRoles := resolveRoles(claims, bindings, pID.String())

[gemini-code-assist]

Update the call to resolveRoles to pass pID.String() to support node-specific bindings and correct prefix resolution.

Suggested change

	resolvedRoles := resolveRoles(claims, bindings)
	resolvedRoles := resolveRoles(claims, bindings, pID.String())

[gemini-code-assist]

The MeshEvent_POLICY_UPDATE event is defined in the proto and handled by the node to trigger an immediate policy sync. However, when the policy is successfully updated via POST/PUT /policies, the control plane does not broadcast/publish this event to the mesh. This forces nodes to wait for their periodic sync interval (up to 1 hour) to receive policy updates, defeating the dynamic nature of the policy API. Please ensure that a MeshEvent_POLICY_UPDATE event is published/broadcasted to the mesh upon successful policy updates.

[aojea]

/gemini review

[gemini-code-assist]

Code Review

This pull request transitions the Sovereign Agent Mesh (SAM) policy configuration from a static YAML file to a dynamic, database-backed policy system managed via a REST API, introducing a Helm chart for deployment and enabling periodic policy synchronization on nodes. The review feedback highlights several critical issues: the resolveRoles function silently ignores email and node-based bindings; the policy update endpoint lacks validation for incoming configurations; and session-level PostgreSQL advisory locks are incorrectly executed on a connection pool rather than a dedicated connection. Additionally, the database operations lack robust error handling, such as missing rows.Err() checks, missing nil pointer checks, and potential failures in SQLite cascade deletes if foreign keys are disabled. Other improvements include addressing a race condition in Kubernetes secret recreation and adding a timeout to the server shutdown context.

[gemini-code-assist]

Update the call to resolveRoles to pass the peer ID pID.String() to support node-based bindings.

Suggested change

	resolvedRoles := resolveRoles(claims, bindings)
	resolvedRoles := resolveRoles(claims, bindings, pID.String())

[gemini-code-assist]

Update the call to resolveRoles to pass the peer ID pID.String() to support node-based bindings.

Suggested change

	resolvedRoles := resolveRoles(claims, bindings)
	resolvedRoles := resolveRoles(claims, bindings, pID.String())

[gemini-code-assist]

There is no validation performed on the incoming PolicyConfigUpdateRequest (roles and bindings) before saving it to the database. This allows administrators to accidentally save invalid role names, malformed service/target patterns, or bindings referencing non-existent roles, which can cause runtime errors or security bypasses on the nodes. Consider implementing a validation helper similar to the old ValidatePolicyConfig to validate the structure and formats of the new relational policies.

[gemini-code-assist]

Enforce defensive programming by adding nil checks for r and b before accessing their fields (e.g., r.Name, b.Role, b.Members). If the input slices contain any nil elements, this will cause a runtime panic.

[gemini-code-assist]

In GetMeshPolicy, the database rows iteration loops (rolesRows, permsRows, bindingsRows) do not check rows.Err() after completion. It is a standard Go best practice to check rows.Err() to ensure that the loop didn't terminate prematurely due to a database connection or data error.

[gemini-code-assist]

If a custom datalog string fails to parse as both a rule and a fact, the error is silently ignored. This can lead to silent security policy failures (fail-open or fail-closed) if there is a typo in the policy. Consider logging a warning when parsing fails.

		for _, dl := range role.CustomDatalog {
			trimmed := strings.TrimRight(strings.TrimSpace(dl), ";")
			if trimmed == "" {
				continue
			}
			r, err := parser.FromStringRule(trimmed)
			if err == nil {
				rules = append(rules, r)
			} else {
				f, err2 := parser.FromStringFact(trimmed)
				if err2 == nil {
					rules = append(rules, biscuit.Rule{
						Head: f.Predicate,
						Body: []biscuit.Predicate{},
					})
				} else {
					logger.Warnf("Failed to parse custom datalog %q as rule (%v) or fact (%v)", trimmed, err, err2)
				}
			}
		}

[gemini-code-assist]

Deleting a Kubernetes secret and immediately recreating it via POST is highly prone to race conditions because Kubernetes deletion is asynchronous. The POST request might fail with a 409 Conflict if the deletion is still in progress. A much more robust approach is to use a single PATCH request with Server-Side Apply (application/apply-patch+yaml or application/merge-patch+json) to create-or-update the secret in a single atomic operation.

[aojea]

/gemini review and update the comments that are stale

[gemini-code-assist]

Code Review

This pull request transitions the Sovereign Agent Mesh (SAM) policy management from a static file-based bootstrapping mechanism (policies.yaml) to a dynamic, database-backed model managed via the Control Plane REST API. It introduces structured protobuf messages for roles and bindings, database migrations for relational storage, dynamic policy synchronization on the node side, and a Helm chart for deployment. The review feedback highlights several critical issues, including a lack of policy validation on the REST endpoint, missing rows.Err() checks during database queries, unhandled storage.ErrNotFound errors during node registration/refresh, potential orphaned rows in SQLite due to disabled foreign key cascades, and the use of context.Background() in background sync goroutines which could cause resource leaks on shutdown.

[gemini-code-assist]

The previous policy validation logic (ValidatePolicyConfig) has been completely removed in this transition. Currently, the /policies endpoint accepts any payload without verifying if the roles are unique, if the bindings reference valid roles, or if the allowed services/targets conform to expected formats. Saving an invalid policy can lead to silent failures or security bypasses when nodes attempt to parse and apply the rules. Please re-introduce a validation step before calling SaveMeshPolicy.

[gemini-code-assist]

In Go's database/sql package, it is critical to check rows.Err() after iterating with rows.Next(). If a database error or connection drop occurs mid-stream, the loop will terminate early and silently, returning a partial/incomplete policy. This is a significant security risk as some roles or bindings might be omitted. Please ensure Err() is checked for rolesRows, permsRows, and bindingsRows.

func (s *SQLStore) GetMeshPolicy(ctx context.Context) ([]*api.PolicyRole, []*api.PolicyBinding, error) {
	rolesRows, err := s.db.QueryContext(ctx, s.rebind("SELECT name FROM roles"))
	if err != nil {
		return nil, nil, err
	}
	defer func() { _ = rolesRows.Close() }()

	rolesMap := make(map[string]*api.PolicyRole)
	var roles []*api.PolicyRole

	for rolesRows.Next() {
		var name string
		if err := rolesRows.Scan(&name); err != nil {
			return nil, nil, err
		}
		role := &api.PolicyRole{Name: name}
		rolesMap[name] = role
		roles = append(roles, role)
	}
	if err := rolesRows.Err(); err != nil {
		return nil, nil, err
	}

	permsRows, err := s.db.QueryContext(ctx, s.rebind("SELECT role_name, resource_type, resource_value FROM role_permissions"))
	if err != nil {
		return nil, nil, err
	}
	defer func() { _ = permsRows.Close() }()

	for permsRows.Next() {
		var roleName, resType, resValue string
		if err := permsRows.Scan(&roleName, &resType, &resValue); err != nil {
			return nil, nil, err
		}
		if r, ok := rolesMap[roleName]; ok {
			switch resType {
			case "target":
				r.AllowedTargets = append(r.AllowedTargets, resValue)
			case "service":
				r.AllowedServices = append(r.AllowedServices, resValue)
			case "custom_datalog":
				r.CustomDatalog = append(r.CustomDatalog, resValue)
			}
		}
	}
	if err := permsRows.Err(); err != nil {
		return nil, nil, err
	}

	bindingsRows, err := s.db.QueryContext(ctx, s.rebind("SELECT role_name, member FROM role_bindings"))
	if err != nil {
		return nil, nil, err
	}
	defer func() { _ = bindingsRows.Close() }()

	bindingsMap := make(map[string]*api.PolicyBinding)
	for bindingsRows.Next() {
		var roleName, member string
		if err := bindingsRows.Scan(&roleName, &member); err != nil {
			return nil, nil, err
		}
		b, ok := bindingsMap[roleName]
		if !ok {
			b = &api.PolicyBinding{Role: roleName}
			bindingsMap[roleName] = b
		}
		b.Members = append(b.Members, member)
	}
	if err := bindingsRows.Err(); err != nil {
		return nil, nil, err
	}

	var bindings []*api.PolicyBinding
	for _, b := range bindingsMap {
		bindings = append(bindings, b)
	}

	return roles, bindings, nil
}

[gemini-code-assist]

In SQLite, foreign key constraints are disabled by default unless explicitly enabled on every connection. If they are disabled, DELETE FROM roles will not cascade to role_permissions and role_bindings, leaving orphaned rows in the database. To ensure database-agnostic correctness and prevent orphaned rows, please explicitly delete from all three tables within the transaction.

func (s *SQLStore) SaveMeshPolicy(ctx context.Context, roles []*api.PolicyRole, bindings []*api.PolicyBinding) error {
	tx, err := s.db.BeginTx(ctx, nil)
	if err != nil {
		return err
	}
	defer func() { _ = tx.Rollback() }()

	// Explicitly delete from all tables to prevent orphaned rows, especially in SQLite where foreign keys might be disabled by default.
	if _, err := tx.ExecContext(ctx, "DELETE FROM role_permissions"); err != nil {
		return err
	}
	if _, err := tx.ExecContext(ctx, "DELETE FROM role_bindings"); err != nil {
		return err
	}
	if _, err := tx.ExecContext(ctx, "DELETE FROM roles"); err != nil {
		return err
	}

[aojea]

/gemini review again and close comments that are no longer relevant

[gemini-code-assist]

Code Review

This pull request transitions the Sovereign Agent Mesh (SAM) policy configuration from a static file-based bootstrapping mechanism to a dynamic, database-backed REST API system, introducing a new Helm chart for deployment and enabling periodic policy synchronization on nodes. Key feedback includes addressing security concerns around unconditionally enabling --insecure-skip-tls-verify in the control plane, restoring missing input validation for custom Datalog rules and binding members, fixing a busybox compatibility issue with nc -z in the init container, ensuring router identity persistence by avoiding emptyDir for router data, and optimizing control plane handlers by removing redundant database queries.

[gemini-code-assist]

The new validatePolicyConfig function completely omits validation of CustomDatalog rules/facts. This is a regression from the old policy validation logic, allowing administrators to save malformed Datalog rules/facts that will be silently ignored by the nodes. Add validation using the Biscuit parser.

		for _, target := range r.AllowedTargets {
			if err := api.ValidateTargetFormat(target); err != nil {
				return fmt.Errorf("invalid allowed_target %q in role %s: %w", target, r.Name, err)
			}
		}
		for _, dl := range r.CustomDatalog {
			trimmed := strings.TrimRight(strings.TrimSpace(dl), ";")
			if trimmed == "" {
				continue
			}
			if _, err := parser.FromStringRule(trimmed); err != nil {
				if _, err := parser.FromStringFact(trimmed); err != nil {
					return fmt.Errorf("invalid custom datalog %q in role %s: %w", dl, r.Name, err)
				}
			}
		}
	}

[gemini-code-assist]

The new validatePolicyConfig function completely omits validation of binding members. This is a regression from the old policy validation logic, allowing administrators to save bindings with invalid formats or unsupported identity prefixes. Add validation to ensure members are non-empty and have valid prefixes.

		if strings.TrimSpace(b.Role) == "" {
			return fmt.Errorf("binding role cannot be empty")
		}
		if !roleNames[b.Role] {
			return fmt.Errorf("binding references undefined role: %s", b.Role)
		}
		if len(b.Members) == 0 {
			return fmt.Errorf("binding for role %q must specify at least one member", b.Role)
		}
		for _, member := range b.Members {
			if member == api.SystemAuthenticated {
				continue
			}
			parts := strings.SplitN(member, ":", 2)
			if len(parts) != 2 {
				return fmt.Errorf("member %q in binding for role %q is invalid, must be in format 'type:value' or %q", member, b.Role, api.SystemAuthenticated)
			}
			prefix := parts[0]
			if _, validPrefix := api.ValidMemberPrefixes[prefix]; !validPrefix {
				return fmt.Errorf("member prefix %q in member %q is invalid, expected one of the standard identity facts", prefix, member)
			}
		}
	}

[gemini-code-assist]

Using emptyDir for router-data means the router's private key (/data/router.key) will be deleted whenever the pod restarts. This causes the router to generate a new identity (PeerID) on every restart, which disrupts peer connections and breaks any persistent routing configurations. Consider using a volumeClaimTemplate to persist the router's identity, or make it configurable.

[aojea]

/gemini review

[gemini-code-assist]

Code Review

This pull request transitions the Sovereign Agent Mesh (SAM) from a static, file-based policy configuration to a dynamic, database-backed policy system managed via the Control Plane REST API. It introduces new database schemas, a /policies endpoint, and periodic policy syncing on nodes. Feedback on these changes highlights several critical issues: a bug in BuildPolicyRules where wildcard targets are treated as exact matches, a security vulnerability allowing revoked nodes with unexpired tokens to still fetch policies, a potential thundering herd problem during policy updates, and duplicate custom access roles being minted into Biscuit tokens during registration and refresh.

[gemini-code-assist]

The BuildPolicyRules function contains a critical bug where wildcard targets (e.g., group:backend.*) are always treated as exact matches (api.FactGrantedTargetExact) instead of using the appropriate wildcard predicates. Additionally, there is a significant amount of duplicate parsing logic for services and targets that can be simplified by leveraging the existing helper functions api.BuildServiceDatalogFact and api.BuildTargetDatalogFact from api/datalog.go.

		for _, svc := range role.AllowedServices {
			fact := api.BuildServiceDatalogFact(svc)
			rules = append(rules, biscuit.Rule{
				Head: fact.Predicate,
				Body: []biscuit.Predicate{
					{Name: api.FactRole, IDs: []biscuit.Term{biscuit.String(roleName)}},
				},
			})
		}

		hasUnrestricted := false
		hasSpecificTargets := false
		for _, t := range role.AllowedTargets {
			if t == "*" {
				hasUnrestricted = true
			} else {
				hasSpecificTargets = true
			}
		}

		if hasUnrestricted {
			rules = append(rules, biscuit.Rule{
				Head: biscuit.Predicate{
					Name: api.FactTargetUnrestricted,
					IDs:  []biscuit.Term{},
				},
				Body: []biscuit.Predicate{
					{Name: api.FactRole, IDs: []biscuit.Term{biscuit.String(roleName)}},
				},
			})
		}

		if hasSpecificTargets {
			rules = append(rules, biscuit.Rule{
				Head: biscuit.Predicate{
					Name: api.FactTargetRestricted,
					IDs:  []biscuit.Term{},
				},
				Body: []biscuit.Predicate{
					{Name: api.FactRole, IDs: []biscuit.Term{biscuit.String(roleName)}},
				},
			})
		}

		for _, t := range role.AllowedTargets {
			if t == "*" {
				continue
			}
			targetStr := t
			if !strings.Contains(t, ":") {
				targetStr = "node:" + t
			}
			fact := api.BuildTargetDatalogFact(targetStr)
			rules = append(rules, biscuit.Rule{
				Head: fact.Predicate,
				Body: []biscuit.Predicate{
					{Name: api.FactRole, IDs: []biscuit.Term{biscuit.String(roleName)}},
				},
			})
		}

[gemini-code-assist]

In HandlePolicies (GET), the endpoint authenticates nodes solely by verifying the signature and expiration of their Biscuit token via identity.VerifyAndExtractPeerID. However, it does not query the database to check if the node is still active or has been revoked/deleted. A revoked node with an unexpired token would still be able to retrieve the entire mesh policy. Consider verifying the node's active enrollment status in the store before returning the policies.

[gemini-code-assist]

When a POLICY_UPDATE event is received, all nodes concurrently trigger a policy sync by calling n.syncMeshPolicy(ctx) without any delay. In a large mesh, this can cause a thundering herd problem and temporarily overload the control plane database. Introducing a small random jitter/delay (e.g., 0 to 5 seconds) before fetching the policy would distribute the load and protect the control plane.

[gemini-code-assist]

There is a regression/bug where the requested role is duplicated in finalRoles if it is a custom access role (i.e., does not start with "sam:role:"). This happens because the check r != req.RequestedRole was omitted when building customAccessRoles. Adding this check prevents duplicate roles from being processed and minted into the Biscuit token.

Suggested change

	resolvedRoles := resolveRoles(pID.String(), claims, bindings)
	var hasCapabilityRoles bool
	var customAccessRoles []string
	resolvedMap := make(map[string]bool)
	for _, r := range resolvedRoles {
	resolvedMap[r] = true
	if strings.HasPrefix(r, "sam:role:") {
	hasCapabilityRoles = true
	} else {
	customAccessRoles = append(customAccessRoles, r)
	}
	}
	resolvedRoles := resolveRoles(pID.String(), claims, bindings)
	var hasCapabilityRoles bool
	var customAccessRoles []string
	resolvedMap := make(map[string]bool)
	for _, r := range resolvedRoles {
	resolvedMap[r] = true
	if strings.HasPrefix(r, "sam:role:") {
	hasCapabilityRoles = true
	} else if r != req.RequestedRole {
	customAccessRoles = append(customAccessRoles, r)
	}
	}

[gemini-code-assist]

Similar to HandleRegister, the requested role is duplicated in finalRoles during token refresh if it is a custom access role, because the check r != nodeRecord.Role was omitted when building customAccessRoles. Adding this check prevents duplicate roles from being processed and minted into the Biscuit token.

Suggested change

	resolvedRoles := resolveRoles(pID.String(), claims, bindings)
	var hasCapabilityRoles bool
	var customAccessRoles []string
	resolvedMap := make(map[string]bool)
	for _, r := range resolvedRoles {
	resolvedMap[r] = true
	if strings.HasPrefix(r, "sam:role:") {
	hasCapabilityRoles = true
	} else {
	customAccessRoles = append(customAccessRoles, r)
	}
	}
	resolvedRoles := resolveRoles(pID.String(), claims, bindings)
	var hasCapabilityRoles bool
	var customAccessRoles []string
	resolvedMap := make(map[string]bool)
	for _, r := range resolvedRoles {
	resolvedMap[r] = true
	if strings.HasPrefix(r, "sam:role:") {
	hasCapabilityRoles = true
	} else if r != nodeRecord.Role {
	customAccessRoles = append(customAccessRoles, r)
	}
	}

[aojea]

/gemini review

[gemini-code-assist]

Code Review

This pull request transitions the Sovereign Agent Mesh from a static, file-based policy configuration to a dynamic, API-driven mesh policy system. It replaces the --policy-file flag with a REST API (/policies) for managing roles and bindings, introduces a MeshAdapter interface to broadcast policy updates and bans via libp2p GossipSub, and enables nodes to periodically and dynamically sync these policies. Additionally, it packages the deployment into a Helm chart with an automated bootstrapping job. The review feedback highlights several critical safety improvements, including potential nil pointer dereferences in policy resolution, database storage, and node authentication, a potential race condition in the interactive OIDC login server, and a missing nil check for the storage parameter in the P2P mesh adapter.

[gemini-code-assist]

The resolveRoles function accesses claims["roles"] and other fields without checking if claims is nil. If claims is nil (which can happen if a node is refreshed or registered without OIDC claims), this will cause a nil pointer dereference panic. Please add a nil check for claims at the beginning of the function.

func resolveRoles(peerID string, claims jwt.MapClaims, bindings []*api.PolicyBinding) []string {
	if claims == nil {
		claims = make(jwt.MapClaims)
	}
	oidcRoles := toStringSlice(claims["roles"])
	oidcGroups := toStringSlice(claims["groups"])
	oidcSub, _ := claims["sub"].(string)
	oidcEmail, _ := claims["email"].(string)

[gemini-code-assist]

The SaveMeshPolicy function iterates over roles and bindings and accesses their fields (e.g., r.Name, b.Role) without checking if the elements themselves are nil. Since validatePolicyConfig in server.go simply skips nil elements during validation but does not filter them out of the slices, SaveMeshPolicy can receive slices containing nil elements, leading to a nil pointer dereference panic. Please add nil checks inside the loops.

	for _, r := range roles {
		if r == nil {
			continue
		}
		if _, err := tx.ExecContext(ctx, s.rebind("INSERT INTO roles (name, description, created_at) VALUES (?, '', ?)"), r.Name, time.Now().UnixMilli()); err != nil {
			return err
		}
		for _, target := range r.AllowedTargets {
			if _, err := tx.ExecContext(ctx, s.rebind("INSERT INTO role_permissions (role_name, resource_type, resource_value) VALUES (?, 'target', ?)"), r.Name, target); err != nil {
				return err
			}
		}
		for _, svc := range r.AllowedServices {
			if _, err := tx.ExecContext(ctx, s.rebind("INSERT INTO role_permissions (role_name, resource_type, resource_value) VALUES (?, 'service', ?)"), r.Name, svc); err != nil {
				return err
			}
		}
		for _, dl := range r.CustomDatalog {
			if _, err := tx.ExecContext(ctx, s.rebind("INSERT INTO role_permissions (role_name, resource_type, resource_value) VALUES (?, 'custom_datalog', ?)"), r.Name, dl); err != nil {
				return err
			}
		}
	}

	for _, b := range bindings {
		if b == nil {
			continue
		}
		for _, member := range b.Members {
			if _, err := tx.ExecContext(ctx, s.rebind("INSERT INTO role_bindings (role_name, member) VALUES (?, ?)"), b.Role, member); err != nil {
				return err
			}
		}
	}

[gemini-code-assist]

There is a potential race condition between the background goroutine starting the HTTP server and the main goroutine calling shutdownSrv(). If ctx.Done() or an error is received before the background goroutine assigns srv, srv will be nil and shutdownSrv() will do nothing. Later, the server will start and run indefinitely, leaking the port and resources. Consider initializing srv in the main goroutine before launching the background goroutine.

[gemini-code-assist]

If nodeErr == nil but nodeRecord is nil (e.g., due to database inconsistency or test mocks), accessing nodeRecord.Banned will cause a panic. It is safer to check nodeRecord != nil before accessing its fields.

Suggested change

	nodeRecord, nodeErr := s.store.GetNode(r.Context(), peerID.String())
	if nodeErr == nil && !nodeRecord.Banned {
	isNode = true
	}
	nodeRecord, nodeErr := s.store.GetNode(r.Context(), peerID.String())
	if nodeErr == nil && nodeRecord != nil && !nodeRecord.Banned {
	isNode = true
	}

[gemini-code-assist]

The store parameter is required to retrieve the signing key for event publishing. If store is nil, PublishEvent will silently publish unsigned events, which might be rejected by other nodes or lead to security bypasses. Please add a nil check for store in NewP2PMeshAdapter.

Suggested change

	func NewP2PMeshAdapter(h host.Host, ps *pubsub.PubSub, store storage.Store) (*P2PMeshAdapter, error) {
	if h == nil || ps == nil {
	return nil, fmt.Errorf("host and pubsub cannot be nil")
	}
	func NewP2PMeshAdapter(h host.Host, ps *pubsub.PubSub, store storage.Store) (*P2PMeshAdapter, error) {
	if h == nil || ps == nil || store == nil {
	return nil, fmt.Errorf("host, pubsub, and store cannot be nil")
	}