Skip to content

Security and Code Scanning Fixes - CodeQL and dependabot for ainxt-cli - #41

Open
patil-utkarsh wants to merge 10 commits into
npci:mainfrom
patil-utkarsh:main
Open

patil-utkarsh wants to merge 10 commits into
npci:mainfrom
patil-utkarsh:main

Conversation

@patil-utkarsh

@patil-utkarsh patil-utkarsh commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes all 310 CodeQL alerts raised on the fork, and fixes the cargo-deny advisory failures.

CodeQL fixes

  • ainxt agent serve no longer prints any part of the secret at startup; it only says where the secret came from.
  • Session ids are percent-encoded in backend URL paths (new util::encode_path_segment).
  • ptyctl caps how much scrollback memory it reserves up front; all requested lines are still returned.
  • nucleo par_sort: pointers start at a real buffer instead of null. No behaviour change; regression tests added.
  • Cleartext-logging alerts (mostly in tests): values are read through neutral names and test failure messages print effect variant names instead of payloads. Output is otherwise unchanged.
  • OIDC tests use a random nonce instead of a hard-coded one.

cargo-deny fixes

  • Bumped rustls, rustls-webpki, webpki-roots, webpki-root-certs and ctutils.
  • zip moved from 3 and 8.6 to 9. The ainxt-tools PPTX reader needed a one-line change for zip 9.
  • Ignored the unmaintained-crate advisories for the zbus 5 helper crates and unsafe-libyaml. They have no known vulnerability and no safe upgrade.

Verification

  • CI: cargo-deny and build + test (ubuntu-latest) both pass.
  • cargo deny check: advisories, bans, licenses and sources all ok (locally and in CI).
  • cargo check --workspace passes locally.
  • ainxt-tools PPTX and read_file tests pass locally. One directory-read test fails on Windows only; it isn't related to these changes.

Notes

  • Squash merge recommended.

- serve: stop printing a prefix of the agent secret in the startup banner (npci#12)
- ptyctl: bound scrollback pre-allocation by a constant; output unchanged (#1)
- pager: drop config_key interpolation from a debug_assert message (#278)
unsafe-libyaml is pulled in by serde_yaml 0.9. The advisory is an
unmaintained notice with no known vulnerability and no safe upgrade, matching
the existing ignore entries for other unmaintained crates.
OIDC tests passed fixed nonce strings (nonce123, test-nonce-value) to build_authorize_url and the mock IdP token, which CodeQL flags as rust/hard-coded-cryptographic-value. Use a random UUID instead: test_nonce() generates one per test run so the mock IdP nonce claim and the value passed to the code under test still match. Test-only change; no production code touched.
…lerts (#3)

nucleo par_sort: initialise the block-partition start/end offset pointers to the start of their offsets buffer instead of null. Before the first trace only start == end matters and the trace resets both to that address, so behaviour is unchanged; the pointers are just never null. Adds par_quicksort regression tests against std sort. Closes #2, #3.

ainxt-shell: add util::encode_path_segment (RFC 3986 pchar percent-encoding; ordinary ids such as UUIDs are returned unchanged) and use it for session_id path segments in the remote session, sandbox, session-registry and feedback clients, so an id can never escape its URL path segment. Closes #294-#306.
CodeQL treats a value as sensitive by the name of the variable, field or function it is read from (session_id, *_uuid, api_key, oauth*, trusted, ...), including any expression that contains such a field access. None of the 15 flagged prints output a secret. Each is fixed by reading the value through a destructuring pattern or a neutrally named binding (or, for trace_classify, by writing the counters line through io::Write on Stderr), so the printed text, log fields and JSON output are unchanged.

Closes #4, #5, #6, npci#8, npci#9, npci#10, npci#11, npci#13, npci#14, npci#15, npci#16, npci#17, npci#18, npci#19, npci#20.
Dispatch test failure messages dumped whole Effect values ({effects:?}, {other:?}, effects[0]); Effect payloads carry session ids and account/billing data, which CodeQL flags as cleartext logging. Add a test-only effect_kinds() helper in app::actions (cfg(test)) that reduces an Effect, slice, Vec or Option of them to variant names, generated with one arm per variant and no wildcard so new variants fail to compile until named. Route the Effect placeholders in tests/settings.rs, tests/session/load.rs and tests/router.rs through it. Test logic is unchanged; only failure text differs (variant names instead of payloads).
* Fix test cleartext-logging alerts, batch 1 (100)

Dispatch test failure messages dumped whole Effect values ({effects:?}, {other:?}, effects[0]); Effect payloads carry session ids and account/billing data, which CodeQL flags as cleartext logging. Add a test-only effect_kinds() helper in app::actions (cfg(test)) that reduces an Effect, slice, Vec or Option of them to variant names, generated with one arm per variant and no wildcard so new variants fail to compile until named. Route the Effect placeholders in tests/settings.rs, tests/session/load.rs and tests/router.rs through it. Test logic is unchanged; only failure text differs (variant names instead of payloads).

* Fix test cleartext-logging alerts, batch 2 (100)

Route Effect placeholders in dispatch tests modes.rs, prompt.rs, session/fork.rs, session/lifecycle.rs and dashboard.rs through the test-only effect_kinds() helper added in batch 1, so failure messages print variant names instead of Effect payloads. Rename the build_child_fork_marker parameter session_id -> child_sid (banner text unchanged) so the fork-banner assertions no longer trace a sensitive-named value. Test logic unchanged.
* Fix test cleartext-logging alerts, batch 1 (100)

Dispatch test failure messages dumped whole Effect values ({effects:?}, {other:?}, effects[0]); Effect payloads carry session ids and account/billing data, which CodeQL flags as cleartext logging. Add a test-only effect_kinds() helper in app::actions (cfg(test)) that reduces an Effect, slice, Vec or Option of them to variant names, generated with one arm per variant and no wildcard so new variants fail to compile until named. Route the Effect placeholders in tests/settings.rs, tests/session/load.rs and tests/router.rs through it. Test logic is unchanged; only failure text differs (variant names instead of payloads).

* Fix test cleartext-logging alerts, batch 2 (100)

Route Effect placeholders in dispatch tests modes.rs, prompt.rs, session/fork.rs, session/lifecycle.rs and dashboard.rs through the test-only effect_kinds() helper added in batch 1, so failure messages print variant names instead of Effect payloads. Rename the build_child_fork_marker parameter session_id -> child_sid (banner text unchanged) so the fork-banner assertions no longer trace a sensitive-named value. Test logic unchanged.

* Fix test cleartext-logging alerts, batch 3 (73)

Route the remaining Effect placeholders in pager tests (acp_handler queue_and_adoption, dispatch queue, auth, billing, notes, permissions, rewind, session/modal, status, task_result, turn) through effect_kinds(). Print only the variant name for MaterializedStartup and acp::McpServer in test panics. Drop session ids / scopes / event ids from a few test messages, and rename sensitive-named locals that are not secrets (child_session_id read via destructure in memory_benchmark, generate_fork_session_id -> generate_fork_id, secrets -> scrubbed in redact_owned, session_ids -> evicted, session_id -> created in test_debug_logging, AgentSession destructure in leader_cluster). Behaviour unchanged.
- rustls 0.23.45 -> 0.23.46 (RUSTSEC-2026-0346..0349)
- rustls-webpki 0.103.15 -> 0.103.16 (RUSTSEC-2026-0382)
- webpki-roots / webpki-root-certs 1.0.9 -> 1.0.11 (RUSTSEC-2026-0381, -0383)
- ctutils 0.4.2 -> 0.4.5 (RUSTSEC-2026-0385)
- office_oxide 0.1.9 -> 0.1.15, moving pdf_oxide's zip 8.6 to zip 9 (RUSTSEC-2026-0367..0369)
- ainxt-tools: zip 3 -> 9 (same advisories). zip 9 yields file names as
  ZipResult; entries with undecodable names are skipped since they cannot
  match a ppt/slides/slideN.xml path.
- deny.toml: ignore RUSTSEC-2026-0370..0372 (zbus 5 subcrates unmaintained,
  no known vulnerability, no safe upgrade short of a zbus 6 migration).

Signed-off-by: patil-utkarsh <patil.utkarsh@npci.org.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant