fix(security): comprehensive security audit remediation (22 findings, 37 files)

[imran-siddique]

Comprehensive Security Audit Remediation

Deep security audit across 4 scanners (Actions, Code, Supply Chain, Infrastructure) identified 29 findings. This PR remediates 22 of them across 37 files.

Batch 1: CI Injection + Action Pinning (8 files)

• Expression injection (CWE-94): Moved github.event.* expressions from run: to env: blocks in 6 workflows
• Secret leak (CWE-200): Redacted matched secrets in secret-scanning.yml CI logs
• Action pinning (CWE-829): SHA-pinned dtolnay/rust-toolchain (only unpinned action)
• Permissions (CWE-250): Added permissions block to markdown-link-check.yml

Batch 2: Supply Chain (8 files)

• Dependency confusion (CWE-427): Replaced agent-primitives==0.1.0 with local file refs
• Dockerfile pinning (CWE-829): Pinned root Dockerfile base image to SHA digest
• Lockfiles (CWE-829): Generated missing package-lock.json for 4 npm packages
• ESRP pipeline (CWE-829): Removed unsafe npm ci fallback

Batch 3: Docker/Infra (17 files)

• Grafana passwords (CWE-798): Replaced hardcoded passwords in 7 docker-compose files
• Wildcard CORS (CWE-942): Replaced allow_origins=["*"] with env-driven origins in 6 services
• .dockerignore (CWE-532): Added .env, *.key, *.pem exclusions
• SECURITY.md (CWE-693): Added contact email, supported versions, disclosure policy
• CODEOWNERS (CWE-862): Added rules for scripts/, Dockerfile, docker-compose*

Batch 4: Code Quality (4 files)

• XSS (CWE-79): Replaced innerHTML with safe DOM APIs in VS Code webview panels
• Rust panics (CWE-252): Replaced .unwrap() with .expect() on production lock calls
• Example safety: Added INTENTIONALLY INSECURE warnings to test fixtures

[github-actions]

🤖 AI Agent: breaking-change-detector — Summary

🔍 API Compatibility Report

Summary

After analyzing the provided diff, no breaking changes were detected in the public API of the microsoft/agent-governance-toolkit repository. The changes primarily focus on security improvements, dependency management, and infrastructure updates.

Findings

Severity	Package	Change	Impact
✅	N/A	No breaking changes to public API	None

Migration Guide

No migration steps are necessary as there are no breaking changes. Downstream users can safely update to the latest version without concerns about compatibility issues.

Notes

• The changes include security enhancements such as secret redaction, action pinning, and improved CORS handling.
• Infrastructure updates like Dockerfile base image pinning and .dockerignore adjustments are non-breaking and improve security.
• Dependency updates and lockfile additions ensure better supply chain security but do not affect the public API.

✅ This update is safe for downstream users.

[github-actions]

🤖 AI Agent: docs-sync-checker — Issues Found

📝 Documentation Sync Report

Issues Found

• ⚠️ packages/agent-hypervisor/README.md — The changes to the CORS configuration in server.py should be reflected in the README, specifically mentioning the new CORS_ALLOWED_ORIGINS environment variable.
• ⚠️ packages/agent-mesh/README.md — The changes to the Grafana password configuration in docker-compose.yml should be documented in the README, including the new requirement for the GF_ADMIN_PASSWORD environment variable.
• ⚠️ CHANGELOG.md — No entry for the comprehensive security audit remediation changes, which include updates to CI workflows, Dockerfiles, and codebase improvements for security.
• ⚠️ docs/ — The project-level documentation should be reviewed and updated to reflect the new security practices, such as the addition of SECURITY.md and the changes to CORS and Grafana configurations.

Suggestions

• 💡 Update the packages/agent-hypervisor/README.md to include details about the CORS_ALLOWED_ORIGINS environment variable and its usage.
• 💡 Update the packages/agent-mesh/README.md to document the new GF_ADMIN_PASSWORD environment variable requirement for Grafana in the docker-compose.yml example.
• 💡 Add a detailed entry in CHANGELOG.md summarizing the security audit findings and the corresponding remediations made in this PR.
• 💡 Review and update the documentation in the docs/ directory to ensure it aligns with the new security practices and configurations introduced in this PR.

Additional Notes

• No new public APIs were introduced in this PR, so no new docstrings or type hints are required.
• Example code in examples/ was updated to reflect the new CORS configuration, which is good.
• Ensure that all documentation updates are consistent across the repository.

Please address the identified issues and suggestions to ensure documentation is fully synchronized with the changes introduced in this PR.

[github-actions]

🤖 AI Agent: test-generator — `packages/agent-hypervisor/src/hypervisor/api/server.py`

🧪 Test Coverage Analysis

packages/agent-hypervisor/src/hypervisor/api/server.py

• ✅ Existing coverage:
  • Basic functionality of create_app is likely covered, including the addition of middleware.
  • General API server initialization and endpoint registration.
• ❌ Missing coverage:
  • The new allow_origins logic that uses os.environ.get("CORS_ALLOWED_ORIGINS") to set allowed origins dynamically.
  • Edge cases for CORS_ALLOWED_ORIGINS, such as empty strings, malformed URLs, or unexpected input.
• 💡 Suggested test cases:
  1. test_cors_env_variable_set — Test that the allow_origins middleware correctly parses a valid CORS_ALLOWED_ORIGINS environment variable (e.g., multiple origins separated by commas).
  2. test_cors_env_variable_empty — Test behavior when CORS_ALLOWED_ORIGINS is set to an empty string.
  3. test_cors_env_variable_malformed — Test behavior when CORS_ALLOWED_ORIGINS contains malformed URLs.
  4. test_cors_env_variable_not_set — Test default behavior when CORS_ALLOWED_ORIGINS is not set in the environment.

---

packages/agent-os/src/agent_os/server/app.py

• ✅ Existing coverage:
  • Likely covers basic server initialization and endpoint registration.
• ❌ Missing coverage:
  • No changes were made to this file in the provided diff, so no new code paths are introduced. However, existing tests should be reviewed to ensure they cover edge cases like malformed inputs, oversized payloads, and injection attempts.
• 💡 Suggested test cases:
  1. test_malformed_input — Test API endpoints with malformed JSON or invalid data types.
  2. test_oversized_payload — Test API endpoints with payloads exceeding the maximum allowed size.
  3. test_sql_injection_attempt — Test API endpoints with SQL injection-like payloads to ensure proper sanitization.

---

packages/agent-sre/src/agent_sre/api/server.py

• ✅ Existing coverage:
  • Likely covers basic server initialization and endpoint registration.
• ❌ Missing coverage:
  • No changes were made to this file in the provided diff, so no new code paths are introduced. However, existing tests should be reviewed to ensure they cover edge cases like timeout handling, partial failures, and cascading failures.
• 💡 Suggested test cases:
  1. test_timeout_handling — Simulate a timeout in an API call and verify the server handles it gracefully.
  2. test_partial_failure — Simulate a partial failure in a multi-step operation and verify the server's behavior.
  3. test_cascading_failure — Simulate a failure that propagates through dependent services and verify the server's resilience.

---

General Recommendations

1. Environment Variable Handling: Ensure that all environment variables introduced in the changes are tested for presence, absence, and edge cases (e.g., empty or malformed values).
2. Security Testing: Given the security focus of this PR, ensure that tests cover scenarios like:
   • Attempted CORS bypass using unexpected CORS_ALLOWED_ORIGINS values.
   • Injection attacks in API inputs.
   • Oversized payloads to test server limits.
3. Concurrency Testing: If the server handles concurrent requests, add tests for potential race conditions or deadlocks, especially in shared state scenarios.
4. Chaos Testing: Simulate failures in dependent services or middleware to ensure the server handles them gracefully without crashing.

Let me know if you'd like me to generate specific test code for any of the suggested cases!

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request addresses 22 security findings across multiple areas, including CI/CD workflows, supply chain dependencies, Docker/Infrastructure configurations, and code quality improvements. The changes are generally well-structured and address critical vulnerabilities. However, there are a few areas that require further attention or clarification.

---

🔴 CRITICAL

1. Hardcoded Default CORS Origins in server.py (CWE-942):

   • The default value for CORS_ALLOWED_ORIGINS in server.py is set to http://localhost:3000,http://localhost:8080. While this is an improvement over the previous wildcard (["*"]), it still poses a security risk if the environment variable is not set correctly. This could allow unintended domains to access sensitive resources.
   • Recommendation: Validate the CORS_ALLOWED_ORIGINS environment variable to ensure it contains only trusted domains. If the variable is not set, fail the application startup instead of defaulting to permissive origins.

2. Potential Sandbox Escape in docker-compose.yml (CWE-798):

   • The GF_SECURITY_ADMIN_PASSWORD is now sourced from an environment variable, but the .env file is included in .dockerignore. If the .env file is not securely managed, it could lead to credential leakage.
   • Recommendation: Ensure .env files are excluded from version control and are securely managed using a secrets management tool (e.g., HashiCorp Vault, AWS Secrets Manager).

3. Expression Injection in GitHub Actions (CWE-94):

   • While moving github.event.* expressions to env: blocks mitigates direct injection risks, the use of unvalidated inputs like github.event.release.tag_name or inputs.tag in shell commands still poses a risk.
   • Recommendation: Validate and sanitize these inputs before using them in shell commands. For example, use shlex.quote() in Python or equivalent methods in shell scripting.

---

🟡 WARNING

1. Action Pinning (CWE-829):

   • The dtolnay/rust-toolchain action is pinned to a specific SHA (29eef336d9b2848a0b548edc03f92a220660cdb8). While this is a good practice for security, it may cause compatibility issues if the action is updated to address bugs or vulnerabilities.
   • Recommendation: Periodically review and update pinned actions to ensure compatibility and security.

2. Dependency Confusion Mitigation:

   • Replacing agent-primitives==0.1.0 with a local file reference is a good step to prevent dependency confusion (CWE-427). However, this change may break compatibility for external users relying on the PyPI package.
   • Recommendation: Clearly document this change in the release notes and consider publishing the updated package to PyPI.

---

💡 SUGGESTIONS

1. Improved Logging for Secret Scanning:

   • The secret-scanning.yml workflow redacts secrets using sed 's/:.*/:***REDACTED***/'. While this is effective, consider logging the number of matches found for better visibility.
   • Example: echo "::warning::Found $FOUND potential secrets matching pattern: $pattern"

2. Backward Compatibility for Example Fixtures:

   • Adding INTENTIONALLY INSECURE warnings to test fixtures is a good practice. However, ensure that these warnings do not interfere with automated testing pipelines that rely on these fixtures.
   • Recommendation: Add a flag or environment variable to suppress warnings during automated tests.

3. Type Safety and Validation:

   • For Python code, ensure that environment variables like CORS_ALLOWED_ORIGINS are validated using Pydantic models or equivalent type-checking mechanisms.
   • Example:

     from pydantic import BaseModel, HttpUrl, ValidationError
     
     class Config(BaseModel):
         cors_allowed_origins: list[HttpUrl]
     
     try:
         config = Config(cors_allowed_origins=os.environ.get("CORS_ALLOWED_ORIGINS", "").split(","))
     except ValidationError as e:
         raise ValueError(f"Invalid CORS_ALLOWED_ORIGINS: {e}")

4. Documentation Updates:

   • The changes to SECURITY.md are excellent. Consider adding a section on how to securely manage .env files and secrets.

5. Thread Safety:

   • While this PR does not directly address thread safety, ensure that changes to server.py (e.g., CORS configuration) are thread-safe, especially if the application uses asynchronous frameworks like FastAPI.

---

Final Notes

This PR addresses critical security issues effectively, but there are areas that require further validation and documentation. The flagged critical issues should be addressed before merging to ensure robust security and compliance.

[github-actions]

🤖 AI Agent: security-scanner — Security Analysis of the Pull Request

Security Analysis of the Pull Request

1. Prompt Injection Defense Bypass

No issues detected. The changes in this PR do not directly involve prompt handling or user input parsing that could lead to prompt injection vulnerabilities.

2. Policy Engine Circumvention

No issues detected. The changes do not appear to weaken or bypass any policy enforcement mechanisms.

3. Trust Chain Weaknesses

No issues detected. The PR includes improvements to dependency management and Docker image pinning, which strengthen the trust chain.

4. Credential Exposure

Finding 1: 🔴 CRITICAL

• Issue: Hardcoded credentials were replaced with environment variables in the docker-compose.yml file. However, the .env file is now excluded in .dockerignore, which could lead to accidental inclusion of sensitive credentials in the Docker build context if the .env file is not properly managed.
• Attack Vector: If the .env file is accidentally committed to the repository or included in the Docker image, it could expose sensitive credentials to unauthorized users.
• Fix: Ensure that .env files are not only excluded from .dockerignore but also from version control (e.g., add .env to .gitignore). Additionally, provide clear documentation on securely managing .env files.

5. Sandbox Escape

No issues detected. The changes do not involve modifications to sandboxing mechanisms or process isolation.

6. Deserialization Attacks

Finding 2: 🟠 HIGH

• Issue: The package-lock.json includes the yaml package (version 2.8.3), which has a history of deserialization vulnerabilities (e.g., CVE-2021-42392). While the specific version included here does not have known vulnerabilities, the use of YAML parsing libraries should be carefully reviewed to ensure safe usage.
• Attack Vector: If untrusted YAML input is parsed without proper validation, it could lead to remote code execution or other attacks.
• Fix: Review the usage of the yaml library in the codebase to ensure that untrusted input is not being deserialized. If possible, use a safer alternative or configure the library to disable unsafe features like arbitrary code execution.

7. Race Conditions

No issues detected. The changes do not introduce any new concurrency or timing-related vulnerabilities.

8. Supply Chain

Finding 3: 🟠 HIGH

• Issue: The package-lock.json file includes a large number of dependencies, some of which may have known vulnerabilities or could be susceptible to dependency confusion attacks.
• Attack Vector: Malicious actors could exploit dependency confusion by publishing malicious packages with the same name as internal dependencies. Additionally, vulnerabilities in third-party dependencies could be exploited by attackers.
• Fix: Use tools like npm audit or snyk to scan for known vulnerabilities in the dependencies. Consider using a private package registry to mitigate dependency confusion risks.

Finding 4: 🟡 MEDIUM

• Issue: The Dockerfile now pins the base image to a SHA digest, which is a good practice. However, there is no automated mechanism to periodically check for updates to the base image.
• Attack Vector: Over time, the pinned base image may become outdated and contain unpatched vulnerabilities, leaving the application exposed to attacks.
• Fix: Implement a process to regularly check for updates to the base image and update the pinned SHA digest accordingly. This can be automated using tools like docker scan or Trivy.

---

Summary of Findings

Finding	Severity	Description	Suggested Fix
1	🔴 CRITICAL	.env files excluded in .dockerignore but not explicitly mentioned in .gitignore.	Add .env to .gitignore and provide documentation for secure .env handling.
2	🟠 HIGH	Potential deserialization risks with the yaml library.	Review usage of yaml library and disable unsafe features or use safer alternatives.
3	🟠 HIGH	Dependency confusion and vulnerabilities in package-lock.json.	Use npm audit or snyk to scan dependencies and consider a private registry.
4	🟡 MEDIUM	No automated update mechanism for pinned Docker base image.	Implement a process to regularly update the pinned SHA digest for the base image.

General Observations

• The PR addresses several critical issues, such as hardcoded credentials, wildcard CORS, and action pinning, which are commendable.
• The changes to CI workflows (e.g., moving GitHub event expressions to env blocks) reduce the risk of expression injection.
• Adding .dockerignore exclusions for sensitive files is a good step, but it must be complemented with .gitignore updates and documentation to prevent accidental exposure.

Final Recommendation

• Address the critical finding related to .env file handling before merging.
• Review the usage of the yaml library and scan dependencies for vulnerabilities.
• Implement a process for updating pinned Docker base images.
• After addressing these issues, the PR can be approved.