[codex] Add workspace threat model - #79
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39c6d5a815
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| | TM-002 | Remote high-volume sender | Internet-facing TCP/APRS-IS/HTTP/MQTT/UDP/KISS/AX.25 ingestion exists. Application may raise limits or omit bounded helpers. | Send oversized batches, long lines, large frames, repeated delimiters, or high packet volume to exhaust memory, CPU, queues, or worker threads. | Targeted denial of service against ingestion or downstream processing. | Availability, transport buffers, worker queues | `DEFAULT_TRANSPORT_READ_LIMIT`, `MAX_PACKET_LEN`, `packets_with_limit`, bounded TCP/APRS-IS/KISS/HTTP/MQTT helpers. Evidence: `crates/libaprs-engine/src/transport.rs:5`, `crates/libaprs-engine/src/transport.rs:98`, `crates/aprs-transport-kiss/src/lib.rs:74` | Application owns socket timeouts, cancellation, queue capacity, retries, rate limits, and TLS/auth. | For internet-facing services, require bounded helper APIs, explicit per-source byte limits, queue caps, read deadlines, rate budgets, and rejection telemetry; avoid exposing unbounded convenience helpers to untrusted input. | Alert on `transport.oversized_input`, timeout/error rates, queue saturation, packet rate, read duration, and per-source rejection spikes. | High | Medium | High | | ||
| | TM-003 | Remote or local attacker controlling APRS-IS profile fields | Application accepts callsign/software/filter from user or config not fully trusted. | Inject CR/LF/control bytes or malformed filter syntax into APRS-IS login/profile lines. | Connection command injection or unintended APRS-IS filter behavior. | APRS-IS session integrity, credentials/session state | `AprsIsLogin::line` rejects CR/LF/control bytes; `profile_line` validates callsign and filters. Evidence: `crates/aprs-transport-aprs-is/src/lib.rs:29`, `crates/aprs-transport-aprs-is/src/lib.rs:54` | `line()` is less strict than `profile_line`; downstream may build login strings manually. | Prefer `profile_line()` for any user/config-derived fields; add docs warning against manual login string construction; consider examples using `AprsIsFilter::new` exclusively. | Log validation failures by stable error code without logging raw credentials/passcodes. | Low | High | Medium | | ||
| | TM-004 | Remote sender or malicious corpus contributor | Production or private packet samples are logged, exported, or added to corpus/fixtures. | Include private station callsigns, precise locations, operator messages, credentials, or incident payloads in public corpora, logs, diagnostics, or release evidence. | Confidentiality loss and unwanted public disclosure. | Raw packet bytes, fuzz corpus, fixtures, logs, release evidence | Corpus hygiene guidance and checks; malformed observability caps raw bytes. Evidence: `docs/security.md:150`, `crates/libaprs-engine/src/lib.rs:564`, `docs/transports.md:204` | Automated checks cannot identify all sensitive APRS/operator data; accepted/rejected events may preserve raw bytes by design. | Add a corpus sanitization checklist to PR template for fixture changes; require review for corpora from production; redact or hash raw bytes in external logs unless replay is explicitly needed. | Monitor PRs touching `fuzz/corpus/` and fixture paths; scan for secrets and high-precision private samples before release. | Medium | Medium | Medium | | ||
| | TM-005 | Malicious maintainer, compromised account, or supply-chain attacker | Attacker can alter manifests, workflows, scripts, release evidence, or run publication with credentials. | Publish malicious crates, stale SBOM/hash evidence, wrong release commit, or misleading GitHub Release. | Downstream package compromise and reputational/integrity loss. | Crates.io packages, GitHub Releases, SBOMs, lockfiles, credentials | Security workflow runs audit/deny; supply-chain workflow verifies evidence; publish script requires clean review/gates, matching commit, clean tree, explicit confirmation. Evidence: `.github/workflows/security.yml:83`, `.github/workflows/supply-chain.yml:77`, `scripts/publish-release.sh:37`, `scripts/publish-release.sh:82` | Env var attestations can be set by a maintainer; tag signing/ruleset enforcement is not evidenced in repo files; manual publishing is still possible. | Enforce branch/tag protection and required checks in GitHub rulesets; prefer signed tags/releases; require two-person review for release script/workflow/publishing changes; store crates.io tokens outside repo with least privilege and rotation. | Alert on workflow/script/manifest/SBOM changes, release creation, crates.io publish events, skipped CI attestations, and tag updates. | Medium | High | High | |
There was a problem hiding this comment.
Avoid mandating two-person release review
In this solo-maintainer repo, this mitigation conflicts with the repository guidance in AGENTS.md to treat the project as solo-maintainer and not recommend independent-approval branch protections that block merges; if adopted for release script/workflow/publishing changes, it can prevent the maintainer from landing required release fixes instead of using the documented PR + Merge Gate model.
Useful? React with 👍 / 👎.
Summary
Validation
scripts/verify-docs.shSecurity review