[codex] Harden repo security audit findings

crates/aprs-cli/src/main.rs

@@ -186,7 +186,13 @@ fn parse_args(args: impl IntoIterator<Item = String>) -> Result<CliOptions, Stri
                 options.fail_on = parse_fail_on(&value)?;
             }
             "--help" | "-h" => return Err(usage()),
-            _ if arg.starts_with('-') => return Err(format!("unknown option: {arg}\n{}", usage())),
+            _ if arg.starts_with('-') => {
+                return Err(format!(
+                    "unknown option: {}\n{}",
+                    diagnostic_value(&arg),
+                    usage()
+                ));
+            }
             _ => {
                 if options.input_path.replace(arg).is_some() {
                     return Err(format!("multiple input paths supplied\n{}", usage()));
@@ -203,7 +209,11 @@ fn parse_fail_on(value: &str) -> Result<FailOn, String> {
         "none" => Ok(FailOn::None),
         "malformed" => Ok(FailOn::Malformed),
         "rejected" => Ok(FailOn::Rejected),
-        _ => Err(format!("invalid --fail-on value: {value}\n{}", usage())),
+        _ => Err(format!(
+            "invalid --fail-on value: {}\n{}",
+            diagnostic_value(value),
+            usage()
+        )),
     }
 }
 
@@ -224,11 +234,14 @@ fn matches_filter(options: &CliOptions, semantic: &str) -> bool {
 
 fn read_input(path: Option<&str>) -> Result<Vec<u8>, String> {
     match path {
-        Some(path) => read_all_with_limit(
-            File::open(path).map_err(|err| format!("failed to open {path}: {err}"))?,
-            libaprs_engine::DEFAULT_TRANSPORT_READ_LIMIT,
-        )
-        .map_err(|err| format!("failed to read {path}: {err}")),
+        Some(path) => {
+            let display_path = diagnostic_value(path);
+            read_all_with_limit(
+                File::open(path).map_err(|err| format!("failed to open {display_path}: {err}"))?,
+                libaprs_engine::DEFAULT_TRANSPORT_READ_LIMIT,
+            )
+            .map_err(|err| format!("failed to read {display_path}: {err}"))
+        }
         None => read_all_with_limit(io::stdin(), libaprs_engine::DEFAULT_TRANSPORT_READ_LIMIT)
             .map_err(|err| format!("failed to read stdin: {err}")),
     }
@@ -381,6 +394,10 @@ fn lossy(bytes: &[u8]) -> String {
     String::from_utf8_lossy(bytes).into_owned()
 }
 
+fn diagnostic_value(value: &str) -> String {
+    format!("{value:?}")
+}
+
 fn usage() -> String {
     "usage: aprs-cli [parse|validate|stats|explain|replay|support-matrix] [--json] [--permissive] [--explain] [--summary] [--filter SEMANTIC] [--fail-on none|malformed|rejected] [PATH]".to_string()
 }

crates/aprs-cli/tests/cli.rs

@@ -110,6 +110,30 @@ fn cli_rejects_oversized_file_input() {
     let _ = std::fs::remove_file(path);
 }
 
+#[test]
+fn cli_diagnostic_errors_escape_control_characters() {
+    let binary = env!("CARGO_BIN_EXE_aprs-cli");
+    let output = Command::new(binary)
+        .arg("--bad\noption")
+        .output()
+        .expect("CLI should run");
+
+    assert_eq!(output.status.code(), Some(2));
+    let stderr = String::from_utf8(output.stderr).expect("stderr should be utf8");
+    assert!(stderr.contains("unknown option: \"--bad\\noption\""));
+    assert!(!stderr.contains("unknown option: --bad\noption"));
+
+    let output = Command::new(binary)
+        .args(["--fail-on", "bad\rvalue"])
+        .output()
+        .expect("CLI should run");
+
+    assert_eq!(output.status.code(), Some(2));
+    let stderr = String::from_utf8(output.stderr).expect("stderr should be utf8");
+    assert!(stderr.contains("invalid --fail-on value: \"bad\\rvalue\""));
+    assert!(!stderr.contains("invalid --fail-on value: bad\rvalue"));
+}
+
 #[test]
 fn cli_validate_command_reports_validity() {
     let binary = env!("CARGO_BIN_EXE_aprs-cli");

crates/aprs-transport-aprs-is/examples/reader.rs

@@ -8,7 +8,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         software: "libaprs-engine 1.1.0",
         filter: Some("r/49/-72/50"),
     };
-    assert!(login.line()?.ends_with("\r\n"));
+    assert!(login.profile_line()?.ends_with("\r\n"));
 
     let input = std::io::Cursor::new(b"# aprs-is banner\r\nN0CALL>APRS:>aprs-is\n");
     for packet_bytes in read_packet_lines_from_reader(input)? {

crates/aprs-transport-aprs-is/examples/session_reconnect.rs

@@ -51,7 +51,7 @@ fn run_receive_session<R>(
 where
     R: io::Read,
 {
-    let login_line = login.line().map_err(io::Error::other)?;
+    let login_line = login.profile_line().map_err(io::Error::other)?;
 
     for attempt in 0..plan.max_attempts {
         match connect(attempt) {

crates/aprs-transport-aprs-is/src/lib.rs

@@ -29,8 +29,9 @@ pub struct AprsIsLogin<'a> {
 impl AprsIsLogin<'_> {
     /// Builds the APRS-IS login line terminated with CRLF.
     ///
-    /// Values containing CR or LF are rejected to prevent line injection into
-    /// the APRS-IS control stream.
+    /// Values containing CR, LF, or other ASCII control bytes are rejected to
+    /// prevent control-line injection. Prefer [`Self::profile_line`] when
+    /// fields may come from untrusted input.
     pub fn line(&self) -> Result<String, AprsIsLoginError> {
         validate_login_field("callsign", self.callsign)?;
         validate_login_field("software", self.software)?;
@@ -78,7 +79,7 @@ impl AprsIsLogin<'_> {
 /// APRS-IS login line validation error.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum AprsIsLoginError {
-    /// A login field contains CR or LF and would inject another line.
+    /// A login field contains CR, LF, or another ASCII control byte.
     LineInjection { field: &'static str },
 }
 
@@ -95,9 +96,10 @@ impl AprsIsLoginError {
 impl std::fmt::Display for AprsIsLoginError {
     fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::LineInjection { field } => {
-                write!(formatter, "APRS-IS login field contains CR or LF: {field}")
-            }
+            Self::LineInjection { field } => write!(
+                formatter,
+                "APRS-IS login field contains CR, LF, or control byte: {field}"
+            ),
         }
     }
 }
@@ -107,7 +109,7 @@ impl std::error::Error for AprsIsLoginError {}
 /// APRS-IS profile validation error.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum AprsIsProfileError {
-    /// A profile field contains CR or LF and would inject another line.
+    /// A profile field contains CR, LF, or another ASCII control byte.
     LineInjection { field: &'static str },
     /// A callsign does not fit the conservative AX.25-like login shape.
     InvalidCallsign,
@@ -136,7 +138,7 @@ impl std::fmt::Display for AprsIsProfileError {
             Self::LineInjection { field } => {
                 write!(
                     formatter,
-                    "APRS-IS profile field contains CR or LF: {field}"
+                    "APRS-IS profile field contains CR, LF, or control byte: {field}"
                 )
             }
             Self::InvalidCallsign => formatter.write_str("APRS-IS profile callsign is invalid"),
@@ -351,14 +353,14 @@ fn read_all(reader: impl Read, max_bytes: usize) -> io::Result<Vec<u8>> {
 }
 
 fn validate_profile_field(field: &'static str, value: &str) -> Result<(), AprsIsProfileError> {
-    if value.as_bytes().contains(&b'\r') || value.as_bytes().contains(&b'\n') {
+    if value.as_bytes().iter().any(u8::is_ascii_control) {
         return Err(AprsIsProfileError::LineInjection { field });
     }
     Ok(())
 }
 
 fn validate_login_field(field: &'static str, value: &str) -> Result<(), AprsIsLoginError> {
-    if value.as_bytes().contains(&b'\r') || value.as_bytes().contains(&b'\n') {
+    if value.as_bytes().iter().any(u8::is_ascii_control) {
         return Err(AprsIsLoginError::LineInjection { field });
     }
     Ok(())

crates/aprs-transport-aprs-is/tests/aprs_is_transport.rs

@@ -41,6 +41,12 @@ fn aprs_is_login_line_rejects_line_injection() {
             software: "libaprs-engine 1.1.0",
             filter: Some("r/49/-72/50\r\nbad"),
         },
+        AprsIsLogin {
+            callsign: "N0CALL",
+            passcode: -1,
+            software: "libaprs-engine\t1.1.0",
+            filter: None,
+        },
     ];
 
     for login in cases {
@@ -75,6 +81,20 @@ fn aprs_is_profile_login_requires_uppercase_callsign_and_valid_filter() {
         lowercase.profile_line().expect_err("lowercase callsign"),
         AprsIsProfileError::LowercaseCallsign
     );
+
+    let control_byte = AprsIsLogin {
+        callsign: "N0CALL-7",
+        passcode: -1,
+        software: "libaprs-engine\u{1b}",
+        filter: None,
+    };
+
+    assert_eq!(
+        control_byte
+            .profile_line()
+            .expect_err("control bytes fail closed"),
+        AprsIsProfileError::LineInjection { field: "software" }
+    );
 }
 
 #[test]

crates/aprs-transport-tcp/src/lib.rs

@@ -14,6 +14,12 @@ use libaprs_engine::{
     read_all_with_limit, LineTransport, DEFAULT_TRANSPORT_READ_LIMIT, MAX_PACKET_LEN,
 };
 
+/// Default timeout for establishing a TCP connection.
+pub const DEFAULT_TCP_CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
+
+/// Default timeout for reads after a TCP connection is established.
+pub const DEFAULT_TCP_READ_TIMEOUT: Duration = Duration::from_secs(10);
+
 /// TCP read options owned by the caller.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub struct TcpReadOptions {
@@ -51,8 +57,8 @@ impl TcpReadOptions {
 impl Default for TcpReadOptions {
     fn default() -> Self {
         Self {
-            connect_timeout: None,
-            read_timeout: None,
+            connect_timeout: Some(DEFAULT_TCP_CONNECT_TIMEOUT),
+            read_timeout: Some(DEFAULT_TCP_READ_TIMEOUT),
             max_bytes: DEFAULT_TRANSPORT_READ_LIMIT,
         }
     }

crates/aprs-transport-tcp/tests/tcp_transport.rs

@@ -5,7 +5,8 @@ use std::time::Duration;
 
 use aprs_transport_tcp::{
     read_packet_lines_from_reader, read_packet_lines_from_reader_with_limit,
-    read_packet_lines_from_tcp_addr_with_options, TcpReadOptions,
+    read_packet_lines_from_tcp_addr_with_options, TcpReadOptions, DEFAULT_TCP_CONNECT_TIMEOUT,
+    DEFAULT_TCP_READ_TIMEOUT,
 };
 
 #[test]
@@ -71,6 +72,14 @@ fn tcp_addr_helper_applies_caller_owned_read_timeout() {
     server.join().expect("server thread");
 }
 
+#[test]
+fn tcp_options_default_uses_finite_timeouts() {
+    let options = TcpReadOptions::default();
+
+    assert_eq!(options.connect_timeout, Some(DEFAULT_TCP_CONNECT_TIMEOUT));
+    assert_eq!(options.read_timeout, Some(DEFAULT_TCP_READ_TIMEOUT));
+}
+
 #[test]
 fn tcp_options_builder_api_remains_source_compatible() {
     let options = TcpReadOptions::default()

crates/libaprs-engine/src/lib.rs

@@ -451,7 +451,7 @@ const TRANSPORT_SUPPORT: &[TransportSupport] = &[
         crate_name: "aprs-transport-tcp",
         boundary: "blocking TCP or Read packet streams",
         status: SupportStatus::Supported,
-        notes: "caller owns socket timeouts and reconnect behavior",
+        notes: "finite default socket timeouts; caller owns reconnect behavior",
     },
     TransportSupport {
         crate_name: "aprs-transport-aprs-is",

docs/release-notes-v3.0.0-rc.1.md

@@ -33,6 +33,8 @@
   source, semver, or behavior regression before final `3.0.0`.
 - Post-publication downstream smoke must regenerate its lockfile from crates.io
   so checksum evidence matches the published RC crates.
+- The RC fix-forward audit summary is tracked in
+  [v3.0.0-rc.1 Security Audit Summary](security-audit-v3.0.0-rc.1.md).
 
 ## Release Gates
 

docs/security-audit-v3.0.0-rc.1.md

@@ -0,0 +1,45 @@
+# v3.0.0-rc.1 Security Audit Summary
+
+## BLUF
+
+- Repo-wide source security audit was repeated during the `v3.0.0-rc.1`
+  fix-forward pass.
+- No surviving reportable security findings remain after the fixes in commit
+  `f5179df`.
+- The fixes harden CLI diagnostics, APRS-IS login field validation, and TCP
+  default timeout behavior.
+- Verification passed with formatting, focused tests, full workspace tests,
+  clippy with warnings denied, and docs verification.
+- Merge should proceed only through PR checks and the normal release gates.
+
+## Closed Findings
+
+- CLI diagnostic control-character injection: user-controlled option, fail-on,
+  and path values are now escaped before display.
+- APRS-IS login/profile ASCII-control injection: login fields now reject all
+  ASCII control bytes, not only CR and LF.
+- TCP default indefinite blocking: default TCP connect and read options now use
+  finite timeouts; callers can explicitly opt into blocking behavior by setting
+  timeout fields to `None`.
+
+## Verification Evidence
+
+- `cargo fmt --check`
+- `cargo test -p aprs-cli -p aprs-transport-aprs-is`
+- `cargo test -p aprs-transport-tcp -p libaprs-engine`
+- `cargo test --workspace`
+- `cargo clippy --workspace --all-targets --all-features -- -D warnings`
+- `scripts/verify-docs.sh`
+
+All commands passed. `scripts/verify-docs.sh` also passed after this summary was
+added and linked.
+
+## PR Task List
+
+- [x] Add tracked security-audit summary for release evidence.
+- [ ] Push `codex/repo-security-audit-fix-forward`.
+- [ ] Open PR to `main`.
+- [ ] Wait for CI, security, merge-gate, and supply-chain checks.
+- [ ] Fix forward on the same branch if any check fails.
+- [ ] Merge only when all checks and secure-review findings are clean.
+- [ ] Rerun release verification from fresh `main` after merge.

docs/security.md

@@ -28,6 +28,9 @@ conservative address metadata needed to establish safe structure.
 See [Threat Model](threat-model.md) for the per-crate untrusted boundaries,
 primary abuse cases, and required controls.
 
+For the current release-candidate audit evidence, see
+[v3.0.0-rc.1 Security Audit Summary](security-audit-v3.0.0-rc.1.md).
+
 The current trusted boundary is:
 
 ```text

docs/threat-model.md

@@ -41,7 +41,7 @@ no lossy text conversion before the codec boundary.
 | `aprs-transport-file` | packet files and reader-backed file input | oversized files, overlong lines, invalid UTF-8 | bounded reads, per-packet limits, byte-preserving records |
 | `aprs-transport-file-watch` | appended file bytes | unbounded append growth, partial records | appended-byte limits, packet-line limits, caller-owned polling policy |
 | `aprs-transport-corpus` | corpus directories and files | private data leakage, oversized corpus files, unstable ordering | bounded file reads, stable ordering, fuzz corpus guard for regression seeds |
-| `aprs-transport-tcp` | TCP streams and generic readers | stalled streams, overlong lines, retry storms | caller-owned timeouts/reconnects, bounded reads, packet-line limits |
+| `aprs-transport-tcp` | TCP streams and generic readers | stalled streams, overlong lines, retry storms | finite default timeouts, caller-owned reconnects, bounded reads, packet-line limits |
 | `aprs-transport-aprs-is` | APRS-IS server lines and login filters | line injection, server comments, oversized lines | CRLF-safe login construction, comment filtering, line limits |
 | `aprs-transport-serial` | serial readers | partial records, invalid bytes, oversized batches | caller-owned serial configuration, bounded reads, packet-line limits |
 | `aprs-transport-http` | HTTP body bytes | oversized request bodies, malformed line framing | body-size limits, packet-line limits, no text normalization |

docs/transports.md

@@ -20,7 +20,7 @@ parser, preserving the protocol-first boundary.
 | Crate | Boundary | Primary use | Security note |
 | --- | --- | --- | --- |
 | `aprs-transport-file` | Newline-separated byte buffers and files | Offline logs, stdin-style files | Bounded path helpers reject oversized batches and packet lines |
-| `aprs-transport-tcp` | Blocking `Read` and TCP address helpers | TCP-connected packet streams | Reader helpers reject oversized batches and packet lines; `TcpReadOptions` keeps connection and read timeouts caller-owned |
+| `aprs-transport-tcp` | Blocking `Read` and TCP address helpers | TCP-connected packet streams | Reader helpers reject oversized batches and packet lines; `TcpReadOptions` applies finite default timeouts that callers can override |
 | `aprs-transport-aprs-is` | APRS-IS login, filters, q constructs, and comment filtering | APRS-IS clients | Server comment lines are filtered before parsing and packet lines are bounded |
 | `aprs-transport-kiss` | KISS frame encoding and decoding | TNC, serial, or TCP KISS streams | Invalid escapes and oversized decoded payloads fail closed |
 | `aprs-transport-serial` | Serial-like byte readers | TNC serial pipelines | Reader helpers reject oversized batches and packet lines; serial configuration stays application-owned |
@@ -101,9 +101,9 @@ silently weakening packet parsing.
 
 For reconnecting services, keep session ownership in the application. The
 compile-tested `crates/aprs-transport-aprs-is/examples/session_reconnect.rs`
-example shows an APRS-IS login line, bounded reader helper, retry loop, backoff,
-and `Engine::process_event()` integration without adding networking behavior to
-the parser core.
+example shows a profile-validated APRS-IS login line, bounded reader helper,
+retry loop, backoff, and `Engine::process_event()` integration without adding
+networking behavior to the parser core.
 
 ## TCP With Caller-Owned Timeouts
 

examples/downstream-smoke/src/main.rs

@@ -30,7 +30,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         software: "libaprs-engine-downstream-smoke 3.0.0-rc.1",
         filter: Some("r/49/-72/50"),
     };
-    assert!(aprs_is_login.line()?.ends_with("\r\n"));
+    assert!(aprs_is_login.profile_line()?.ends_with("\r\n"));
 
     let aprs_is_packets = read_aprs_is_packet_lines(b"# banner\nN0CALL>APRS:>aprs-is\n");
     assert_eq!(aprs_is_packets.len(), 1);