mirror of
https://github.com/azaion/gps-denied-onboard.git
synced 2026-06-21 10:41:12 +00:00
bf13549b32
ci/woodpecker/push/02-build-push Pipeline failed
- Enhanced `.env.example` with detailed CMake build flags and replay-mode strategy flags for development and CI environments. - Updated `.gitignore` to include a new deploy rollback bookmark. - Revised `_docs/_autodev_state.md` to reflect the current task status and steps. - Added new lessons to `_docs/LESSONS.md` regarding testing and architectural improvements. - Documented changes in `_docs/02_document/deployment/ci_cd_pipeline.md` to reflect the relaxed OpenCV version pin. - Updated test data documentation in `_docs/02_document/tests/test-data.md` to clarify fixture usage and paths. This commit continues the cycle-1 documentation sync and addresses various configuration updates for improved clarity and functionality.
113 lines
12 KiB
Markdown
113 lines
12 KiB
Markdown
# Phase 2 — Static Analysis (SAST)
|
|
|
|
**Scan date**: 2026-05-19
|
|
**Scope**: `src/gps_denied_onboard/**` — 259 Python files across 14 components, 8 helpers, composition root, fdr_client, frame_source, helpers, logging, replay_input, CLI, healthcheck.
|
|
**Excluded**: `tests/`, `e2e/`, `scripts/` (project-controlled test code with no production exposure); third-party deps (covered by Phase 1).
|
|
**Method**: pattern-driven ripgrep scan against the SAST checklist in `.cursor/skills/security/SKILL.md` § Phase 2 — Static Analysis. Findings cross-validated by reading the call site.
|
|
|
|
## Findings
|
|
|
|
**Total: 0 Critical, 0 High, 0 Medium, 1 Low (informational).**
|
|
|
|
The SUT is a closed-system, single-tenant onboard binary that does not accept external untrusted input on any production path. The only HTTP-inbound surface is the FastAPI test mock (`e2e/fixtures/mock-suite-sat`, out of this Phase 2 scope). The only HTTP-outbound surfaces are C11 `TileUploader` / `TileDownloader` against a pinned host, and C12 `FlightsApiClient` against a pinned host. All inputs the SUT acts on are file-backed (tile cache, IMU CSV, FDR fixtures, calibration JSON) and either operator-provisioned or built by the SUT itself.
|
|
|
|
| # | Severity | Category | Location | Title |
|
|
|---|---|---|---|---|
|
|
| F13 | Low (informational) | Hardening | `src/gps_denied_onboard/runtime_root/*_factory.py` (7 sites) | `__import__(module_name, fromlist=...)` used for plugin loading — input is config-validated against a closed whitelist; recommend a hardening assertion |
|
|
|
|
## Pattern-by-Pattern Results
|
|
|
|
### Injection
|
|
|
|
| Pattern | Result | Evidence |
|
|
|---|---|---|
|
|
| SQL injection via f-string / `.format()` / `+` concatenation into `cursor.execute` | **0 findings** | All 19 `cur.execute(...)` sites in `c6_tile_cache/{postgres_filesystem_store,freshness_gate,migrations,tools}.py` use psycopg `%s` placeholders. The two sites that use dynamic clause composition (`postgres_filesystem_store.py:523-533`, `:918-927`) build the SQL template from hardcoded column names + `%s` placeholders and parameterize ALL value bindings via `tuple(params)` / `params` tuple. Filter columns are hardcoded — only filter VALUES are user/config-controlled and they go through the placeholder. |
|
|
| Command injection — `shell=True` / `os.system` / `os.popen` | **0 findings** | 1 `subprocess.run` call total — `c7_inference/tensorrt_runtime.py:826` for `trtexec` (TensorRT engine builder). `shell=False` (default with list args), arguments come from internal `build_config` (precision mode, optimization profiles, paths from SUT-managed `engine_path` / `model_path`). No untrusted input on this path. Timeout + capture_output + structured `EngineBuildError` on non-zero exit. |
|
|
| XSS via unsanitized output in HTML | **0 findings** | SUT has no HTML output surface. |
|
|
| Template injection (jinja2 `Template()` with user data) | **0 findings** | jinja2 is a transitive dep of `cryptography`; SUT does not import it directly. |
|
|
|
|
### Authentication & Authorization
|
|
|
|
| Pattern | Result | Evidence |
|
|
|---|---|---|
|
|
| Hardcoded credentials / API keys / passwords / tokens (literal `password=`/`secret=`/`api_key=`/`token=` with 8+ char string) | **0 findings** | `c12_operator_orchestrator/config.py:133` has `flights_api_auth_token: str = ""` — empty default, value injected via `Config` at runtime. No literal secret. |
|
|
| Missing authentication on endpoints | **N/A** | SUT exposes no inbound HTTP/RPC endpoint in production. Healthcheck binary (`src/gps_denied_onboard/healthcheck.py`) is a local exit-code probe. |
|
|
| Missing authorization checks (IDOR / privilege escalation paths) | **N/A** | Single-tenant onboard binary; no per-user authorization model. |
|
|
| Weak password validation | **N/A** | No password handling. |
|
|
|
|
### Cryptographic Failures
|
|
|
|
| Pattern | Result | Evidence |
|
|
|---|---|---|
|
|
| Weak hash algorithms (MD5, SHA1) on security-critical paths | **0 findings** | 0 `hashlib.md5(` / `hashlib.sha1(` / `.md5(` / `.sha1(` matches in `src/`. Content hashing uses SHA-256 (`helpers/sha256_sidecar.py`, `c6/postgres_filesystem_store.py` content_sha256 column). Ed25519 signing for tile uploads (C11 `PerFlightKeyManager`, AZ-318). |
|
|
| Plaintext password storage | **0 findings** | No password store. |
|
|
| Hardcoded encryption keys / salts | **0 findings** | All key material is generated per-flight: `c11_tile_manager/signing_key.py` uses `Ed25519PrivateKey.generate()`; MAVLink signing passkey at `c8_fc_adapter/pymavlink_ardupilot_adapter.py:513` uses `secrets.token_bytes(_SIGNING_KEY_LEN)` (cryptographically secure RNG). The MAVLink-side passkey is loaded from a Docker secret (`MAVLINK_SIGNING_PASSKEY_FILE`) for the test path; production binary reads the same secret from a tmpfs-mounted runtime secret. |
|
|
| Missing TLS/HTTPS enforcement (`verify=False`, `VERIFY_NONE`, `InsecureRequestWarning`) | **0 findings** | 0 matches. All `httpx.Client(...)` constructions use default settings (TLS verification ON). Production hosts are pinned at compile/config time. |
|
|
| Secret zeroisation on session end | **GOOD pattern** | `c11_tile_manager/signing_key.py:340-365` `_zeroise_secret_buffer()` overwrites the project-controlled `bytearray` with zeros; OpenSSL-side buffer freed via `Ed25519PrivateKey` refcount drop. Double-storage tradeoff documented (AZ-318 Risk-1). Best-effort, bounded by upload-session lifetime, mitigated by RESTRICT-OPS-1 (operator workstation no-swap). |
|
|
|
|
### Data Exposure
|
|
|
|
| Pattern | Result | Evidence |
|
|
|---|---|---|
|
|
| Secrets / tokens / passwords leaked in logs / error messages | **0 findings** | C12 `flights_api/httpx_client.py:140,175,191` redacts the auth token as `auth_token={_REDACTED}` in both success and retry log paths. 13 source files use the `_REDACTED` / `redact` convention (audited list in `Grep` output above). |
|
|
| Sensitive fields in API responses | **N/A** | SUT publishes no API. |
|
|
| Debug endpoints / verbose error mode in prod | **0 findings** | No FastAPI / Flask / Django app in production scope. CLI exits with structured exception messages; full traceback only when `DEBUG=1` is explicitly set. |
|
|
| Secrets in version control | **0 findings** | `e2e/fixtures/secrets/mavlink-test-passkey.txt` is a documented TEST-ONLY passkey (commit comment: "TEST ONLY"); explicitly excluded from production deployment per Phase 1 / Dockerfile build args. No `.env`, `.envrc`, or `config.local.*` with literal secrets found. |
|
|
|
|
### Insecure Deserialization
|
|
|
|
| Pattern | Result | Evidence |
|
|
|---|---|---|
|
|
| `pickle.loads()` / `marshal.loads()` / `cPickle.*` on untrusted data | **0 findings** | 0 matches in `src/`. (The 7 `model.eval()` matches found by the original SAST regex were PyTorch `nn.Module.eval()` mode-switch calls, not Python `eval`.) |
|
|
| `eval()` / `exec()` of strings | **0 findings** | 0 matches. |
|
|
| `yaml.load()` / `yaml.unsafe_load()` on untrusted data | **0 findings** | 0 matches in `src/`. (Config loader uses `yaml.safe_load`, validated in module-layout review.) |
|
|
| `json.loads()` on untrusted data | **0 findings** | 3 sites total: `c10_provisioning/provisioner.py:512` reads SUT-built manifest; `c1_vio/bench/okvis2.py:64,73` reads SUT-built calibration / IMU bench fixtures; `runtime_root/airborne_bootstrap.py:965` reads operator-provisioned calibration. All inputs are filesystem-local, operator-controlled, schema-validated downstream. |
|
|
| `__import__()` of dynamic strings | **F13** (informational) | 7 sites — `runtime_root/{vpr,vio,matcher,rerank,refiner,inference,c11,...}_factory.py`. Each call passes a `module_name` looked up from a CLOSED `_STRATEGY_TO_MODULE` table keyed by `config.components[<c>].strategy`, which `<C>Config.__post_init__` validates against `KNOWN_*_STRATEGIES`. The input is config-validated against a whitelist BEFORE the import. Documented in the strategy-registry comments (AZ-591). |
|
|
|
|
### F13 Detail — `__import__` plugin loading
|
|
|
|
**Severity**: Low (informational / hardening recommendation, not a current vulnerability).
|
|
|
|
**Location**: `src/gps_denied_onboard/runtime_root/{vpr,vio,matcher,rerank,refiner,inference,c11}_factory.py` (7 call sites).
|
|
|
|
**Description**: The composition-root factories use `__import__(module_name, fromlist=[class_name])` to load concrete strategy implementations lazily. Today this is safe because `module_name` is looked up via a closed `_STRATEGY_TO_MODULE` dict keyed by `strategy: str`, and `strategy` is validated against `KNOWN_*_STRATEGIES` in each component's `<C>Config.__post_init__`. The validation gate happens at `Config` instantiation, before `compose_root` ever runs.
|
|
|
|
**Impact**: None today — config validation blocks any string from reaching `__import__` that isn't in the whitelist.
|
|
|
|
**Failure mode**: if a future refactor were to drop the `KNOWN_*_STRATEGIES` validation, or if a strategy were added to `_STRATEGY_TO_MODULE` without a matching `KNOWN_*` entry, the failure mode would be "config-file-controlled module import" — an attacker who controls the config YAML could load arbitrary importable modules. The threat is bounded because (a) the config file is operator-provisioned (not network-fetched), and (b) the worst-case payload is "load and call `create(...)` on a module already on `sys.path`", which is the project's own dependency closure.
|
|
|
|
**Remediation (hardening)**: add a defensive assertion at the start of each `_factory.py`'s build function:
|
|
|
|
```python
|
|
if strategy not in KNOWN_*_STRATEGIES:
|
|
raise StrategyNotAvailableError(
|
|
f"Strategy {strategy!r} not in whitelist; refusing dynamic import."
|
|
)
|
|
```
|
|
|
|
Today this is enforced by `<C>Config.__post_init__` — the recommended hardening is to repeat the check at the import call site so the invariant survives future refactors. Not blocking for production.
|
|
|
|
## Architectural Safeguards Observed (defense-in-depth)
|
|
|
|
These are not findings — they are positive observations that reduce the project's overall attack surface:
|
|
|
|
| Safeguard | Location | Why it matters |
|
|
|---|---|---|
|
|
| Public-boundary discipline for the blackbox harness | `e2e/README.md` § Public-Boundary Discipline + `e2e/runner/Dockerfile` (no SUT install) | The test runner cannot import any SUT module, eliminating cross-contamination of test fixtures into the production code path. |
|
|
| `paramiko` `RejectPolicy` (NOT `AutoAddPolicy`) | `c12_operator_orchestrator/paramiko_ssh_session.py:218` | C12 SSH to operator companion always validates host key against pinned `known_hosts`. CVE-2026-44405 (SHA-1 RSA, Phase 1 F6) is materially mitigated by this. |
|
|
| Per-flight ephemeral Ed25519 signing key | `c11_tile_manager/signing_key.py` | Compromise of a single flight key only exposes that flight's upload window; key never persists past `end_session`. |
|
|
| Cryptographically-secure RNG for MAVLink passkey generation | `c8_fc_adapter/pymavlink_ardupilot_adapter.py:513` (`secrets.token_bytes`) | Not `random.random()`. |
|
|
| Internal-only Docker network for test stack | `e2e/docker/docker-compose.test.yml` (`e2e-net.internal: true`) | RESTRICT-SAT-1 / NFT-SEC-02 enforcement — runtime network egress to non-`e2e-net` destinations is impossible during tests. |
|
|
| Token redaction in logs (`_REDACTED` convention) | 13 source files including `c12_operator_orchestrator/{cli,build_cache,operator_reloc_service}`, `c11_tile_manager/{config,tile_downloader}`, `c13_fdr/headers`, `c10_provisioning/manifest_builder`, `cli/replay` | Disciplined enforcement across components; not just one-off in C12. |
|
|
| Build-flag gates (`BUILD_PYTORCH_FP16_RUNTIME`, `BUILD_FAISS_INDEX`, `BUILD_VPR_*`, etc.) at composition-root level | `runtime_root/*_factory.py` | Strategies are excluded from the production binary at compile time, not just runtime — eliminates classes of misconfiguration attacks. |
|
|
| `verify=True` (default) on all `httpx.Client` constructions | `c11_tile_manager/{tile_downloader,tile_uploader}.py`, `c12_operator_orchestrator/flights_api/httpx_client.py`, `runtime_root/c11_factory.py` | TLS validation is never bypassed. |
|
|
|
|
## Self-Verification
|
|
|
|
- [x] All `src/gps_denied_onboard/` Python files scanned (259 files)
|
|
- [x] Each finding has file:line evidence
|
|
- [x] No false positives from comments or test files (e.g., the `model.eval()` PyTorch matches were verified by reading the call site)
|
|
- [x] All `cur.execute(...)` sites in C6 reviewed for parameterization
|
|
- [x] All `subprocess.*` / `os.system` / `eval` / `exec` / `pickle.loads` / `__import__` matches reviewed
|
|
- [x] All `verify=False` / weak-crypto / hardcoded-secret patterns checked
|