Repository navigation
fix(log): panel round 1 for #765/#766 — keyset redaction, hashed secret set, one log-off seam - #772
Merged
Merged
Conversation
…, read the switch once Panel round 1 on #771 (F1, F2, F4, F5, F7). - F1: plan_secret_values now pairs each after/before value with its order key, resolving a traversed key through the joins, and adds it when the key's column is secret. A plain-str cursor on a SecretStr order key is *** in the log and in .sql(). - F7: one query_secret_values(py, name, plan, python_collected) serves every query terminal and its .sql() twin. - F2: the secret-value set is sql_log::SecretValues, hashed (text and bytes looked up without copying; other scalars by equality class, a float by its bits), so redaction is one lookup per bind and a value collected by both Python and the engine is held once. The mask is computed before Python::attach. - F4/F5: Python no longer reads the log switch. Every operation collects through one function, _bind_payload.secret_values_in; a write or raw statement hands the engine a lazy iterable it reads only when its one switch read says the log is on. The two _secrets_to_mask copies are gone. The FFI keyword secret_values now takes any iterable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hb9U9A7Zu9ypdHTsDvdScP
Panel F11 on #771: the ADR described a positional per-bind mask decided in Python; the shipped rule is a per-operation set of secret values matched by equality, with Rust adding the values bound to declared secret columns. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hb9U9A7Zu9ypdHTsDvdScP
This was referenced Oct 21, 2026
0x054
commented
Oct 21, 2026
0x054
left a comment
Contributor
Author
There was a problem hiding this comment.
Spec: PASS
Spec = panel findings F1, F2, F4, F5, F7, F11 (PR #771 panel report); #765/#766 bodies and the #700 plan's Deviated line for everything else.
- F1 ADDRESSED · src/operations.rs:1630 at 06bc945 ·
plan_secret_valueszips eachafter/beforevalue with its order key (1:1 is enforced byposition_condition, src/query.rs:787), resolves the table throughtable_for_path, and adds the value when the column is secret. Pinned by tests/test_statement_log.py:826 (after, before, traversed) and three parity-pin cases at tests/test_rendered_statement.py:174, 187, 200 (both dialects). They fail onbcb9931, where the plan's positions were never read. Throwaway probe on SQLite and Postgres:.after(("CUR2", 0)).first.sql()renders(***, ***, 0, 1);before(...).first()logs(***, ***, 0, 1). - F2 ADDRESSED · src/sql_log.rs:220, 354 at 06bc945 ·
SecretValuesis hashed. Text and bytes are looked up borrowed. TheSecretScalar::ofmatch overEngineBindValueis exhaustive, so a new variant is a compile error and cannot silently miss. A float is matched by its bits with-0.0folded;NaNmatchingNaNonly over-masks;NULLis never inserted. Membership agrees with the old derivedPartialEqfor every variant except that it adds NaN. String and Bytes stay distinct, as before. Both sides of the comparison go through the same conversions as before the PR: Python's values throughpython_to_engine_bind_value(str to String, bytes to Bytes), and plan literals throughliteral_secret_binds. So the hashing opens no canonicalization gap. The mask is computed beforePython::attachinobserveandrendered_parts. Pinned by the Rust testsa_value_collected_twice_is_held_onceandmembership_is_by_value_for_every_kind_of_bind, and by tests/test_statement_log.py:977. That test fails onbcb9931, which the panel measured at 20k rows: 9.99 s with the log on against 0.13 s off. - F4 ADDRESSED · tests/test_statement_log.py:888 at 06bc945 · The test spies the one collector,
_bind_payload.secret_values_in, through the module attribute that wire.py,deferred_secret_valuesand therefore models.py and raw.py all resolve. It also spies_sql_log.emit, which the engine looks up at each call. It covers save new, save fetched, bulk_create, the three raw calls, a transaction,first,count,exists,Query.update,Query.deleteandModel.delete. With the log off: no emit and no write collection. A query collects at compile time whatever the level; #769's review accepted that becauseCompiledQuery.secret_valuesis part of the.sql()artifact. The log-on run proves the spy sits on the seam the operations actually use. - F5 ADDRESSED · src/operations.rs:1413 at 06bc945 · No
isEnabledFororgetEffectiveLevelcall onferro.sqlis left in src/ferro (grepped). Everysql_log::enabledcall site is an operation entry. Writes and raw statements pass a lazy generator that the engine iterates only after its own switch read. - F7 ADDRESSED · src/operations.rs:1648 at 06bc945 ·
query_secret_valuesserves the four query entries and the fourrender_*twins. Writes go throughpayload_secret_values(:1668), andoperation_secret_valuesis gone. - F11 ADDRESSED · docs/adr/0057-sql-is-seen-through-the-statement-log-and-a-terminals-sql.md:38 at 06bc945 · Checked the amendment against the shipped rule and found it accurate on each point: the value set, the Python and Rust sources (keyset positions included), one lookup per bind before the GIL, over-redaction in the safe direction, the engine's single switch read, and query values collected at compile time.
- Interface deviations checked: (1) the
secret_valueskeyword isOption<Bound<PyAny>>iterated withtry_iteron all 14 entries;_core.pyideclaresIterable[object] = ...on the same 14, and Rust's defaultNonematches. All callers checked: builder.py:1308, 1336, 1404, 1448 and 1884 passcompiled.secret_values(a tuple) to call and render twins alike; models.py:465 and 818, and raw.py:120, 136, 151, 178, 185 and 192 passdeferred_secret_values(...). Insave()the one generator reaches exactly one FFI call on every branch, so it is never re-iterated after being exhausted. (2)secret_values_of/secret_valuesare replaced bysecret_values_in/deferred_secret_values/field_values: no remaining reference in src/ferro or tests (grepped). Both are in-repo private signatures, and the reasons given (one collector, laziness for F5) show the old signatures could not deliver the findings. Accepted.
Cannot verify: none
Quality: PASS
- Depth check:
SecretValues(new; three-method interface hiding the per-kind hashing, tested throughinsert/contains/mask),query_secret_values/payload_secret_values(one entry per terminal family, earning their keep by the deletion test: removing them re-spreads F7's eight pastes),secret_values_in/deferred_secret_values/field_values: no check fired. - Checked I-3: no
unwrap/expectadded outside#[cfg(test)];secret_binds_from_pypropagates iteration and conversion errors asPyErr.
Verify: PR head 06bc945 passed (#765 + #766 pytest files together, 211 passed with --db-backends=sqlite,postgres, Postgres live; cargo test --no-default-features --features testing 384 passed; ruff check and ty check clean; tests._docs_checks line skipped, module absent on this branch). Full suite -n auto: 4887 passed, 77 skipped, 2 xfailed.
Trial merge: not run (base unchanged)
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
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.
Part of #700 (panel round 1 on PR #771). Fixes F1, F2, F4, F5, F7 and F11 from the panel report, all in files #765 owned.
F1 (Critical): a plain keyset position on a secret key is now
***Before, with
token: SecretStrand a cursor decoded from a page token:After:
params=(***, ***, 0, ...), in the log and in.all.sql().plan_secret_valuesnow pairs eachafter/beforevalue with its order key. A traversed key (order_by(lambda note: note.account.token)) resolves through the joins the same waycollect_secret_leaf_bindsdoes. It is added when that column is secret.The other places a value binds against a secret column were checked (I-6). Predicate literals,
existsbodies and literalSETs were already covered. ASETexpression (+,-,merge) cannot target a secret column, because the compiler accepts only numeric or JSON targets.Model.delete()andgetrun through the query terminals. The Rustfetch_one/delete_recordPK entries have no Python caller.Tests:
tests/test_statement_log.py::test_a_plain_position_on_a_secret_order_key_is_redacted(after, before, traversed key) and three new parity-pin cases intests/test_rendered_statement.py:all-after-plain-position-on-a-secret-key,all-before-plain-position-on-a-secret-key,first-after-plain-position-on-a-traversed-secret-key. Each runs on both dialects.RED on
bcb9931:GREEN: the same command,
8 passed.F7: one
query_secret_valuesquery_secret_values(py, name, plan, python_collected)builds the set forfetch_filtered,count_filtered,update_filtered,delete_filteredand all fourrender_*twins. That is how F1 reached every terminal in one place. Writes usepayload_secret_values(py, name, rows, python_collected)and raw statements usepython_secret_values.operation_secret_valuesis gone. Covered by the F1 tests and the existing parity pin (15 cases × 2 dialects).F2 (Important): redaction is one hash lookup per bind
sql_log::SecretValuesreplaces theVec. Text and bytes are looked up without copying the bind. The other scalars are kept by equality class: a float by its bits, with-0.0stored as0.0. ANaNmatches aNaN, which over-masks.NULLis never a secret. A value collected by both Python (from theSecretStr) and the engine (from the secret column) is stored once. The mask is computed beforePython::attach, both inobserveand inrendered_parts.StatementContext.secret_valuesis anArc<SecretValues>, so the scope clones inwith_operation/with_subjectstay cheap.Tests:
sql_log::tests::a_value_collected_twice_is_held_onceandmembership_is_by_value_for_every_kind_of_bind.test_redacting_a_bulk_create_costs_one_lookup_per_bindbulk-creates 20k secret-bearing rows with the log on and off, and assertson < 4 * off + 1.0.RED on
bcb9931:GREEN:
2 passed. The measured times were 0.33 s on against 0.25 s off on SQLite, and 0.41 s against 0.29 s on Postgres.F4 + F5: one switch read and one collector
Python no longer reads
ferro.sql's level. Each operation's single read of the switch is the engine'soperation_log, at the FFI entry.ferro._bind_payload.secret_values_in(values). It searches lists, tuples and sets. A query's compile, a write and a raw statement each call it once.secret_values_ofandsecret_values(instance)were folded into it.deferred_secret_values(...), a lazy generator. The engine iterates it only when its switch read says the log is on. The two_secrets_to_maskcopies are deleted..sql()masks with the same values (accepted in feat: every operation's statements on the statement log, secrets redacted (#765) #769's review).Test:
test_with_the_log_off_no_operation_emits_or_collects_secretsis replaced bytest_secret_values_are_collected_once_and_only_when_needed[log-on|log-off]. It spies the one collector andemit, and runs save new, save fetched,bulk_create, the three raw calls and a save insidetransaction(). It then runsfirst,count,exists,Query.update,Query.deleteandModel.delete.("sk_live_1", b"k\x00\xff"), and records are emitted.The log-on case also shows the spy is on the seam the operations actually use.
RED on
bcb9931:AttributeError: module 'ferro._bind_payload' has no attribute 'secret_values_in', 4 failed. GREEN: 4 passed.F11: ADR-0057 amended
A dated
Amended 2026-10-21 (panel F11 on #771):paragraph sits under the redaction section. It records the value-set rule, the over-redaction of a plain value that equals a secret, the column-bound values Rust adds from the plan and payload (keyset positions included), and the engine's single switch read. The rest of the ADR is unchanged.Interfaces changed
secret_valuesonfetch_filtered,count_filtered,update_filtered,delete_filtered,save_record,update_record,save_bulk_records,raw_execute,raw_fetch_all,raw_fetch_oneand the fourrender_*twins:list[object]becameIterable[object], defaultNone/(). The engine iterates it only when it needs it._core.pyiis updated.ferro._bind_payload:secret_values_of(fields)andsecret_values(instance)are replaced bysecret_values_in(values),deferred_secret_values(values)andfield_values(instances). These are private modules.sql_log::StatementContext.secret_valuesisArc<SecretValues>.sql_log::shown_params(py, binds, mask: &[bool])now takes the mask.literal_secret_bindswrites into&mut SecretValues.CompiledQuery.secret_valuesis unchanged.No file owned by fix-764 (
src/run.rs,src/backend.rs,src/ddl_exec.rs,docs/pages/guide/connections.md,tests/test_ddl_lock_timeout.py) was touched or needs to change for these findings.Verify
Verify block corrected: no
On
06bc945, withFERRO_POSTGRES_URL=postgresql://postgres:password@localhost:5432/postgres(Postgres accepted connections; the Postgres cases ran):The
tests._docs_checksline of #766's Verify block was skipped because that module is not on this branch.cargo clippy --all-targetsandcargo fmt --checkare also clean.Exit steps
Closes, because Statement log: every operation statement, with secrets redacted #765 and Rendered statement: .sql() on every Query terminal, and the parity pin #766 are already closed. The Statement log: every operation statement, with secrets redacted #765 progress comment names this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01Hb9U9A7Zu9ypdHTsDvdScP