docs: add inline comments to Helm chart values.yaml

[mvanhorn]

Adds Bitnami-style inline YAML comments to every value in
packages/agent-mesh/charts/agentmesh/values.yaml. Each comment
documents the parameter's purpose, type (bool, int, string, map, list),
and valid values or ranges.

Sections covered: global config, serviceAccount, pod/container security
contexts, trustEngine, policyServer, auditCollector, apiGateway,
monitoring, autoscaling, podDisruptionBudget, and networkPolicy.

Also added short descriptions to the section headers explaining what
each component does (e.g. "Trust Engine - validates agent identity and
issues trust tokens").

Closes #339

This contribution was developed with AI assistance (Claude Code).

[github-actions]

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

[github-actions]

🤖 AI Agent: code-reviewer

Feedback on Pull Request: docs: add inline comments to Helm chart values.yaml

This PR introduces inline comments to the values.yaml file of the Helm chart for the agentmesh package. The comments provide detailed documentation for each configuration parameter, including its purpose, type, and valid values. While this improves the usability and maintainability of the Helm chart, there are some areas that require attention.

---

🔴 CRITICAL

1. SPIFFE Trust Domain Configuration

   • The spiffe.trustDomain parameter is documented as a string, but no validation or example is provided for acceptable formats (e.g., domain.local). Misconfiguration of the trust domain could lead to identity validation issues, potentially allowing unauthorized agents to bypass security mechanisms.
   • Action: Add validation logic or examples for acceptable trust domain formats in the documentation and ensure the application validates this parameter at runtime.

2. TLS Configuration

   • The tls.certSecretName parameter is critical for securing inter-component communication. If misconfigured or left empty, it could lead to unencrypted communication between components, exposing sensitive data.
   • Action: Add a note emphasizing that this parameter must be set and validated during deployment. Consider adding a default value or a validation check in the Helm chart to ensure it is not empty.

3. Pod Security Context

   • While the podSecurityContext.runAsNonRoot and securityContext.allowPrivilegeEscalation parameters are set to secure defaults (true and false, respectively), there is no explicit mention of whether these settings are enforced across all components.
   • Action: Ensure that these security settings are applied consistently across all pods and containers. Add runtime checks or validation in the application code to enforce these settings.

---

🟡 WARNING

1. Backward Compatibility

   • Adding inline comments does not directly affect the functionality of the Helm chart, but the extensive documentation could lead to confusion if users rely on outdated or incorrect interpretations of the parameters.
   • Action: Ensure that any future changes to the Helm chart parameters are clearly documented and communicated to users. Consider versioning the values.yaml file or providing a changelog for updates.

2. Default Values

   • Some parameters, such as global.imageTag and policyServer.image.tag, rely on default values. If these defaults are changed in future releases, it could lead to unexpected behavior for users upgrading the chart.
   • Action: Document the implications of relying on default values and provide clear guidance on how users can override them.

---

💡 SUGGESTIONS

1. Validation for Numeric Ranges

   • Parameters such as trustEngine.replicas, policyServer.replicas, and apiGateway.rateLimitPerMinute specify valid ranges or minimum values in the comments but do not enforce these constraints.
   • Suggestion: Add validation logic in the Helm chart or application code to enforce these numeric constraints.

2. Examples for Complex Parameters

   • Parameters like serviceAccount.annotations and trustEngine.nodeSelector are documented as maps, but no examples are provided.
   • Suggestion: Add examples for these parameters to help users understand how to configure them correctly.

3. Consistency in Documentation Style

   • The inline comments are detailed and helpful, but some sections (e.g., autoscaling.targetCPUUtilizationPercentage) could benefit from additional context or examples.
   • Suggestion: Review all comments for consistency and completeness, ensuring that every parameter has sufficient documentation.

4. Security Context Documentation

   • The securityContext.capabilities.drop parameter is documented as a list but does not specify valid values (e.g., ALL, NET_ADMIN).
   • Suggestion: Add a list of valid capability names to the documentation.

5. Monitoring Configuration

   • The monitoring.prometheus.scrapeInterval parameter is documented as a string but does not specify valid formats (e.g., 15s, 1m).
   • Suggestion: Provide examples of valid formats for time durations.

---

Summary

This PR significantly improves the documentation of the Helm chart, making it easier for users to understand and configure the chart. However, there are critical security concerns related to SPIFFE trust domain configuration, TLS settings, and pod/container security contexts that need to be addressed. Additionally, some parameters could benefit from more detailed examples and validation logic to ensure correctness and prevent misconfiguration.

Action Items:

1. Address critical security concerns related to SPIFFE, TLS, and security contexts.
2. Add validation logic for numeric ranges and acceptable formats where applicable.
3. Provide examples for complex parameters like maps and lists.
4. Ensure consistency and completeness in the documentation style.

This PR is a good step forward in improving the usability of the Helm chart, but addressing the above issues will further enhance its reliability and security.

[github-actions]

🤖 AI Agent: security-scanner

After reviewing the provided diff for the values.yaml file in the microsoft/agent-governance-toolkit repository, I have identified the following potential security concerns:

---

1. Trust Chain Weaknesses: SPIFFE/SVID Validation Gaps

• Issue: The spiffe.trustDomain and spiffe.socketPath parameters are configurable via the Helm chart. If these values are misconfigured or set to malicious endpoints, it could compromise the trust chain and allow unauthorized agents to be trusted.
• Impact: If an attacker can manipulate these values (e.g., via a supply chain attack or misconfiguration), they could potentially impersonate trusted agents, bypassing the trust engine and compromising the entire system.
• Rating: 🔴 CRITICAL
• Recommendation:
  • Validate the spiffe.trustDomain and spiffe.socketPath values at runtime to ensure they conform to expected formats and point to trusted sources.
  • Consider hardcoding or validating the spiffe.trustDomain against a whitelist of known trusted domains.
  • Log a warning or error if spiffe.enabled is set to false, as this disables SPIFFE-based workload identity, which is critical for secure communication.

---

2. Policy Engine Circumvention

• Issue: The policyServer.policyMountPath is configurable via the Helm chart. If this path is misconfigured or points to an unintended location, it could result in the policy server loading malicious or unintended policies.
• Impact: An attacker could potentially modify the policies used by the policy server, allowing them to bypass governance controls or enforce malicious policies.
• Rating: 🔴 CRITICAL
• Recommendation:
  • Validate the policyServer.policyMountPath to ensure it points to a secure and expected directory.
  • Implement runtime checks to verify the integrity of the policy files (e.g., using checksums or signatures).
  • Consider restricting the ability to modify this value unless absolutely necessary.

---

3. Credential Exposure

• Issue: The global.imagePullSecrets parameter is used to specify Docker registry secrets. If these secrets are logged or exposed in error messages, they could be leaked to unauthorized users.
• Impact: Leaked secrets could allow attackers to pull private container images, leading to potential compromise of sensitive application components.
• Rating: 🟠 HIGH
• Recommendation:
  • Ensure that global.imagePullSecrets values are never logged or exposed in error messages.
  • Use Kubernetes secrets to securely store and reference these credentials, and avoid hardcoding them in the values.yaml file.

---

4. Sandbox Escape

• Issue: The securityContext.allowPrivilegeEscalation is set to false, which is good. However, there is no validation to ensure that this value cannot be overridden by a user setting it to true in the Helm chart.
• Impact: If a user or attacker modifies this value to true, it could allow containers to escalate privileges, potentially leading to a sandbox escape.
• Rating: 🟠 HIGH
• Recommendation:
  • Enforce allowPrivilegeEscalation: false at the Kubernetes admission controller level or through a Pod Security Policy (PSP) or Open Policy Agent (OPA) policy.
  • Consider removing the ability to override this value in the Helm chart if privilege escalation is never required.

---

5. Deserialization Attacks

• Issue: The policyServer.policyMountPath parameter suggests that YAML policy files are mounted and likely deserialized by the policy server. If these files are not validated or sanitized before deserialization, they could be a vector for deserialization attacks.
• Impact: An attacker could craft malicious YAML files to execute arbitrary code or cause denial-of-service (DoS) attacks on the policy server.
• Rating: 🔴 CRITICAL
• Recommendation:
  • Use a safe YAML parser that prevents execution of arbitrary code during deserialization.
  • Validate the structure and content of the YAML files against a strict schema before processing them.

---

6. Network Policy Misconfiguration

• Issue: The networkPolicy.enabled parameter allows enabling or disabling Kubernetes NetworkPolicies. If this is set to false, it could leave the system vulnerable to lateral movement attacks within the cluster.
• Impact: Disabling NetworkPolicies could allow an attacker to move laterally between pods and compromise other components of the system.
• Rating: 🟠 HIGH
• Recommendation:
  • Consider enforcing networkPolicy.enabled: true by default and documenting the risks of disabling it.
  • Provide a default restrictive NetworkPolicy that only allows necessary traffic between components.

---

7. Lack of Resource Constraints

• Issue: While resource requests and limits are defined for some components (e.g., trustEngine, policyServer, auditCollector, apiGateway), there is no validation to ensure these values are set appropriately. Misconfigured or overly permissive resource limits could lead to resource exhaustion or denial-of-service (DoS) attacks.
• Impact: An attacker could exploit misconfigured resource limits to cause service disruptions or degrade performance.
• Rating: 🟡 MEDIUM
• Recommendation:
  • Provide recommended default values for resource requests and limits in the documentation.
  • Validate that resource limits are set to reasonable values during deployment.

---

8. Supply Chain Risks

• Issue: The image.repository and imageTag parameters are configurable for all components. If these values are set to untrusted or malicious images, it could compromise the entire system.
• Impact: An attacker could inject malicious container images into the deployment, leading to unauthorized access or data exfiltration.
• Rating: 🔴 CRITICAL
• Recommendation:
  • Use image signing and verification (e.g., Cosign) to ensure only trusted images are deployed.
  • Consider restricting the image.repository and imageTag values to a whitelist of trusted registries and tags.

---

Summary of Findings:

Finding	Severity
SPIFFE/SVID Validation Gaps	🔴 CRITICAL
Policy Engine Circumvention	🔴 CRITICAL
Credential Exposure	🟠 HIGH
Sandbox Escape	🟠 HIGH
Deserialization Attacks	🔴 CRITICAL
Network Policy Misconfiguration	🟠 HIGH
Lack of Resource Constraints	🟡 MEDIUM
Supply Chain Risks	🔴 CRITICAL

General Recommendations:

1. Implement runtime validations for critical configuration parameters (e.g., spiffe.trustDomain, policyServer.policyMountPath, image.repository).
2. Use Kubernetes security features like Pod Security Policies (PSPs), Open Policy Agent (OPA), and NetworkPolicies to enforce security best practices.
3. Document the security implications of configurable parameters in the Helm chart and provide secure defaults.
4. Regularly audit and validate the integrity of container images and dependencies to mitigate supply chain risks.

Let me know if you need further assistance!

[imran-siddique]

Great start @mvanhorn — the comments you added are accurate, well-formatted, and follow Helm conventions nicely. Two things to complete before merge:

1. Incomplete coverage (42 values missing): You documented \ rustEngine\ and \policyServer\ image/resources/probes thoroughly, but the same fields under \�uditCollector\ and \�piGateway\ are undocumented. Please add the matching comments to all 4 components for consistency:

• \image.repository, \image.tag, \image.pullPolicy\ (6 missing)
• 
  esources.requests/limits\ cpu/memory (12 missing)
• Liveness/readiness probe parameters (24 missing)

2. Quick fix: The pattern is mechanical — copy the same comment blocks from \ rustEngine\ to the other 3 components. Should be ~15 minutes of work.

The documentation quality is solid — just needs to cover all components equally. Happy to merge once the gaps are filled.

[mvanhorn]

Added the 42 missing comments in f4f0b41. All 4 components now have matching documentation for image, resources, probes, and scheduling fields.

[github-actions]

🤖 AI Agent: security-scanner — Security Review of PR: `docs: add inline comments to Helm chart values.yaml`

Security Review of PR: docs: add inline comments to Helm chart values.yaml

This PR primarily adds inline comments to the values.yaml file of the Helm chart for the agentmesh package. The changes are documentation-focused and do not introduce new functionality or modify existing logic. However, since this repository is a security-focused project, it's critical to ensure that the documentation does not inadvertently introduce security risks or mislead users into misconfiguring the system.

---

Findings

1. 🔵 LOW — Potential for Misconfiguration Due to Ambiguous Comments

• Issue: Some comments in the values.yaml file could lead to misconfigurations if users misinterpret them. For example:
  • The comment for global.spiffe.trustDomain states: SPIFFE trust domain, e.g. "agentmesh.local". While this is technically correct, it does not emphasize that the trust domain must match the SPIFFE configuration of the SPIRE server, which is critical for secure workload identity.
  • The comment for auditCollector.retentionDays does not clarify whether the cleanup is automatic or requires additional configuration.
  • The comment for apiGateway.rateLimitPerMinute does not specify whether this is a global rate limit or per-client rate limit, which could lead to incorrect assumptions about the level of protection provided.
• Attack Vector: Misconfigurations in security-critical components like SPIFFE, audit logging, or API Gateway rate limiting could lead to trust chain weaknesses, incomplete audit trails, or denial-of-service vulnerabilities.
• Recommendation:
  • Update the comment for global.spiffe.trustDomain to clarify that it must match the SPIFFE server's trust domain configuration.
  • Specify whether auditCollector.retentionDays requires additional configuration for automatic cleanup.
  • Clarify the scope of apiGateway.rateLimitPerMinute (e.g., "Maximum API requests per minute per client before throttling").
  • Perform a broader review of all comments to ensure they are unambiguous and provide sufficient guidance to prevent misconfigurations.

---

2. 🔵 LOW — No Validation for Critical Parameters

• Issue: While this PR does not introduce new functionality, it highlights the lack of validation for critical parameters in the values.yaml file. For example:
  • global.spiffe.trustDomain should be validated to ensure it is a valid SPIFFE trust domain.
  • trustEngine.replicas, policyServer.replicas, and other replica counts should be validated to ensure they are integers greater than or equal to 1.
  • apiGateway.rateLimitPerMinute should be validated to ensure it is a positive integer.
• Attack Vector: Lack of validation for these parameters could lead to misconfigurations that compromise security or availability. For example:
  • An invalid trustDomain could break SPIFFE-based workload identity, leading to trust chain weaknesses.
  • Setting replicas to 0 could disable critical components like the Trust Engine or Policy Server, effectively bypassing security policies.
  • Setting rateLimitPerMinute to 0 could disable rate limiting, making the system vulnerable to denial-of-service attacks.
• Recommendation:
  • Implement schema validation for the values.yaml file using a tool like helm lint or a JSON schema. This will ensure that invalid configurations are caught during deployment.

---

Overall Assessment

This PR is primarily documentation-focused and does not introduce new functionality or modify existing logic. However, the comments added in this PR could lead to misconfigurations if they are not sufficiently clear or if the underlying parameters are not validated. While these issues are not directly caused by the changes in this PR, they are highlighted by the new comments and should be addressed to ensure the security and reliability of the system.

Rating of Findings

1. Potential for Misconfiguration Due to Ambiguous Comments: 🔵 LOW
2. No Validation for Critical Parameters: 🔵 LOW

Recommendations

• Update the comments in values.yaml to provide clearer guidance and prevent potential misconfigurations.
• Implement schema validation for the values.yaml file to enforce constraints on critical parameters.

While these findings are not critical, addressing them will improve the overall security posture of the project and reduce the risk of misconfigurations by downstream users.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces inline comments to the values.yaml file of the agentmesh Helm chart. The comments provide detailed documentation for each configuration parameter, including its purpose, type, and valid values. Additionally, section headers now include short descriptions of the components they configure. This improves the maintainability and usability of the Helm chart, especially for new users and contributors.

While the changes are primarily documentation-related, they touch on critical components of the agent-mesh package, including security contexts, SPIFFE/SVID configurations, and resource limits. Below is a detailed review of the changes.

---

🔴 CRITICAL

1. SPIFFE/SVID Configuration

   • The SPIFFE trust domain (global.spiffe.trustDomain) is set to agentmesh.local by default. This could lead to potential security issues if users deploy the chart without customizing this value. Attackers could exploit this default configuration to impersonate workloads within the cluster.
   • Actionable Recommendation: Add a warning in the comments for global.spiffe.trustDomain to emphasize the importance of customizing this value for production deployments. For example:

     # -- SPIFFE trust domain, e.g. "agentmesh.local" (string)
     #    WARNING: Change this value to a unique domain for your deployment to avoid security risks.
     trustDomain: agentmesh.local

2. Pod Security Context

   • The podSecurityContext.runAsUser and podSecurityContext.fsGroup are hardcoded to 1000. While this is generally safe, it assumes that the container images are configured to run as non-root. If a user deploys a custom image that requires a different UID/GID, this could lead to runtime failures or privilege escalation risks.
   • Actionable Recommendation: Add comments suggesting that users verify the UID/GID requirements of their container images before deployment. For example:

     # -- UID for the container process (int)
     #    Ensure this matches the UID expected by your container image.
     runAsUser: 1000

---

💡 SUGGESTIONS

1. Validation for Resource Limits

   • While resource requests and limits are documented, there is no guidance on recommended values for production environments. This could lead to suboptimal resource allocation or instability.
   • Suggestion: Add comments recommending default values for production environments based on typical workloads. For example:

     # -- Minimum CPU allocation (string, Kubernetes resource quantity)
     #    Recommended value for production: "500m"
     cpu: 100m

2. TLS Configuration

   • The global.tls.enabled and global.tls.certSecretName parameters are documented, but there is no mention of the importance of securing inter-component communication in production.
   • Suggestion: Add a note emphasizing that TLS should always be enabled in production environments. For example:

     # -- Enable TLS for inter-component communication (bool)
     #    It is strongly recommended to enable TLS in production environments to secure communication.
     enabled: true

3. Rate Limiting

   • The apiGateway.rateLimitPerMinute parameter is documented, but there is no guidance on how to choose an appropriate value.
   • Suggestion: Add comments suggesting typical values based on expected traffic patterns. For example:

     # -- Maximum API requests per minute per client before throttling (int)
     #    Adjust based on expected traffic. Typical values: 1000 for moderate traffic, 5000 for high traffic.
     rateLimitPerMinute: 1000

4. Monitoring

   • The monitoring.prometheus.scrapeInterval is set to 15s by default, which may not be suitable for all environments.
   • Suggestion: Add a comment suggesting that users adjust this value based on their monitoring needs and Prometheus server capacity. For example:

     # -- How often Prometheus scrapes metrics (string, duration, e.g. "15s")
     #    Adjust based on your monitoring requirements and Prometheus server capacity.
     scrapeInterval: 15s

---

🟡 WARNING

1. Backward Compatibility
   • Adding comments to the values.yaml file does not introduce breaking changes to the Helm chart itself. However, the default values for certain parameters (e.g., global.spiffe.trustDomain, podSecurityContext.runAsUser) could lead to unexpected behavior if users rely on these defaults without customization.
   • Actionable Recommendation: Document these defaults in the release notes and encourage users to review their configurations before upgrading.

---

Final Assessment

The changes in this pull request significantly improve the usability and maintainability of the values.yaml file. However, there are critical security concerns related to default values for SPIFFE/SVID and pod security contexts that must be addressed. Additionally, providing more guidance on production-grade configurations would further enhance the value of this documentation.

Recommended Actions:

1. Address the 🔴 CRITICAL issues before merging.
2. Consider implementing the 💡 SUGGESTIONS to improve the documentation further.
3. Include a note in the release documentation about potential backward compatibility concerns.

Once the critical issues are resolved, this pull request can be approved for merging.

[mvanhorn]

Addressed in f4f0b41: added matching inline comments to auditCollector and apiGateway for all image, resources, and probe fields. All 4 components now have consistent documentation coverage.

[imran-siddique]

All 4 components now have consistent documentation — image fields, resources, probes, scheduling, and component-specific values all covered. Clean, follows Helm conventions. Great follow-up @mvanhorn. Merging.