Repository navigation
Conversation
…3312) sanitize_dict / sanitize_list returned the raw subtree once depth exceeded max_depth, so a secret nested 12+ containers deep (a stream-json tool input with ~6 levels of its own nesting) was persisted unredacted. Past the cap the subtree is now replaced with REDACTION_PLACEHOLDER; recursion stays bounded. sanitize_json_string (both copies) and the agent's sanitize_subprocess_line also catch RecursionError from json.loads on pathologically deep input and fall back to the linear sanitize_text pass instead of raising. Tests: strict-xfail removed from test_secret_below_max_depth_is_still_redacted; new 2000-level dict/list cases and a 200k-deep JSON string case (backend edges + agent copy, incl. sanitize_subprocess_line); both test_max_depth_protection now assert the secret is absent; the property test drops its depth<=11 assume. Mutation: with the fix reverted, 6 backend tests (both S12 params, both deep-object params, deep-JSON, backend max_depth_protection) and 5 agent tests (max_depth_protection + the 4 TestDepthFailsClosed cases) went red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dded_credentials (#3323) `//tok@github.com/o/r` was prefixed with the assumed scheme, giving `https:////tok@…` whose netloc parses empty, so the token passed the check and validate_skills_library_url's shorthand branch persisted it verbatim to skill_sources.url and the audit row. reject_embedded_credentials now strips its input and applies the same `had_authority` rule as strip_url_credentials (`://` or a leading `//`). A ValueError from urlparse is caught ONLY for `//` inputs and falls back to the shared _AUTHORITY_USERINFO_RE, so `//[oops/x` does not become a new 500; every other input raises exactly as before (#3322 / D4 stays a strict xfail). Empty userinfo (`//@host`) is still not refused. Tests: D3 strict-xfail marker removed (+4 refuse cases incl. the regex fallback lane); new must-not-raise cases for `//host`, `//[oops/x`, `//@host` and the `///tok@` scope boundary. Mutation: with the fix reverted the 7 D3 refuse cases go red; with only the `//` try/except removed, D3-protocol-relative-unparseable-fallback and D3-protocol-relative-unparseable-no-userinfo go red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d as a rewrite (#3314) The #2915 sync fingerprint must reproduce what ingest stored. Two seams diverged, so an untouched agent entry read SYNC_CHANGED and every operator answer was refused 409 item_diverged: - options: the store keeps a falsy value ([], "", {}, 0, False) as NULL, but `_canonical_options` serialised it ("[]"). It now canonicalises any falsy value like None, mirroring the store's truthiness. - title / question: a JSON number or bool passed the clamp untouched and was stored verbatim (read back backend-dependently: SQLite True -> "1", PG "true"), while `_entry_content` fingerprinted it as the default. A new `_scalar_text` stores it as its text ("123", "True") in the clamp and mirrors that in `_entry_content`, so both sides match on every backend. Not covered: no number conversion before the #3243 title cap check (a numeric title is still never cap-held; scope kept narrow). `type` handling untouched (#3385). Stale rows: asks stored before this fix with a boolean title hold "1" on SQLite and stay flagged as diverged; integer-titled rows recover on the next sync. No migration. Tests: the strict xfails r88-r91 in test_ec_operator_queue_edges.py are removed and r90b (boolean title) added. With the fix absent all five go red: r88/r89 `['options'] == []`, r90/r90b `['title'] == []`, r91 `['question'] == []`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sy options and non-string titles (#3314) The Tier-2 real-SQLite property (entry -> clamp -> store -> sync read -> changed_fields == []) excluded the #3314 inputs while they were strict xfails. It now generates falsy options ([], "", {}, 0, False) and int/bool/float(nan, inf)/[]/{} titles and questions, with explicit examples for `options: []` and `title: 123`. This is the parity guard for the fingerprint mirroring the store's options truthiness. Only the null `type` exclusion (#3315/#3385) remains. With the #3314 service fix reverted the property goes red (`['title', 'question'] == []` on the explicit examples). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#3320) idempotency_service.begin() reports an in-flight claim as replay=True, in_flight=True, so post_message's replay return ran first and its 409 branch was unreachable: a same-key duplicate got 200 {"replayed": True} with no message. Check in_flight first, and use /chat's detail shape (error=request_in_progress, message, execution_id - None for rooms). TestRoomsInFlightBoundary: strict xfail removed; _drive takes `posted` so the test asserts nothing was dispatched and the detail shape. Mutation: with the router reverted, test_in_flight_duplicate_is_409_ not_a_silent_success fails (DID NOT RAISE HTTPException). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
) load_hardened_yaml let two failure classes escape its HardenedYamlError contract, so deploy_system (which catches only ManifestError/ValueError) answered an unnamed 500: - Control characters: PyYAML's Reader checks printability in __init__, and `loader(text)` sat above the try that maps YAMLError -> `{kind}_yaml_invalid`. Construction now sits inside the try, and the finally only disposes an instance that was actually built. - Deep nesting: no depth bound, so ~330 flow levels (6 KB) or 500 block levels (~125 KB), both under the byte cap, exhausted the stack. A compose_node depth gate (DEFAULT_MAX_DEPTH = 64, per-call `max_depth`) now refuses `{kind}_too_deep` before recursing. An `except RecursionError` backstop maps recursion the gate cannot see (a `<<` merge chain walked by flatten_mapping) to the same code. The agent-server copy is byte-identical (Invariant #5). Tests: removed the two #3324 strict-xfail markers in TestDocumentShape (the deep test now pins `k_too_deep`); new ent314 tests cover the 64/65 boundary, block form, per-call max_depth, the manifest codes through parse_manifest, and the merge-chain budget + backstop. Before the fix they failed on RecursionError / ReaderError (and on the missing API), and they pass after it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ot_number (#3319) slot_number is ZCARD+1 at acquire and repeats after out-of-order releases, so get_status's slots[0] (sorted by slot_number) could surface a newer slot as current_execution. SlotInfo now carries the ZSET score as a required start_score field, and get_status takes min((start_score, execution_id)). get_slot_state's order, the /capacity response, and acquisition/release/ renewal/cleanup/admission are unchanged. Tests: the strict xfail on test_r56 is removed; the new test_r56b covers sub-second ordering where every duration_seconds truncates to 0. Both went red on the unfixed code (picked 'D' instead of 'C') and are green with the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ope shape (#3325) CredentialEncryptionService.decrypt documents ValueError for a bad format, but a non-object JSON document ("[]", "null", "1", '"x"') hit data.get() and raised AttributeError, and a non-string nonce/ciphertext made b64decode raise TypeError outside the (KeyError, ValueError) arm. .credentials.enc is agent-writable, so POST /api/agents/{n}/credentials/import answered 500 with a stack trace where its ValueError arm promises 400. Reject a non-object envelope up front and widen the base64 arm to TypeError. Any input that decrypts today decrypts identically. Removes the two strict xfails (M50a-f, M50g); they went red with the markers removed and the fix absent (7 failed), green with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… "add the key" (#3325) encrypt_secret_setting wrapped the whole encrypt() call in `except ValueError` -> MissingEncryptionKeyError. A lone UTF-16 surrogate in the value makes the UTF-8 encode raise UnicodeEncodeError (a ValueError), so a configured install was told to add CREDENTIAL_ENCRYPTION_KEY and the settings routes answered 500. - secret_settings: probe the key on its own (the only key-error source), then encrypt; an UnicodeEncodeError becomes the new SecretSettingValueError (ValueError subclass, .key). It is raised outside the except block and `from None`, so neither __cause__ nor __context__ holds the UnicodeEncodeError, whose .object is the whole secret. The message names the key, never the value. - error_handlers + main.py: one app-level handler maps it to 422 {"detail"}. - routers/settings/credentials.py: pass-through arms in the three routes whose `except Exception` would turn it into a 500 (Anthropic key, GitHub PAT, PUT /slack). The routes without a try reach the handler directly. Missing/invalid key is unchanged: still MissingEncryptionKeyError, and now reported before any value check. Tests: removes the M58 strict xfail; new tests/unit/test_3325_credential_error_types.py (value absent from message, args, chain and rendered traceback; key-first order; anthropic / slack / slack-connect -> value-free 422 with no log line carrying it; main.py registers the handler). Mutation: with the anthropic pass-through arm and the main.py registration reverted, test_route_answers_a_value_free_422[anthropic] and test_main_registers_the_handler went red; restored byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e DB sink (#3385) `_clamp_ingested_item` bounded title, question, context and options but copied an agent's free-text `type` straight through. A queue-file entry could therefore store, and broadcast, a multi-MB type. - `_bounded_type`: truncate-with-marker at OPERATOR_QUEUE_TYPE_MAX = 64, strings only. A fixed constant: every platform type literal (the longest is 24 chars) and every ASK_TYPES value passes unchanged. - The clamp applies it. `_comparable_type` applies the same bound before the empty-value default, so the #2915 fingerprint does not report a clamped row as `changed` against its own raw entry. Without that, responses would be refused with 409 `item_diverged`. - DB belt: `_insert_values` raises ValueError for a type over 1 KiB (`_DB_BELT_TYPE_MAX_BYTES`), like the id/title/question/context belts, so a caller that skips the clamp is quarantined (#1525). - Reworded the `ingested` audit comment that said the clamp does not bound `type`. Stacked on feature/3314-queue-fingerprint-falsy (923b1ed). Mutation record (fix reverted on a scratch copy, then restored byte-identical): - clamp line removed: TestClampType::test_oversize_type_shortened_with_marker and ::test_one_over_cap_gets_marker go red - `_comparable_type` bound removed: test_3385_clamped_type_is_not_a_rewrite [clamped-row-vs-raw-entry] goes red - belt removed: TestDbBelt::test_belt_rejects_oversize_type goes red Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…3321) _build_claim_response composed the platform system prompt with the row's recorded timeout_seconds, and claim_next_task only clamped it to the agent timeout (#2846) afterwards. A row enqueued at 1800s under an agent now set to 600s told the model "Timeout: 1800s" and was killed at 600s. _build_claim_response now takes an optional cap and clamps with the existing _turn_limit before composing; the #2846 warning moves with it (same text). claim_next_task passes cap= and drops its after-the-fact clamp. A row with no recorded timeout now states the agent timeout in its prompt, as push does. _turn_limit, _shortened_note, the lease length and the row's recorded timeout_seconds are unchanged. Tests: T14's strict-xfail marker removed; T15 (absent -> agent cap) and T16 (under cap -> unchanged) added. T14 and T15 were red before the fix (prompt said 1800s; compose saw None), green after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The platform contract parser read only `user_invocable`, while Claude Code, the skill libraries and the agent server use `user-invocable`. A library skill marked `user-invocable: false` was therefore reported invocable by GET /api/skills/library and MCP list_skills. Read both spellings per source through the existing `_first_present` helper (hyphen first), so a `trinity:` block still wins whichever spelling either level uses and a null value masks nothing. Non-bool values still read as invocable. Wire name stays `user_invocable`. Mutation check (scratch copy): the issue's literal merged-scope form turns [trinity-underscore-beats-flat-hyphen] and [null-hyphen-does-not-mask] red; the underscore-only original turns [hyphen], [trinity-hyphen-beats-flat-underscore], [flat-hyphen-beats-underscore] and [hyphen-wins-in-trinity] red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… leaf (#3324) Review I1: the compose_node counter incremented for every node, so DEFAULT_MAX_DEPTH = 64 admitted only 63 collection levels for any document ending in a scalar. Count only mapping/sequence starts; add a leaf-bearing 64/65 boundary test for flow and block style. Both copies byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the measured figure (#3324) Review I2: measured on PyYAML 6.0.3 with the compose_node override, 4 frames per level, ~265 at depth 64, ~725 of 1000 left for the caller (a 64-level doc parses with the caller 725 frames deep). Comment only; both copies byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ict-xfail (#3323) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lative rule (#3323) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…3385) The three docs that enumerate the _clamp_ingested_item truncate-with-marker fields and the create_item DB belt now name type: 64 characters at ingest (OPERATOR_QUEUE_TYPE_MAX, with the truncation marker) and 1 KiB at the belt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…3314) merge-train: mechanical, per the merge-train note on the PR. `_scalar_text(0)` returned "0", but `create_item` stores the `or`-default ("Agent request") for a falsy title/question, and every row ingested before this fix carries that default — so the entry fingerprint "0" diverged from the row and flipped such rows to 409 `item_diverged` on the next sync. Only truthy scalars are stringified now; a 0/False title keeps displaying the default as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…#3321) merge-train: mechanical, per the merge-train note on the PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Oct 9, 2026
fix(sanitizer): the depth cap fails closed — nested secrets no longer escape redaction (#3312)
#3413
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration surface for #3413, #3415, #3416, #3419, #3417, #3418, #3420, #3414, #3421, #3422. Never merged; members merge individually once green.