No actionable comments were generated in the recent review. 🎉

sequenceDiagram
  participant CLI as lint command
  participant Analysis as checkForIssues
  participant Governance as runProjectGovernanceChecks
  participant SQL as checkSqlQuery
  participant Secrets as findSecrets
  CLI->>Analysis: notebook blocks and governance option
  Analysis->>Governance: collected blocks and block map
  Governance->>SQL: SQL block content
  SQL-->>Governance: SQL findings
  Governance->>Secrets: eligible block content
  Secrets-->>Governance: credential findings with fingerprints
  Governance-->>Analysis: findings and project summary
  Analysis-->>CLI: lint result
  CLI-->>CLI: print findings and scan scope

Merge Risk: ⚪ Minimal · up to ac9e3

This adds an opt-in lint mode for SQL and credential checks, and the default behavior is unchanged. No outstanding merge-blocking risk was identified in the supplied context.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

Codecov Report

❌ Patch coverage is 99.51456% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.36%. Comparing base (aa891df) to head (ac9e3de).

@@            Coverage Diff             @@
##             main     #555      +/-   ##
==========================================
+ Coverage   90.04%   90.36%   +0.31%     
==========================================
  Files         213      218       +5     
  Lines       12499    12911     +412     
  Branches     3608     3744     +136     
==========================================
+ Hits        11255    11667     +412     
  Misses       1241     1241              
  Partials        3        3              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Audit - Production / Audit - All: pre-existing on main, out of scope here

Checked before touching anything, per the usual rule that a red security job is guilty until proven innocent.

This stack changes no dependency files:

$ git diff --name-only main..feat/governance-dependency-sbom -- '**/package.json' 'package.json' 'pnpm-lock.yaml'
(empty)

main fails identically today:

$ git checkout main && pnpm audit --audit-level high --prod
critical  proxy-addr  <2.0.8           via packages__mcp>@modelcontextprotocol/sdk>express>proxy-addr   GHSA-jqcg-44mw-7w3h
high      @modelcontextprotocol/sdk  >=1.12.0 <1.31.0                                                   GHSA-6qxp-vccf-f47h
2 vulnerabilities found

Both reach the tree through @modelcontextprotocol/sdk in packages/mcp — nothing this stack imports or touches.

main's last CI run was 2026-10-05 and was green; these two advisories were published after it. So "main is green" and "the audit fails on main" are both true — the job is green on main only because nothing has re-run there since the advisories landed. Any PR opened now goes red on it, regardless of content.

Fixing it means bumping @modelcontextprotocol/sdk to >=1.31.0 and overriding proxy-addr to >=2.0.8, which is a dependency change in packages/mcp with its own test surface. That belongs in its own PR — same shape as #551 — not smuggled into a CLI governance stack where it would be invisible to review. Leaving it.

🤖 Generated with Claude Code

Suggested merge strategy

Six stacked drafts, one author, no human review yet. That is not landable as a unit — the diff is ~8k lines across a lint gate, a workspace command, a DSAR index, a scoring model, a consensus check and an SBOM, and reviewing it in one pass means reviewing none of it properly.

Recommended order:

1. Take feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555 and feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556 to ready-for-review now, and get a human on them. They are the foundation everything else imports — the scanner, the secret/subject redaction, the workspace loader — and they are also where the bugs found so far have clustered: the unreadable-workspace silent pass, the redaction span corruption, the null-safe operator inversion. Those two are self-contained and each is useful alone (lint --governance is a CI gate; audit is the inventory).
2. Leave feat(cli): deepnote subjects — fingerprinted subject index and DSAR lookup #557–feat(cli): dependency inventory — an SBOM, and what will not install the same twice #560 in draft until feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555/feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556 land. They rebase cleanly on top and have no value to a reviewer until the base is settled — every review comment on them risks being invalidated by a change underneath.
3. feat(cli): SQL divergence — integration-scoped, measurable, with optional local-model triage #559 and feat(cli): dependency inventory — an SBOM, and what will not install the same twice #560 should stay draft longer than feat(cli): deepnote subjects — fingerprinted subject index and DSAR lookup #557/feat(cli): rank governance findings by liveness-weighted severity #558. feat(cli): SQL divergence — integration-scoped, measurable, with optional local-model triage #559 now carries a model-triage layer and a precision-measurement workflow that did not exist when it was opened; it wants its own review pass rather than being waved through behind the others. feat(cli): dependency inventory — an SBOM, and what will not install the same twice #560 depends on feat(cli): SQL divergence — integration-scoped, measurable, with optional local-model triage #559's rebase.

One thing a reviewer of #555/#556 should know: the per-kind weights in the divergence ranking (#559) are unmeasured starting points, and --export-review / --import-review exists so they can be replaced with a measurement. Nothing in #555 or #556 depends on them.

🤖 Generated with Claude Code

@coderabbitai review

Audit jobs: resolved upstream, stack rebased

#561 landed the @modelcontextprotocol/sdk / proxy-addr bumps on main — the separate dependency PR I pointed at above. The whole stack is now rebased onto main so every branch picks it up, not just this one (the others are based on each other's branches, so they would not have inherited it otherwise).

$ pnpm audit --audit-level high --prod
No known vulnerabilities found

Nothing else in the rebase: main's change is confined to package.json and the lockfile, which this stack does not touch, and all six rebased without a conflict.

🤖 Generated with Claude Code

Stack integrity note, since it affects what every reviewer here is looking at.

Until this push, origin/feat/governance-subject-index did not contain origin/feat/governance-audit-workspace's tip commit. The last force-push updated #556 and #557 without re-cascading, so #557–#560 were all missing #556's final commit, and each PR's diff against its base showed a revert of it. Every base→head link is now a real ancestor, verified branch by branch.

That is also why the CHANGES_REQUESTED labels on #556–#560 are stale: the reviews predate this push. I re-checked every round-1 item against the current tree and they are all still fixed. Reviews re-requested on all six.

Merge strategy — unchanged recommendation, one update. Six stacked drafts with one author is not landable as a unit. Take #555 and #556 to ready-for-review first and get a human on them; hold #557–#560 in draft until the base lands.

#559 should stay draft longest. The triage layer has now had two bugs found in it after it was written — ids that did not line up with the cache, and an export path that corrupted -o json — and both were in the plumbing around the measurement rather than in the model-facing part. The precision-measurement loop it exists to enable has still never been run end to end by a person. That is the thing that should gate it, not CI.

One finding against this PR that I need to correct from my own earlier summary.

I previously reported that the block-label credential leak was fixed on #555. It is not — the redaction lands on #556. On this branch alone:

runProjectGovernanceChecks([{ id: 'b1', type: 'code', content: 'AWS_KEY = "AKIAIOSFODNN7EXAMPLE"' }], map)
// credential-hardcoded | blockLabel: "AWS_KEY = \"AKIAIOSFODNN7EXAMPLE\""

details carries the fingerprint, deliberately, and blockLabel prints the key next to it. deepnote lint --governance -o json is the CI surface, so that lands in build logs.

I have not fixed it here, because the fix is a PR-boundary decision that is yours. SecretFinding deliberately never retains the matched secret — only a fingerprint — so there is no way to redact a label on this branch without #556's span-returning scanner (scanSecrets → redactSecrets). The options:

1. Move scanSecrets/redactSecrets down to feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555. Correct — it belongs with the secrets module and this is the first branch that needs it — but it relocates a chunk of feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556's commit and means re-cascading four branches.
2. Leave it, and do not take feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555 to ready-for-review alone. The leak exists only in the window where feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555 is merged without feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556.

Given the merge plan is "#555 and #556 first", option 2 is survivable if they go together, and option 1 is right if #555 might land on its own. I would do option 1 on your say-so; it is maybe thirty minutes including the re-cascade.

Related, and also worth knowing before you choose: the same label path has a truncation-order bug, which I did fix on #557 where that code lives. getBlockLabel truncates to 50 characters before anything is redacted, so an identifier straddling the cut survives as a prefix matching no pattern — jane.doe.reporting@acme-corp.io became jane.doe.report…, a person's name. Whichever option you pick for #555, its label needs the same treatment: redact the content, then label it.

The credential check was printing the credential. Fixed here, where it belongs.

runProjectGovernanceChecks set blockLabel: info.label on every finding. That label comes from getBlockLabel → getCodeBlockLabel, which returns the block's first non-empty line truncated to 50 characters — so for the one-line TOKEN = "…" assignment this check most often fires on, the label is the secret. The finding reported a fingerprint and the plaintext side by side, and lint -o json puts that in CI logs.

Reproduced before fixing:

$ deepnote lint leak.deepnote --governance -o json | jq -r '.issues[] | select(.code=="credential-hardcoded") | .blockLabel'
SEGMENT_WRITE_KEY = "AKIAIOSFODNN7EXAMPLE"

Why SecretFinding retaining no secret did not prevent this. The credential did not arrive through the credential code. It arrived through blockLabel — a field that predates governance and was never a secrecy boundary. Anything added to index.ts later that reaches for info.label has the same problem, which is why the decision is made once per block rather than per check: a SQL block can hold both a broken predicate and a connection string, and the SQL finding carries the same label field.

Blocks found to hold a credential are labelled by identity — code (b1c2d3e4), the same shape getBlockLabel already uses as its own fallback. It derives from nothing the author typed, so it needs no masking, and it needs no span scanner. getBlockLabel itself is untouched; other commands depend on it.

redactSecrets and the span-tracking refactor stay on #556. There, masking in place supersedes this stopgap: the label becomes TOKEN = "<redacted>" and the finding stays locatable, which this cannot do.

The regression test asserts against the whole serialized LintIssue — every contiguous run of the secret 8 characters or longer must be absent from every field — rather than against named fields, so the next field added here cannot reintroduce it.

One thing deliberately left alone. details.snippet from sql-checks is raw block text. Today those snippets span only a comparison and a column name (u.deleted_at = NULL), so an arbitrary literal cannot be inside one, and #557 redacts them anyway. Flagging it rather than leaving it unremarked.

Also still true, and outside governance: the pre-existing unused-variable check prints the same content-derived label, so SEGMENT_WRITE_KEY = "AKIA…" still appears in its finding. Fixing that means changing getBlockLabel for every command that uses it — a separate decision, not one to make inside a governance PR.

Coverage on this branch is 100% lines/statements for the governance modules.

@coderabbitai full review

Merge posture.

#555 and #556 are out of draft and need a human reviewer. #557–#560 stay in draft until #555 lands — nothing in the stack merges until it does, and six stacked branches from one author with only bot review is not landable as a unit.

Suggested order:

1. feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555 — the CI gate. Self-contained: a SQL scanner, four checks, no new dependencies, no network. Reviewing it needs no context from the rest of the stack.
2. feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556 — the demo, and the first branch that reads a whole workspace. It also carries the span scanner that supersedes feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555's label stopgap, so reviewing the two together is cheaper than reviewing either twice.
3. Rebase feat(cli): deepnote subjects — fingerprinted subject index and DSAR lookup #557–feat(cli): dependency inventory — an SBOM, and what will not install the same twice #560 onto main as feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555 and feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556 land, and take them out of draft one at a time.

#559 should stay in draft longest. Its triage layer wants its own pass: it is the only part of the stack that can send workspace content anywhere, and the guarantees that make that acceptable — no default endpoint, redaction before the payload is built, caching, fail-soft — are each a test rather than an argument, but they have had no human read them. Its approval, when it comes, is a different kind of approval from the others'.

Note that #555's APPROVED/CHANGES_REQUESTED state has been stale all round: the labels predate several force-pushes. Worth re-reading rather than trusting the badge.

Correction to my previous comment, and a second leak closed.

I wrote above that details.snippet was safe to leave alone, because "those snippets span only a comparison and a column name, so an arbitrary literal cannot be inside one". That is wrong, and the review caught it. A literal which is itself an operand of the flagged comparison is inside the span:

$ deepnote lint q.deepnote --governance -o json | jq -r '.issues[0].details.snippet'
'AKIAIOSFODNN7EXAMPLE' = NULL

Both shapes are things people write — a placeholder key compared the wrong way, a DSN tested for presence with dsn = 'postgres://u:…@h/db' = NULL. The narrow span is what makes this rare. It is not what makes it safe, and I had been treating the two as the same claim.

Fixing details.snippet alone would have moved the leak one field over rather than closed it: the finding's message quotes the snippet verbatim, so the credential sat in the sentence beside the redacted field. Message and evidence are now handled together.

On #555 the snippet is withheld and the message keeps its shape — Comparison "<redacted>" is never true … — because findSecrets can say a credential is present but not where it sits. On #556, where the span scanner exists, this is superseded by masking in place: '<redacted>' = NULL, so the comparison's shape survives. On #557 redactSubjects joins it, covering WHERE 'jane@acme.io' = NULL.

Why the first test did not catch it. My regression test asserted against the whole serialized issue, as asked — but only for credential-hardcoded findings. The SQL findings on the same block went unchecked. The new tests assert across every finding on the block.

And a stale comment on #557. It claimed the snippet redaction there was "defense in depth rather than a fix for a live leak", with a test apparently confirming it. The test put the address in a different comparison from the flagged one, so it was pinning the narrow span rather than testing the claim. Both the comment and the test note now say what the span does and does not buy.

The PR body's note on details.snippet is corrected accordingly.

@coderabbitai full review

Note for whoever picks this up: force-pushing this branch puts the PR back into draft (something in the repo automation does it). I have re-marked #555 and #556 ready — if you see them drafted again after a push, that is why, not a deliberate change.

@coderabbitai full review

Valid, and broader than reported — fixed.

Confirmed: details.columnName (then details.column) carries the column name as written, so a quoted identifier puts arbitrary block text in the JSON report. safeSqlFinding cleared the snippet and the call site then spread ...finding.details back over it. Text output looked clean because the renderer prints only the message, so this was specifically a JSON exposure — as you said.

This is the third field to leak here: block label, snippet, now the details. Each was found separately because each was reasoned about separately, and they have one cause — every one of them is derived from block text, and block text is where credentials are.

So the fix stops naming the fields it protects. It walks the finished details object, so a key added to a check tomorrow is covered without anyone remembering to come back here. The message keeps a guard as well as its snippet substitution, for a template that interpolates some other operand.

While verifying this I found a second defect in the same object: details.column was two different things. The lint position (a number) for sql-null-comparison, and the column name (a string) for the other two, because the check's own column key overwrote the position when the objects were merged. A consumer reading details.column got a number or a string depending on which check fired. The name is now details.columnName.

Both have regression tests checked against the unfixed code.

@coderabbitai full review

Valid, and the per-field rescan was the wrong shape — fixed.

Reproduced exactly as described:

-- dsn: postgres://admin:hunter2…@warehouse/db
SELECT * FROM t WHERE hunter2… = 'true'

details: { "columnName": "hunter2…", ... }   # beside fingerprint de7f0988fcfcf8e8 of the same secret

The underlying mistake was mine and it is the one you named: re-scanning each field on its own asks a different question from the one the block scan already answered. Half the provider patterns need surrounding context to fire — a URI password is recognizable in postgres://admin:…@host/db and an ordinary identifier outside it.

On #555 the block's own scan result now gates the whole finding: in a block holding a credential, the SQL finding keeps code, line and column and withholds the text it would otherwise quote. findSecrets reports presence, not position, so there is nothing more precise available here — this is the same trade the block label already makes.

Withholding everything in such a block would be safe and useless, so the checks now declare which of their details keys hold verbatim block text and which they chose from a closed vocabulary. operator (=), suggestion (IS NULL) and the integration id survive; columnName, literal and the snippet do not. A key no check declares is treated as verbatim — forgetting to list one fails towards withholding rather than publishing, which is the direction the previous three leaks here all failed in.

On #556 this is superseded properly. redactSecretsWithContext(text, context) masks, in a derived string, every credential the block is known to contain as well as anything the string reveals alone — so the evidence keeps its shape (<redacted> = 'true') and a clean finding in the same block keeps its snippet. The secret values never leave secrets.ts: the caller passes context in rather than getting values out. The block label and #560's dependency labels go through the same function, for the same reason.

@coderabbitai full review

Merge posture — a recommendation, not an action

All six PRs are still in draft and I have not changed that. Recording the recommendation here rather than enacting it, since it is a call about who reviews what.

The stack is six stacked drafts from a single author with only bot review, and nothing in it merges until this PR does. That is the whole constraint. #556–#560 each sit on the one below, so the base moving under them is a rebase for every branch above; the later a base changes, the more of that cost is paid.

#555 and #556 are the two that most need a human, and for different reasons:

• feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555 is the gate. deepnote lint --governance is advertised as a pre-publish CI check, which means its output lands in build logs. It is also where the credential checks and the secret-aware block labelling live — the leak this round fixed reached the logs of a workflow this stack itself creates. Whether these checks are right is a judgement about what a CI gate should fail on, and that is not a judgement a bot should be the only one to make.
• feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556 carries the redaction boundary. One function now decides what every section of every audit report is allowed to print, and four sections added above it inherit that decision without restating it. If the boundary is wrong, it is wrong everywhere at once and the branches above it have no second line of defence. It is also the branch where the structural choice between a boundary pass and a branded Redacted<string> was made, with its limit written down — that trade deserves a second opinion before feat(cli): deepnote subjects — fingerprinted subject index and DSAR lookup #557–feat(cli): dependency inventory — an SBOM, and what will not install the same twice #560 are built further on top of it.

#557–#560 are better reviewed once their base has settled. They are not less important — #557 is the regulatory surface and #559 is the largest change in the stack — but reviewing them against a base that is still moving spends attention twice.

What I would suggest:

1. A human reviews feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555 and feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556. Take them out of draft when someone is actually ready to look, not before — a draft that has been ready-for-review for a week reads as noise.
2. Land feat(cli): governance checks for lint — SQL correctness and hardcoded credentials #555, then feat(cli): deepnote audit — workspace integration inventory, data flow map, and governance findings #556.
3. Rebase feat(cli): deepnote subjects — fingerprinted subject index and DSAR lookup #557–feat(cli): dependency inventory — an SBOM, and what will not install the same twice #560 and review them in order, with feat(cli): SQL divergence — integration-scoped, measurable, with optional local-model triage #559 last: it is the biggest, and the triage layer and the precision-measurement loop in it both want their own pass rather than a share of one.

Two pre-existing gaps are now filed rather than carried as PR prose, since they outlive this work: #570 (projects containing file-input blocks cannot be exported, so workspace-wide checks never see them) and #571 (.deepnote files using YAML aliases fail to parse). Neither blocks the stack; both silently shrink the corpus every workspace check runs on.