Repository navigation
chore(train): merge train 2026-10-12 #7b — 10 approved PRs incl. audit_logs SoR, SSO code exchange, sandbox hardening (#17225) - #18496
Conversation
…d workspace Docker clients (#17267)
…w-privileges and a pids limit from the shared hardening builder (#17267) SECURITY-SENSITIVE: container runtime privileges; review on Opus. Owner decision (c) on #17267: the non-behavioural subset only. Changed: - container_hardening: new baseline_hardening_kwargs() (security_opt no-new-privileges, cap_drop ALL, pids_limit); hardening_kwargs() now composes it, output byte-identical. CONTAINER_PIDS_LIMIT (env AUTOBOT_WORKSPACE_PIDS_LIMIT, default 512) moves here; the workspace constant aliases it. - docker_backend.py: both containers.run sites (cold start, snapshot restore) add **baseline_hardening_kwargs(). - container_pool.py: its only caller passes no container kwargs, so pooled containers were unhardened; the same subset is applied, and the floor wins over caller kwargs. - Guard extended: each docker_backend containers.run must spread the builder; pool and backend are consumers; contrast cases added. - Pin tests assert the full kwargs of every run site; they pass on the parent with the old kwargs and now differ by exactly the three keys. Unchanged: network, read_only, mounts/volumes, memory, cpus, user, environment. Refs #17267 — option (b) full hardening stays open
…no network and a read-only root unless a named opt-in says otherwise (#17267)
…that widen their reach (privileged, devices, host namespaces, docker socket or host-root binds) (#17267)
… the tree, not only the execution backend (#17267)
… sensitive host root, or is any socket (#17267)
…e_driver join the refused run kwargs (#17267)
…de the builder spread fails the guard (#17267)
… run and create sites (#17267)
…cker-py run kwarg and every cold start raised TypeError (#17267)
…reading as an empty pool (#17267)
…bsent, and a guard keeps it declared for CI (#17267)
…led; main's entries plus this branch's one scope (#17267)
…on decisions, pin the writer set (#17225)
…, adapt audit_logger, route SecurityLayer, one retention table (#17225) Owner decision 2026-10-10, D2/D3/D5/D6: - D2: services/audit/audit_log.py, services/event_log.py and knowledge/audit_log.py are deleted; their vocabularies move to services/audit/taxonomy.py and every caller uses the canonical service (voice_bundle_admin and auth via emit_compliance; the knowledge audit API via services/audit/knowledge_view.py, which ends the double write). Old keys are left to expire by their TTL; no stored data is touched. - D3: services/audit_logger.py is an adapter: an AuditEntry is recorded as a SECURITY AuditEvent and read back from the canonical trail. - D5: services/audit/retention.py is the one retention table; projection TTLs and the one purge path (services/audit/purge.py) read it. The nightly task no longer trims every audit:* zset at one period. - D6: SecurityLayer.audit_log records through emit_security; the JSONL file is no longer written or read. Fixes found on the way: audit.emit was shadowed by an EventType bridge, so emit(AuditEvent(...)) recorded a COMPLIANCE event named after the event's repr (7 producers); a producer on a thread with no loop now hands its write to the serving loop; reads scan newest-first; an unreadable trail raises instead of reading as empty (#13258). The writer guard detects audit ORM rows, INSERT INTO ...audit and file writes in modules binding an audit path, which found five stores the first inventory missed. D1 (which Postgres table) and D4 (daemon key move) are open; see docs/developer/AUDIT_SYSTEMS.md.
…its projection (#17225) Owner decision 2026-10-11 on D1: extend the existing Postgres audit_logs table rather than add audit_event. - Additive schema: category (NOT NULL, server default 'security'), actor, session_id, resource_ref and two indexes. Alembic 20261011_098 on the backend; add_audit_logs_canonical_columns on the SLM's slm_users database, which maps the same shared model. No existing row is rewritten; the pre-existing twelve columns are pinned unchanged. - One row constructor, autobot_shared/user_management/audit_rows.audit_row, used by the canonical service and by the shared RBAC denial, organisation and team writers, so both processes write one schema. The backend and SLM user_service.py still construct AuditLog directly; they move onto audit_row after #18267, which rewrites both files. - services/audit/store.py: record() writes the row, then the Redis projection; every query is one SQL statement on audit_logs; rebuild_projection() reconstructs the projection. store_authority declares audit_trail POSTGRES with a REDIS projection. - .secrets.baseline: drop the entries for the two deleted legacy modules.
…ontrol plane map has an audit-trail row (#17225) with audit_rows.audit_row, so every audit_logs row has one constructor. The writer guard drops the two user_service allowances and fails on any AuditLog(...) outside the row builder (the SLM's own node-audit model excepted). - CONTROL_PLANE_MAP.md: row 18, Audit trail, Enforced. - AUDIT_SYSTEMS.md: D1 done; rows 5 and 10-12 point at #18284, #18285 and #18286. - symbol_fork_baseline.json: drop KnowledgeAuditLog, audit_record, emit and query_events, which are no longer fork clusters. - audit_rows_test records the mapping without configuring the shared mapper registry, which needs each backend's concrete Organization.
…ound the orphan lock, lift the request cap off the background scan (#18013) _run_duplicate_analysis signalled 'lock held' and 'deadline expired' with the same None, so every caller reported 'timed out after 120s' - including a request that returned in 0.2s because a scan was already running, with advice (raise min_similarity) that cannot help. They are now ScanBusy (carrying the holder's age) and ScanTimedOut (carrying the deadline that actually elapsed), each with its own payload; report.py logs which. The deferred lock release (#13602) relied on the orphaned executor thread exiting. A thread that never exits left every later call declined for the life of the process. Past AUTOBOT_DUPLICATE_ORPHAN_LEASE_SECONDS after the scan was told to stop, the lock is reclaimed; the late callback releases the lock object it captured, so it can never free one a newer scan owns. The Celery task applied the 120s request deadline, so the queued path that exists to escape it timed out the same way and reported completed. It is now bounded by AUTOBOT_DUPLICATE_BACKGROUND_TIMEOUT_SECONDS and a timeout raises instead of returning a partial result (#18067). duplicates.py leaves its grandfathered ceiling (660 -> 470): result shaping and config-duplicate helpers moved unchanged to duplicate_results.py. Refs #18013, #18067
… failure, and a lookup that threw is not 'no such source' (#17982) Celery records a RETURNED value as SUCCESS whatever it says, so the three scan tasks that return a refusal dict (import_tree, duplicate, dependency, all through _scan_root_or_refusal) surfaced as completed / progress 100 / error None. celery_result_to_status now maps a payload whose own status is 'error' to failed and carries the reason to the status endpoint; this is the one shared mapper, so all three are covered and asserted, not only the duplicate one. The duplicate task also refuses a scan that examined zero files. /duplicates/cached answered HTTP 200 for a failed scan, which is how a consumer checking only the status code rendered it as 'no duplicates'. It is 409 now with a fixed message; the task's exception text is logged and served by the status endpoint, not echoed (CWE-209). resolve_source_root swallowed every lookup failure into logger.debug and returned None, and source_storage.get_source returned None when it could not build its Redis client. A fault and an unknown id were therefore the same observable, and the refusal asserted the id was unknown. Both now raise RegistryUnavailable, logged at error with the cause chained; the default-source lookup no longer falls through to AutoBot's own tree on a registry fault. A structural guard fails if any broad handler around a source lookup swallows instead of re-raising, anywhere in the package - the #2760 extraction left a copy behind because nothing guarded more than one file. Not established: why the source id did not resolve on the deployment that produced the original report. The swallowed exception was never logged above DEBUG, so the cause is unrecoverable; what is fixed is that the next occurrence names its cause. Refs #17982
…wn tree, and config-duplicates says when it read nothing (#18067) Why the 120s timeout and the zero results. Measured on the deployment's own backend log for the registered AutoBot source: 8,377 files -> 130,257 blocks -> an LSH index of ALL 130k blocks -> 3.96M candidate pairs -> 2.54M reported 'duplicates' (2.43M of them 'low'), about 9 minutes end to end. The request deadline (120s) always fired first, so the response was an empty 'partial'. Three causes, each independent: 1. LSH was built at a hardcoded 0.5 whatever min_similarity was, so the advice in every timeout message ('try a higher min_similarity') changed the answer but not the work. 2. Half of the blocks have <= 17 distinct tokens. Token-set Jaccard on sets that small reaches 0.5 by coincidence; they flooded the candidate set. 3. 'No artificial limit on returned pairs' (#609) meant 2.5M pairs were built, held, and returned - no panel, Redis result or JSON response can carry that. Fix, in a new duplicate_lsh.py (duplicate_detector.py was grandfathered at its size ceiling and shrinks 1326 -> 1120): the fuzzy pass takes blocks with >= 20 distinct tokens (exact-hash matching is unchanged and still covers every size); the index is built LSH_RECALL_MARGIN (0.1) below min_similarity because MinHash-LSH retrieves a pair AT the threshold only ~50% of the time; MinHash is one update_batch call per block; the query loop is cancellable. The returned list is capped at AUTOBOT_DUPLICATE_MAX_RETURNED (5000) while total_count and the high/medium/low statistics still describe every pair, and `truncated` says so. Measured on this repository after the change: 8,384 files, 175s, 56,557 pairs found (25,491 high), 5,000 returned, truncated=true - on the background path, which is bounded by AUTOBOT_DUPLICATE_BACKGROUND_TIMEOUT_SECONDS and not the request deadline. The synchronous GET still exceeds 120s on this tree and now says so honestly. Also fixed: detect_duplicates_async had no min_similarity parameter, so the one caller that passed it raised TypeError on every semantic request and silently fell back to the standard scan. config-duplicates: detect_config_duplicates scanned <root>/src and <root>/backend. AutoBot has neither, so it read zero files and answered 'No configuration duplicates found!' in 0.7s. It also skipped any file whose ABSOLUTE path contained 'test'. It now scans from the root when the legacy directories are absent, prunes SKIP_DIRS, judges test files on the relative path, counts files scanned and failed (parse failures were logged at DEBUG), and the endpoint answers 422 when it read nothing. Its report states how many files the zero is about. Known-answer fixtures: a planted duplicate pair that must be found and a near-miss that must not, for the block detector and for config-duplicates. Not done here, still open on #18067: the symbol-fork detector reaching analytics, the per-detector ran/zero/failed record for the cross-file rules, and the problem-type counts. Refs #18067, #18013, #17982
…an unreadable registry on purpose (#17982) get_source and get_default_source_id now raise RegistryUnavailable where they returned None. Audited every caller (backend, LLC, tasks; SLM and scripts have none): authorization answers 503 instead of a false 404, HTTP handlers resolve the default source through shared.default_source_id (503), an app-level handler turns any remaining fault into 503 without echoing the cause, the LLC project list degrades without the source summary, and project disposal aborts rather than orphaning the clone. Celery tasks let it fail the task. Refs #17982
…e registry reach its 503 handler (#17982)
…r indexes concurrently (#17225) Review of #18343: - 20261011_098 downgrade() dropped the four new columns, deleting every audit value written into them since the upgrade -- a deletion path in the audit store (#17038). It is now a documented no-op; the columns are additive and harmless to leave. The SLM migration has no down path and says it may not grow one that drops them. - Both index builds were plain CREATE INDEX, which blocks writes to audit_logs for the whole build. Alembic now uses postgresql_concurrently=True inside autocommit_block(); the SLM runner commits the columns and builds CONCURRENTLY IF NOT EXISTS on an autocommit connection. Both drop and rebuild an INVALID index a failed concurrent build left behind, so a re-run repairs rather than skips it. - Tests: downgrade has no statements; every create/drop index call is concurrent and inside autocommit_block; the SLM build runs in autocommit with CONCURRENTLY and rebuilds an invalid leftover.
…ound the orphan lock, lift the request cap off the background scan (#18013) _run_duplicate_analysis signalled 'lock held' and 'deadline expired' with the same None, so every caller reported 'timed out after 120s' - including a request that returned in 0.2s because a scan was already running, with advice (raise min_similarity) that cannot help. They are now ScanBusy (carrying the holder's age) and ScanTimedOut (carrying the deadline that actually elapsed), each with its own payload; report.py logs which. The deferred lock release (#13602) relied on the orphaned executor thread exiting. A thread that never exits left every later call declined for the life of the process. Past AUTOBOT_DUPLICATE_ORPHAN_LEASE_SECONDS after the scan was told to stop, the lock is reclaimed; the late callback releases the lock object it captured, so it can never free one a newer scan owns. The Celery task applied the 120s request deadline, so the queued path that exists to escape it timed out the same way and reported completed. It is now bounded by AUTOBOT_DUPLICATE_BACKGROUND_TIMEOUT_SECONDS and a timeout raises instead of returning a partial result (#18067). duplicates.py leaves its grandfathered ceiling (660 -> 470): result shaping and config-duplicate helpers moved unchanged to duplicate_results.py. Refs #18013, #18067
… failure, and a lookup that threw is not 'no such source' (#17982) Celery records a RETURNED value as SUCCESS whatever it says, so the three scan tasks that return a refusal dict (import_tree, duplicate, dependency, all through _scan_root_or_refusal) surfaced as completed / progress 100 / error None. celery_result_to_status now maps a payload whose own status is 'error' to failed and carries the reason to the status endpoint; this is the one shared mapper, so all three are covered and asserted, not only the duplicate one. The duplicate task also refuses a scan that examined zero files. /duplicates/cached answered HTTP 200 for a failed scan, which is how a consumer checking only the status code rendered it as 'no duplicates'. It is 409 now with a fixed message; the task's exception text is logged and served by the status endpoint, not echoed (CWE-209). resolve_source_root swallowed every lookup failure into logger.debug and returned None, and source_storage.get_source returned None when it could not build its Redis client. A fault and an unknown id were therefore the same observable, and the refusal asserted the id was unknown. Both now raise RegistryUnavailable, logged at error with the cause chained; the default-source lookup no longer falls through to AutoBot's own tree on a registry fault. A structural guard fails if any broad handler around a source lookup swallows instead of re-raising, anywhere in the package - the #2760 extraction left a copy behind because nothing guarded more than one file. Not established: why the source id did not resolve on the deployment that produced the original report. The swallowed exception was never logged above DEBUG, so the cause is unrecoverable; what is fixed is that the next occurrence names its cause. Refs #17982
…wn tree, and config-duplicates says when it read nothing (#18067) Why the 120s timeout and the zero results. Measured on the deployment's own backend log for the registered AutoBot source: 8,377 files -> 130,257 blocks -> an LSH index of ALL 130k blocks -> 3.96M candidate pairs -> 2.54M reported 'duplicates' (2.43M of them 'low'), about 9 minutes end to end. The request deadline (120s) always fired first, so the response was an empty 'partial'. Three causes, each independent: 1. LSH was built at a hardcoded 0.5 whatever min_similarity was, so the advice in every timeout message ('try a higher min_similarity') changed the answer but not the work. 2. Half of the blocks have <= 17 distinct tokens. Token-set Jaccard on sets that small reaches 0.5 by coincidence; they flooded the candidate set. 3. 'No artificial limit on returned pairs' (#609) meant 2.5M pairs were built, held, and returned - no panel, Redis result or JSON response can carry that. Fix, in a new duplicate_lsh.py (duplicate_detector.py was grandfathered at its size ceiling and shrinks 1326 -> 1120): the fuzzy pass takes blocks with >= 20 distinct tokens (exact-hash matching is unchanged and still covers every size); the index is built LSH_RECALL_MARGIN (0.1) below min_similarity because MinHash-LSH retrieves a pair AT the threshold only ~50% of the time; MinHash is one update_batch call per block; the query loop is cancellable. The returned list is capped at AUTOBOT_DUPLICATE_MAX_RETURNED (5000) while total_count and the high/medium/low statistics still describe every pair, and `truncated` says so. Measured on this repository after the change: 8,384 files, 175s, 56,557 pairs found (25,491 high), 5,000 returned, truncated=true - on the background path, which is bounded by AUTOBOT_DUPLICATE_BACKGROUND_TIMEOUT_SECONDS and not the request deadline. The synchronous GET still exceeds 120s on this tree and now says so honestly. Also fixed: detect_duplicates_async had no min_similarity parameter, so the one caller that passed it raised TypeError on every semantic request and silently fell back to the standard scan. config-duplicates: detect_config_duplicates scanned <root>/src and <root>/backend. AutoBot has neither, so it read zero files and answered 'No configuration duplicates found!' in 0.7s. It also skipped any file whose ABSOLUTE path contained 'test'. It now scans from the root when the legacy directories are absent, prunes SKIP_DIRS, judges test files on the relative path, counts files scanned and failed (parse failures were logged at DEBUG), and the endpoint answers 422 when it read nothing. Its report states how many files the zero is about. Known-answer fixtures: a planted duplicate pair that must be found and a near-miss that must not, for the block detector and for config-duplicates. Not done here, still open on #18067: the symbol-fork detector reaching analytics, the per-detector ran/zero/failed record for the cross-file rules, and the problem-type counts. Refs #18067, #18013, #17982
…an unreadable registry on purpose (#17982) get_source and get_default_source_id now raise RegistryUnavailable where they returned None. Audited every caller (backend, LLC, tasks; SLM and scripts have none): authorization answers 503 instead of a false 404, HTTP handlers resolve the default source through shared.default_source_id (503), an app-level handler turns any remaining fault into 503 without echoing the cause, the LLC project list degrades without the source summary, and project disposal aborts rather than orphaning the clone. Celery tasks let it fail the task. Refs #17982
test_container_hardening_17267: the legitimate /home bind case uses the allowed sandbox home, not a developer-shaped /home/agent path the code-quality check refuses. hardcoded_values_baseline: drop the audit_logger.py row #18343 left matching nothing.
# Conflicts: # autobot-infrastructure/shared/scripts/utilities/init_memory_graph_redis.py # repo_tests/_reach_policy.py # repo_tests/config_router_auth_coverage_test.py # repo_tests/python_file_size_ratchet_baseline.py # repo_tests/reach_scope_claim_17844_test.py # scripts/python_file_size_known_large.py
- THREAT_MODEL.md back to its 160-line cap: the scrape-token trust boundary is folded into the compare_digest invariant; record/delivery/rotation move, verbatim, to PROMETHEUS_SCRAPE_TOKEN_ROTATION.md. - one_claim_registry: provider_degradation.py's nx replay is a state write-back, not a lock (#15022) -- allowed with its reason. - Ten reach floors re-pinned mid-window on this tree; audio-extension-allowlist at 6030, inside both the local window and CI's lower completed count.
|
Important Review skippedToo many files! This PR contains 282 files, which is 182 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters (5)
📒 Files selected for processing (282)
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
Applied verbatim from CI's generated-types.patch (run 38284429693): #18372's RUM and Prometheus docstrings plus the scrape-token header param were never regenerated on its own branch.
…d of interpolating it (#17225) Semgrep autobot-sql-string-format flagged #18343's SLM migration: cursor.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}"). name is a module constant, but SQL is never built by string interpolation here: use psycopg2.sql.Identifier, as rename_audit_logs_to_slm_node_audit_logs does (#11223), and drop the nosec. The test's fake cursor renders Composed SQL the way Postgres receives it. Semgrep: 0 findings; migration tests 2 passed.
- sso_exchange_code.py (#18399): SSO_CODE_TTL_SECONDS was a bare int(os.getenv()) cast, which env_var_bare_cast_test refuses (a malformed value raises at import); env_int_clamped(5..600), default 60 unchanged. - llc/tests/conftest.py: #18362 made llc/api/sprints.py read through get_source_or_none_if_unreadable; the llc source_storage stub only bound get_source, so test_project_repo_api failed to import. Same fake for both, on the existing line (the file is at its 832-line ceiling).
Thinking Path
Merge train #7b, under the owner decisions of 2026-10-11 and 2026-10-12 (every approved PR lands through a train). It replaces train #7 (#18401, closed):
@autobot/uisubpath alias. It is back with its owner.The branch is built on current
main(including train #6b, df211d3) and lands with a merge commit. The drift-prone guards were pre-run on the final tree: the reach declarations give 566 passed, and the one failure is local only (tomllibis missing from the local Python 3.10; CI runs 3.14).What Changed
Constituents (
#PR @ approved head):audit_logsbecomes the one system of record (tech-debt(audit): #6475 closed on three parallel audit systems — there are now six #17225). Tombstone ledgers; additive migrations.knowledge_categoriesmoved toautobot_shared./rum/metrics, a trusted-proxyget_client_ip, an ASGI body cap, and the Prometheus scrape token.autobot_shared.Train-only commits:
_reach_policy,reach_scope_claim,env_registry_security(by variable name; the shared blocks are identical), and the import union inpopulate_kb_chromadb.py.origin/main:config_router_auth_coverage_test: security(api): gate the 14 remaining ungated mutating routers — sign-in for reads, admin for shared-state writes (#16375) #18351 on main and security(api): RUM split into admin, user and bounded-open routes, and Prometheus metrics behind an SLM-issued scrape token (#16375) #18372 here each gated part of the old list, soKNOWN_UNGATEDis now empty and the max is 0;init_memory_graph_rediskeeps both imports./home/sandboxpath rather than a developer-shaped one;audit_logger.pyhardcoded-values row is dropped.compare_digestinvariant, and the record, delivery and rotation text moves verbatim todocs/developer/PROMETHEUS_SCRAPE_TOKEN_ROTATION.md.provider_degradation.py'snxreplay is allowed with its reason; it is a state write-back, not a lock.audio-extension-allowlistis pinned at 6030, a value inside both the local window and CI's lower measured count.Closes #18013
Closes #18368
Closes #17848
Closes #18345
Closes #18328
Closes #18394
Closes #15022
Closes #15028
Refs #17225
Refs #17267
Refs #18289
Refs #18067
Refs #17982
Refs #15722
Refs #16375
Refs #15026
Refs #15021
Refs #18397
Refs #12363
Refs #11682
Refs #18241
Refs #14776
Verification
Model Used
Claude Opus 5.5 (merge orchestration). Constituent reviews used Sonnet and Opus by risk tier.
🤖 Generated with Claude Code