Chore/bisect

[danielbodnar]

This pull request introduces several foundational improvements to the BitBuilder Cloud CLI (bbctl) project, focusing on documentation, configuration, code quality, and initial planning. The changes establish a clear project structure, add configuration and linting for both Rust and TypeScript development, and provide comprehensive guides and plans for future development.

Project Planning and Documentation:

• Added a detailed implementation plan for bbctl, outlining architecture, phases, and directory structure in PLAN.md.
• Introduced an API documentation guide using Zod and OpenAPI in docs/api-readme.md, including instructions for schema validation, type safety, and client generation.
• Added a comprehensive configuration guide in docs/configuration-guide.md, covering all aspects of bbctl configuration, provider and credential management, and best practices.
• Expanded the main README.md with instructions for testing against a VyOS lab environment.

Development Environment and Tooling:

• Added .eslintrc.json for TypeScript linting with recommended rules and project-specific overrides.
• Introduced bunfig.toml to configure Bun for dependency management, testing, and building TypeScript code.
• Updated Cargo.toml to include the uuid crate for networking and security features, preparing for future API and RPC additions.

Developer Experience and Guidelines:

• Enhanced CLAUDE.md with more detailed build, test, and style guidelines, including project structure, code formatting, and naming conventions. [1] [2] [3] [4]

These changes lay the groundwork for robust development practices, clear documentation, and a maintainable codebase as bbctl evolves.

[Copilot]

Pull Request Overview

This PR adds a comprehensive VyOS test lab infrastructure with Docker-based deployment scripts, network configuration, and integration documentation for the bbctl CLI. The changes primarily introduce new test infrastructure files and documentation to support multi-tenant networking scenarios.

Key Changes

• Added Docker-based VyOS test lab setup with automated provisioning scripts
• Introduced network configuration files for L3VPN/EVPN, WireGuard, and OSPF
• Added comprehensive documentation for VyOS networking and test lab setup
• Created TypeScript schema definitions and OpenAPI integration files

Reviewed Changes

Copilot reviewed 57 out of 61 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/vyos-lab/setup-lab.sh	Main orchestration script for VyOS lab deployment with router configuration
tests/vyos-lab/scripts/*.sh	Individual setup scripts for base infrastructure, containers, L3VPN, and WireGuard
tests/vyos-lab/README.md	Documentation for the VyOS test lab environment
docs/vyos-*.md	Comprehensive networking architecture and test lab documentation
src/main.rs	Added TestVyOS command for connectivity testing
schema.ts	TypeScript/Zod schemas for API validation
package.json	Added npm scripts and dependencies for TypeScript tooling

Comments suppressed due to low confidence (6)

src/main.rs:1

• The TestVyOS command destructuring pattern is missing several fields that are defined in the command struct (password, key_path, api_key). This will cause a compilation error. Include all fields in the destructuring pattern or use .. to ignore unused fields.
  src/main.rs:1
• [nitpick] The default host IP '5.254.54.3' appears to be a public IP address that may not be appropriate as a default value. Consider using 'localhost' or a more generic local IP address as the default.
  tests/vyos-lab/setup-lab.sh:1
• The script references ${SCRIPT_DIR}/scripts/ but SCRIPT_DIR is defined relative to the current script location. The scripts directory should exist at ${SCRIPT_DIR}/scripts/, but the actual scripts are in a sibling 'scripts' directory based on the file structure. This path should be verified or corrected.
  tests/vyos-lab/scripts/setup-base.sh:1
• Hardcoded interface name 'eth0' may not exist on all systems. Consider detecting the default route interface dynamically using ip route show default | awk '{print $5}' or making it configurable.
  tests/vyos-lab/scripts/configure-wireguard.sh:1
• The regex pattern for extracting the WireGuard private key expects exactly 43 characters, but WireGuard keys are base64-encoded 32-byte values resulting in 44 characters (including padding). The pattern should be [A-Za-z0-9+/=]\{44\} to correctly match WireGuard keys.
  tests/vyos-lab/scripts/setup-vyos-container.sh:1
• The encrypted password hash is exposed in plain text in the configuration script. While this is a test environment, it's better practice to generate unique passwords per deployment or document that this is the hash for the default 'vyos' password.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The CIDR regex validation only checks the format but doesn't validate that octets are in the valid range (0-255) or that the prefix length is valid (0-32 for IPv4). Consider using a more robust validation or IP validation library to prevent invalid CIDR blocks like '999.999.999.999/99'.

[Copilot]

Pull request overview

Copilot reviewed 57 out of 61 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (8)

vyos-lab/setup-network.sh:1

• The hardcoded absolute path /home/bodnar/vyos-lab makes this script non-portable. Use $(dirname "$0") or a relative path to make the script work for other users and environments.
  vyos-lab/mkosi.default:1
• Hardcoded path contains a specific username 'bodnar'. This should use a relative path or environment variable to ensure portability across different user environments.
  tests/vyos-lab/setup-lab.sh:1
• The default host IP 5.254.54.3 appears to be a public IP address. For a test lab, consider using private IP ranges (e.g., 192.168.x.x or 10.x.x.x) to avoid potential conflicts with real public IPs.
  tests/vyos-lab/scripts/setup-base.sh:1
• The script assumes network interface eth0 exists. Modern systems may use different interface naming schemes (e.g., ens33, enp0s3). Consider detecting the default interface dynamically or documenting this assumption.
  tests/vyos-lab/scripts/setup-vyos-container.sh:1
• Using a hardcoded password hash in test scripts is a security concern if these scripts are ever used in non-test environments. Add a prominent comment warning that this is for testing only, or better yet, generate unique passwords per deployment.
  src/main.rs:1
• The error message refers to implementation details ('tokio runtime') that may confuse users. This appears to be a placeholder that returns an error, but the actual implementation exists later in main(). Consider removing this unreachable code path or updating the comment to clarify the control flow.
  src/network.rs:1
• The code accesses config.port with unwrap_or(), but the WireguardConfig struct defines port as a non-optional u16 field. This suggests either the struct definition is incorrect or this code is outdated. The struct should have port: Option<u16> if this fallback is intended.
  src/services/instance.rs:1
• The hardcoded node name 'pve' is a placeholder that could cause issues in production. This should either retrieve the actual node name from the instance metadata or be documented as a limitation that requires configuration.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The package requires Bun >=1.0.0 but doesn't document this requirement prominently in the README or installation instructions. Users following the installation guide might not realize they need Bun installed.

In general, the fix is to stop disabling TLS certificate verification. With reqwest::Client::builder, certificate and hostname verification are enabled by default, so the most direct fix is to either remove the danger_accept_invalid_certs call or set it to false. This preserves all other behavior (timeouts, headers, API logic) while restoring secure TLS handling.

The best targeted fix here is to edit init_http_client in src/api/vyos.rs so that the Client::builder() no longer calls .danger_accept_invalid_certs(true). We can simply remove that method call entirely, allowing the default secure behavior. No other logic in this file depends on certificates being invalid or on that option’s return value, so this change does not affect functionality beyond enforcing proper TLS verification. No new imports or helper methods are needed.

Concretely:

• In src/api/vyos.rs, within fn init_http_client(&mut self) -> Result<()>, locate the Client::builder() chain.
• Remove the line .danger_accept_invalid_certs(true) // VyOS might use self-signed certs.
• Optionally keep the comment adjusted if you want to document that a proper certificate (or trust anchor configuration) is now required, but that is not strictly necessary for the fix.