Skip to content

Fix 12 issues from second code audit#2

Merged
felizvida merged 3 commits into
felizvida:mainfrom
lagostimgomes:fix/code-audit-findings-2
May 25, 2026
Merged

Fix 12 issues from second code audit#2
felizvida merged 3 commits into
felizvida:mainfrom
lagostimgomes:fix/code-audit-findings-2

Conversation

@lagostimgomes

Copy link
Copy Markdown
Contributor

Summary

Full-pass code audit follow-up covering safety, correctness, and clarity issues across 9 files.

Safety / correctness

  • notebook_search_service.dart — add a 120 s timeout to the OpenAI response body read; previously only the header exchange was time-bounded, leaving the body read able to hang indefinitely
  • backup_models.dart — replace hard casts (as String, as int) in BackupRecord.fromJson with null-safe fallbacks (as String? ?? '', as int? ?? 0) to prevent TypeError on malformed ledger entries
  • backup_parser.dart — validate the table argument against an explicit allowlist before interpolating it into a PRAGMA string; all current call sites use hard-coded names so no behaviour change, but this closes the injection path for future callers
  • backup_service.dart (_csv) — strip CR and LF characters before quoting so that multi-line values don't corrupt CSV row boundaries
  • backup_service.dart (_setOwnerOnlyPermissions) — log a stderr warning when chmod 600 exits non-zero or throws instead of swallowing the failure silently
  • readable_notebook_exporter.dart (_splitText) — return const [] (not const ['']) for empty input; the empty-string chunk would generate a search index entry with no content

Progress / UX

  • backup_service.dart (download progress) — emit a progress event on the first chunk so small notebooks (< 25 MB) are no longer silently downloaded with no feedback

Dead code / clarity

  • archive_extractor.dart — remove the redundant startsWith('//') check; any path starting with // already matches startsWith('/')
  • scripts/labarchives_auth_flow.py — replace the deprecated hmac.new() with hmac.digest() (removed in Python 3.13)
  • secure_secret_store.dart — add a comment explaining why MacOSKeychainReadOnlySecretStore exposes delete() despite its "read-only" name
  • labarchives_client.dart — document that SHA-1 is required by the LabArchives API signature scheme, not a discretionary choice
  • backup_service.dart (constructor) — add a comment explaining when DisabledSecretStore is selected over the Keychain store

Test plan

  • flutter analyze passes with no new warnings
  • Search a backed-up notebook — AI response arrives and streaming timeout fires correctly on slow connections
  • Trigger a backup of a small notebook (< 25 MB) — confirm a download-started progress message appears
  • Corrupt a backup ledger entry (remove a required field) — confirm BackupRecord.fromJson returns a default rather than throwing
  • Run scripts/labarchives_auth_flow.py --help under Python 3.13 — no deprecation warning

🤖 Generated with Claude Code

lagostimgomes and others added 3 commits May 25, 2026 17:24
- notebook_search_service: add 120s timeout to OpenAI response body read
- backup_models: null-safe casts in BackupRecord.fromJson to avoid TypeError
- backup_parser: validate table name against allowlist before PRAGMA interpolation
- backup_service: sanitize CR/LF in _csv(); emit first-chunk download progress;
  document why DisabledSecretStore is chosen when a custom root is supplied;
  log a warning instead of silently swallowing chmod failures
- readable_notebook_exporter: return [] (not ['']) for empty _splitText input
- archive_extractor: remove dead startsWith('//') branch (covered by '/')
- labarchives_auth_flow: replace deprecated hmac.new() with hmac.digest()
- secure_secret_store: clarify why MacOSKeychainReadOnlySecretStore has delete()
- labarchives_client: document that SHA-1 is mandated by the LabArchives API

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@felizvida felizvida merged commit 1a43def into felizvida:main May 25, 2026
3 checks passed
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.

2 participants