Repository navigation
security: validate_ident let a name with a trailing newline through - #788
Merged
Merged
Conversation
Python's `$` also matches just before a trailing newline, so
`_SAFE_IDENT.match("orders\n")` succeeded and validate_ident returned
"orders\n" unchanged. Every DDL emitter routes identifiers through it,
and a name written into a line-based file (a Java properties file, as
in #784) gains a second line.
The four whole-string allowlists in _sql_safety.py now end in `\Z`.
Only _SAFE_IDENT changes behaviour: the type-name, type-parameter and
RLS-expression helpers already strip their input, so `\Z` there keeps
a later edit that drops the strip from reopening the gap.
tests/providers/test_sql_safety_type_param.py pins validate_ident
against "\n", "\r\n" and an embedded "\n", and each pattern against a
trailing "\n".
github-actions
Bot
added
provider
📄 Documentation ReminderThis PR appears to be missing a documentation reference. Our docs live in a separate repo. Please update the PR description with one of:
See the Contributing Guide for details. |
github-actions
Bot
added
security
1 task done
fas89
added a commit
that referenced
this pull request
Nov 3, 2026
* security: refuse a trailing newline in whole-string validators Python's `$` also matches just before a final newline, so `re.match(r"^[a-z]+$", "orders\n")` succeeds and the caller goes on to use "orders\n". #788 closed this in providers/_sql_safety.py. The same `^...$` plus `.match` shape guarded 36 more validator patterns across fluid_build/, fed by contract YAML (a `|` block scalar ends in a newline), .env values, CLI flags, LLM tool calls and percent-encoded URL queries. 31 of those patterns now end in `\Z`. Two call sites use `fullmatch` instead: the catalog quoting chokepoint, which also covers the BigQuery patterns callers pass in, and the federation workspace id, whose error message prints the pattern. Seen through the CLI before the fix, `fluid generate iac` put `region = "eu-west-1\n"` on the AWS provider, and `branch = "main\n"` on a Databricks job from `?branch=main%0A`. 81 other `$`-ending patterns stay as they are. Their input is stripped first, or they are deny lists and detectors where `\Z` would loosen the check, or they are parsers whose captured groups are what is used. tests/security/test_trailing_newline_anchors.py pins each fixed site with a value and the same value plus "\n". All 39 cases fail on the parent commit. The fix follows Django's username validators (ticket #30257, `$` to `\Z`) and the MCP Python SDK's tool-name check (#3076, `fullmatch`). * docs(changelog): 36 more validators refuse a value with a trailing newline * fix(iac): refuse an AWS binding region with whitespace around it With `\Z`, provider_block_for dropped a binding region of "eu-west-1\n" as no region code. That left the provider on the environment's region and skipped _guard_region_move. Pinning the stripped value is no better. Both sovereignty checkers (policy/sovereignty.py, providers/aws/util/sovereignty.py) compare the region as written, so "us-east-1\n" passes `deniedRegions: [us-east-1]` while the provider would be pinned to us-east-1. provider_block_for now raises UnsupportedBindingError (aws-region-whitespace) for a region code with whitespace around it. generate iac and apply render it as a typed CLI error before anything is applied, and fluid diff reports the inspector error. On main the value went onto the provider with its newline. * security: match the BigQuery catalog and imported-schema patterns whole copilot/catalog/bigquery.py::_validate_ident checked the same `$` patterns with `.match` that the quoting chokepoint now checks with `fullmatch`. So `scope.tables` values and FQN parts such as "orders\n" reached `table_name IN ('...')`. The ODCS and ODPS importers compile the bundled schema's identifier and semver patterns, which end in `$`, and also used `.match`: an imported id "orders\n" was kept verbatim, and "1.2.3\n" counted as semver. All three now call `fullmatch`. The two patterns that keep `$` because their call sites use `fullmatch` (`_DEFAULT_IDENTIFIER`, `_WORKSPACE_ID_RE`) now say so in a comment beside them. * fix: show the refused newline in AWS name and mission path messages These messages quoted the value as '{value}', so a name refused for a trailing newline printed as a valid-looking name split over two lines. They now use repr, which prints any other value exactly as before. * chore: keep the domain slug patterns in step with the profile-name guard The comment on forge_domain_enrichment._SAFE_DOMAIN_RE says it matches forge_copilot_prompts._PROFILE_NAME_RE, and the comment on _ai_setup_storage._DOMAIN_SLUG_RE says it mirrors _SAFE_DOMAIN_RE. The profile-name guard now ends in `\Z`, so both do too. Neither is reachable with a trailing newline: one runs after a disk lookup, the other on stripped input. * docs: drop the validator count and reword validate_id's summary The count in the changelog entry depended on how patterns were grouped. The entry now lists what is covered instead, including the AWS region refusal and the imports. validate_id's summary said it refused a trailing newline "as well", which the shared grammar now does itself. * test: skip the BigQuery validation pin without the GCP SDK bigquery_validation imports google.cloud.bigquery at module load. The iac Stage 1 unit lane installs no GCP extra, so the new pin failed collection there with ModuleNotFoundError. It now skips via pytest.importorskip, the guard tests/providers/test_validation_sql_injection.py already applies to the same module.
fas89
added a commit
that referenced
this pull request
Nov 3, 2026
…gelog-rules conflict (#798) * chore(ci): bump pypa/gh-action-pypi-publish from 1.14.2 to 1.14.3 (#783) Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.14.2 to 1.14.3. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@dc37677...7245906) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-version: 1.14.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(ci): bump docker/login-action from 4.6.0 to 4.7.0 (#781) Bumps [docker/login-action](https://github.com/docker/login-action) from 4.6.0 to 4.7.0. - [Release notes](https://github.com/docker/login-action/releases) - [Commits](docker/login-action@dbcb813...81c6f93) --- updated-dependencies: - dependency-name: docker/login-action dependency-version: 4.7.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat(databricks): column masks and row filters are enforced in Unity Catalog (#785) * feat(databricks): column restrictions and row filters compile to Unity Catalog SQL functions policy.authz.columnRestrictions and rowFilters were refused. They now compile to SQL functions named by a hash of their definition, with the semantics GCP and AWS give the same fields: deny is a mask that returns NULL, mask is last_four, first_four or NULL, one mask per column with a deny beating a mask, and one row filter per table that is the union of its principals' predicates for their members and every row for everyone else. A predicate is parsed as one boolean expression, checked node by node against an allowlist and rendered from the tree. access: allow, a principal SQL cannot test and a text mask on a non-string column are refused. Nothing runs them yet. * feat(databricks): fluid apply binds the masks and row filter around tofu apply, and revokes SELECT when it cannot The provider has no function resource and no mask or filter attribute, so the statements run on the SQL warehouse through the SQL Statement Execution API. pre_apply binds on the tables that exist before tofu apply runs, so a grant the apply adds lands on a protected table; post_apply binds on the tables the apply created, drops a mask or filter this module bound that the contract no longer asks for, and, for a table it cannot bind, revokes SELECT from the principals the contract restricts and exits non-zero. Live bindings are read from the Tables API and only what differs runs, so a second apply executes nothing. A dry run prints the statements and runs none. The stand-in executes the statements and serves the bindings. * feat(verify): fluid verify checks a Databricks table's row filter and column masks The contract's restrictions are Unity Catalog functions bound on the table, named by a hash of their definition. Verify compares the table's live row_filter and each column's mask, from the GetTable it already reads, with the names the contract expects: a binding that is missing, or is another function (a changed predicate is a new name), is CRITICAL and the fix is fluid apply. No restriction in the contract skips the dimension. * test(databricks): a real tofu apply binds the masks and row filter against the stand-in, and fails safe Apply binds the filter and masks and a second apply executes no statement; a table that exists is bound before the apply can grant on it; a changed predicate binds a new function and keeps the old; a removed restriction is dropped and said to widen access; a binding that fails revokes SELECT from the restricted principals only, fails the run, and the next apply binds first and grants again; a failure before the apply stops it with nothing applied. * docs(databricks): the changelog and tested-honestly rows say what is enforced, how, and what is not proven live * fix(databricks): a contract with no restriction needs no databricks-sdk at apply and says nothing without it * fix(databricks): a mask or filter tests every identity as a group and as a user, so a remapped principal is still restricted The test was chosen from the logical principal's prefix. binding.principals may map a group: principal to a service principal's application ID or a user's email, and the group test then matched no one while the grant to that identity still applied: the mask and filter never took effect. Every identity is tested as is_account_group_member(x) OR current_user() = x; a name that is both a group and a user is restricted either way, which errs closed. The prefix only refuses domain: and unprefixed principals now. * fix(databricks): a mask rule belongs to the principal that names it, and the strictest matching one applies One rules[column] kept the last restriction's rule for every masked member, so a nullify for one principal and a last_four for another gave the first the last four characters, a weaker mask than declared. Members are grouped by rule and the function has one WHEN branch per rule, strictest first: deny, nullify, first_four, last_four, so a member of several groups gets the strictest that matches. Each text rule is type-checked against the column. restrictions_for takes rule_per_principal for a platform whose mask tests each principal itself; GCP and AWS keep refusing two rules for one column. * fix(databricks): a tofu apply that fails part way still binds the masks and row filter of the tables that exist A failed apply raised before post_apply ran, so a table it created, its grants and the dbt job that fills it could exist with no mask or filter, and a scheduled run would expose unmasked rows. Before the apply error is raised, post_apply now runs best effort in a bind-only mode (after_failed_apply): it binds what exists, revokes SELECT where it cannot bind, skips a table that does not exist, and drops nothing, since dropping widens access and waits for an apply that succeeded. A failure there is printed; the run still raises opentofu_apply_failed. * docs(databricks): the changelog and tested-honestly rows describe per-identity tests, per-principal mask rules and the bind-only pass after a failed apply * fix(databricks): a function under a hashed name is read back and must be the one this module creates, before it is bound The name fluid_mask_<table>_<column>_<hash8> can be computed from the contract, so anyone with CREATE FUNCTION on the schema could create it first with a body that returns the value unmasked: CREATE FUNCTION IF NOT EXISTS kept theirs, SET MASK bound it and fluid verify, which compared names only, passed. After each create and before any bind the function is read (GET /api/2.1/unity-catalog/functions/{name}) and its owner, body, parameters and return type must be the expected ones, whitespace-normalised; the owner must be the identity the run authenticates as (userName of scim/v2/Me, who CREATE FUNCTION makes the owner). A mismatch is a failed bind, printed with what differs. fluid verify reads every bound function back and checks the same, accepting the table's owner as well since verify may run as someone else. * fix(databricks): a row filter or mask set by hand is never replaced silently SET ROW FILTER and SET MASK overwrite whatever is bound, so a filter or mask an admin set (a function without the fluid_ prefix) was lost without a word when the contract asked for its own. Every table is read and planned before any statement runs, and a live binding this module did not create, where the contract asks for one, is refused naming the table, the column and the live function, with the remedy: remove it, or move its rule into the contract. Nothing is changed on any table. After an apply it is a failed bind (the fail-safe runs). * fix(databricks): a Tables API read that fails for one table is a failed bind for it, and the other tables are still tried _read raised outside the try that triggers the fail-safe, so a transient 429 or 503 on one table aborted post_apply with that table granted and unbound and the rest never tried. Read failures are caught per table, after an apply and after a failed one: a table with restrictions is a failed bind (the fail-safe runs, the error names it) and the loop goes on; one without is a note. A read that fails before the apply still stops the run. * fix(databricks): a failed bind binds a deny-all row filter first, and only then locks the table by revoking The fail-safe revoked SELECT from the restricted principals alone, which misses a user in another granted group and SELECT inherited from the schema or catalog, and the message claimed none of them could read unmasked rows. A failed bind now binds a fixed-purpose deny-all row filter for the restricted identities (a zero-column function, RETURN NOT <any restricted identity>, content-hashed and checked for owner and body like every function): it blocks them whatever the grant path. If that cannot be bound, or would replace a row filter this module did not create, SELECT (and ALL PRIVILEGES) is revoked on the table from every grantee but its owner, listed through the permissions API. The message says exactly what was done and names each grant path still open, a schema or catalog grant a table-level REVOKE cannot close, with the statement to run. * fix(databricks): a row filter that needs a column the apply adds is held by a deny-all filter until then pre_apply skipped a filter whose columns the table does not have yet, and the table exists with rows, so the restricted principals read every row until post_apply. When the real filter cannot bind and the table exists, pre_apply now binds the interim deny-all row filter (the fail-safe's own: a zero-column function, SET ROW FILTER ... ON (), owner and body checked) for the restricted identities, and post_apply replaces it with the real one. A dry run shows it; a hostile function under its name, or a row filter this module did not create, stops the run before the apply. SET replaces a bound filter directly: a DROP first would leave a window with no filter. * fix(databricks): generate iac refuses a module a plain tofu apply would leave unmasked The masks and row filters are bound on the SQL warehouse around tofu apply, not in the module, so fluid generate iac wrote a module that creates the table and grants SELECT with none of them and told the reader to review and apply it with OpenTofu. generate iac now asks the plugin (standalone_module_refusal) before it writes one, and refuses a Databricks contract that compiles any mask or row filter as databricks-enforcement-outside-module, saying that a plain tofu apply would grant unmasked access and to use fluid apply. emit stays pure; fluid apply, plan and validate are unaffected. * fix(databricks): a dbt job that writes a table not yet protected is created paused A continuous or frequent cron job created in the same tofu apply as its restricted table and the grant on it could run and fill the table before post_apply binds the masks and row filter. DatabricksIacPlugin.emit (the apply path; the pure compile validate uses is unchanged) now reads the live bindings of each restricted table a job writes and sets pause_status to PAUSED on the job's schedule or continuous block unless the table exists, its row filter and masks are the expected functions on the expected columns, and each function is the one this module creates. An unreadable workspace counts as not protected. The next fluid apply, once they are bound, unpauses it in place, with no drift from unpausing by hand. A manual job has nothing to pause. plan_notices says why a job is created paused and that run-now works meanwhile. The engine comment that claimed binding happens before any build can fill a table is corrected. * docs(databricks): the changelog and tested-honestly rows describe the verified functions, the deny-all fail-safe, the paused jobs and what is still unproven live * chore(tests): the unused names the code-quality review found are gone An unused import of copy and Optional, two unused helper imports and an unused unpacking target in the Databricks tests; the stand-in's deploy identity is now the one constant the module itself uses. * chore(ci): bump reviewdog/action-actionlint from 1.80.6 to 1.81.1 (#782) Bumps [reviewdog/action-actionlint](https://github.com/reviewdog/action-actionlint) from 1.80.6 to 1.81.1. - [Release notes](https://github.com/reviewdog/action-actionlint/releases) - [Commits](reviewdog/action-actionlint@48e36f5...619a81e) --- updated-dependencies: - dependency-name: reviewdog/action-actionlint dependency-version: 1.81.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(ci): bump github/codeql-action init and analyze to 4.39.1 together (#745) Dependabot split the 4.39.1 bump into #745 (init) and #747 (analyze). CodeQL fails any run whose steps are on different versions, so each PR alone was red on all three Analyze jobs. This one now bumps both to the same pinned SHA (9b3254085494b55a0f7c4c2b3d2d52cc712ce370, checked against the upstream v4.39.1 tag), and dependabot.yml groups github/codeql-action* so init, analyze and upload-sarif arrive in one PR from now on. * fix: doctor --strict, an honest roadmap, and a readable, redacted OpenTofu apply failure (#786) * feat(doctor): --strict exits 1 when doctor reports "Action needed" fluid doctor printed "Action needed" for a copilot that is not ready and still exited 0, so a CI gate on doctor could not fail. With --strict it exits 1 whenever the summary says "Action needed", in the text report and with --json, whose payload now carries action_needed and copilot_ready. The default exit code is unchanged; --strict has no effect with --features-only, which prints no summary. Found by an external hands-on assessment of 0.26.0. * fix(roadmap): stop claiming a 1.0 release; name every provider in the description fluid roadmap printed "forge-cli v1.0 shipped 2026-04-23" while the package is 0.x and classified Beta. The roadmap now says so and calls its "v1" headings an internal milestone, and a test fails if it announces a 1.x release while the package is below 1.0. The package description named GCP, Snowflake and Local; it now also names AWS and Databricks. Found by an external hands-on assessment of 0.26.0. * fix(apply): a failed OpenTofu apply names its diagnostic and the way on, redacted The runner uses -json, so the error a failed apply printed was the last 800 characters of diagnostic JSON lines, with no hint. The error now reads the first error diagnostic (summary, resource address, provider detail), falls back to the first "Error:" or the exit code, and says that OpenTofu records state as it goes, so fixing the cause and re-running fluid apply normally continues; it names the workdir for `tofu state list`, and only a state-lock error points at `tofu force-unlock`. The raw tail goes to --debug. Every value a CLIError prints is now redacted and has its control characters spelled out, as the other error handlers already did: the new detail field carries provider text, which can hold a credential. Found by an external hands-on assessment of 0.26.0. * fix(cli): error values go through the full secret redactor; a locked plan says how to clear it The top-level CLIError handler ran context values through _console's narrow pattern only (bare password/token/secret/api key followed by : or =), which leaves an AKIA access key id, a Bearer token, a URL password, JSON-quoted and *_secret_key shapes and env-literal secrets in clear. Context values and suggestions now go through redact_secret_text first, the redactor the logs already use. _apply_failure redacts the diagnostic before cutting it at 800 characters, since a credential split at the limit (a URL that lost its @) matches no pattern. A stale state lock first surfaces at tofu plan, which printed only the raw tail; _plan_failure now redacts it and, for a state-lock error only, says to wait for the other run or clear it with tofu force-unlock in the named workdir. Found by the security review of this branch. * test: allowlist the deliberate fake credentials the secret scan flags detect-secrets failed Lint & Format on the AWS-example key and the admin:s3cretpw DSN these redaction tests feed in on purpose. Each value now carries `# pragma: allowlist secret` on every line that holds it, not just the first: detect-secrets reports one finding per value per file, so allowlisting only lines 690/696 and 412/413 would have surfaced the copies at 695/701 and 424 on the next run. Two lines were pre-split so the pragma stays on the value. With the comment appended, black would otherwise rewrap them and leave the pragma on a closing bracket, where detect-secrets does not look. * test: make welcome-scan-dependent tests independent of probe timing (#792) run_welcome_scan drops any probe result that misses its 0.4s/1.0s budget, and pick_mode takes its default from those results. On a loaded runner the workspace probe overran and test_pick_mode_default_picks_from_product_when_ workspace_has_products saw "ai" instead of "from_product" (PR #782, Python 3.14 coverage leg). Add a deterministic_welcome_scan fixture that lifts the budgets and points the dbt probe at "no dbt" (it sizes its `dbt --version` subprocess from the same budget), and apply it to the four modules whose tests assert on probe results. Exposure audit: with every probe result dropped, 13 tests in those four modules fail; the other test files that reach the scan do not. A new test pins the original failure with a deliberately slow workspace probe. The product budget is unchanged. * security: validate_ident let a name with a trailing newline through (#788) * security: validate_ident let a name with a trailing newline through Python's `$` also matches just before a trailing newline, so `_SAFE_IDENT.match("orders\n")` succeeded and validate_ident returned "orders\n" unchanged. Every DDL emitter routes identifiers through it, and a name written into a line-based file (a Java properties file, as in #784) gains a second line. The four whole-string allowlists in _sql_safety.py now end in `\Z`. Only _SAFE_IDENT changes behaviour: the type-name, type-parameter and RLS-expression helpers already strip their input, so `\Z` there keeps a later edit that drops the strip from reopening the gap. tests/providers/test_sql_safety_type_param.py pins validate_ident against "\n", "\r\n" and an embedded "\n", and each pattern against a trailing "\n". * docs(changelog): validate_ident refuses a name with a trailing newline * chore(secrets): move the two _sql_safety baseline hits to their new lines * ci: freeze dependency weight and startup cost before the diet (#791) * ci: freeze dependency weight and startup cost before the diet Guards at today's numbers, so nothing grows while later PRs move litellm, mcp and openlineage out of the base install. - scripts/ci/dep_budget.py and .github/workflows/dep-budget.yml resolve the base install with `uv pip compile` for linux/3.12, linux/3.10 and windows/3.12 and fail above a committed ceiling (88/86/89), naming the packages added and removed against a baseline. It is its own workflow (PRs touching pyproject, main, weekly), because an upstream release can move the count with no change here. - scripts/ci/bare_install_smoke.sh, called from ci.yml build-check and release.yml, caps the bare wheel's build_parser() import cost (869) and runs validate, plan, bundle, generate iac and doctor on an AWS example. - .importlinter-external is an import-linter forbidden contract with external packages: litellm, mcp, openlineage and boto3 stay off the core modules. The 8 lazy routes that exist today are listed one by one. - ruff TID253 bans module-level imports of the heavy SDKs, with 4 exemptions. It does not see an import inside a module-level try:, which the config says. - deptry 0.25.1 joins [dev]. CI runs it as advisory until 0.29.0. - tests/perf/test_startup_budget.py lowers MAX_MODULES from 1100 to 895 (852 on 3.12, plus 5%) and adds a discover_providers() probe capped at 916. Both counts print in the CI log. Borrowed, not built: uv, import-linter (include_external_packages, ignore_imports), ruff flake8-tidy-imports, deptry. * ci: keep the bare-install smoke out of the release build job release.yml's build job holds id-token: write and attestations: write. Running the wheel and its ~88 transitive dependencies (resolved at release time, not hash-locked) through validate, plan, bundle, generate iac and doctor there widened an exposure that `fluid init` already had. A failing doctor check could also block a tag. A tag's merge commit has already run the same smoke on the same wheel build in ci.yml build-check. So release.yml goes back to its --version and init check, and the smoke runs only in CI. * ci: the dependency budget fails loudly and ratchets Fixes from the defect hunt on the budget guard: - A uv failure (a resolver conflict or an index outage) now prints uv's own stderr. CalledProcessError's message left it out. - `name @ url` requirements are counted. Any line the parser does not understand raises, so no package slips past the budget uncounted. - --update compares against the committed baseline before rewriting it, so an over-ceiling run still names the packages added and removed. - A count under its ceiling prints a notice to lower the ceiling. - Pushes to main run the workflow only when pyproject.toml, the script, the baselines or the workflow change, as PRs already did. Upstream drift still shows in the weekly run without turning unrelated pushes red. The bare-install smoke now resolves the venv path to an absolute one, because its probe runs from /tmp, and removes its temp dir on exit. * fix: install hints name data-product-forge, not fluid-build (#790) * fix: install hints name data-product-forge, not fluid-build Doctor, the copilot, the MCP, ODCS and DuckDB errors, the generated Jenkinsfile and the agent docs told users to `pip install fluid-build[...]`. The distribution is `data-product-forge`; every hint now names it with the right extra. - Add an empty `mcp` extra so `data-product-forge[mcp]` resolves today; it is filled when mcp leaves the base install. - tests/test_no_wrong_package_name.py fails on any `fluid-build` not followed by `.` (domains are a separate fence). The allowlist matches a line, not a file (only OTel's service.namespace), and a coverage guard keeps the scan honest. Pattern borrowed from tests/test_no_reserved_tld_defaults.py. * test: catch every shape of a wrong install hint, and missing extras Follow-up from the security review: - An extras-exist check: every `data-product-forge[...]` in the scanned files must name extras that pyproject declares. It failed first on two existing hints. tracing.py now names the OTel packages it imports (opentelemetry-sdk, opentelemetry-exporter-otlp-proto-http), and the CONTRIBUTING.md example names `atlasclient`. Neither extra existed, so pip installed the base package and the feature still failed. - A sentence-final "fluid-build." no longer passes as a domain. - Install commands (pip, pip3, pipx, uv pip) fail on any fluid[-_.]build spelling, since PyPI normalises them to one name. The import package `fluid_build` stays legal everywhere else. - The scan now covers the non-.py files shipped in the wheel and CONTRIBUTING.md, and asserts that it read them. - odcs.py quotes its extra, which zsh rejected unquoted. * fix: the vowl hint keeps its extra, and the secrets baseline follows From the verifier: - `fluid odcs validate` printed the vowl hint through Rich markup, which read `[odcs-strict]` as a tag and dropped it. Users were told to install the base package, which has no vowl. It now prints with markup=False, as generate_ci already does. A test drives the real cprint. - tests/test_litellm_backend.py moved a baselined fake key from line 566 to 607. CI's detect-secrets-hook on changed files would exit 3, so .secrets.baseline is refreshed. The diff is that line number and generated_at only; no secret entry was added. * fix(diff): fluid diff --baseline sees the columns of a 0.7.x contract (#789) fluid diff --baseline read an expose's columns from exposes[].schema, but a 0.7.x contract keeps them in exposes[].contract.schema, so a removed column, a type change or a new NOT NULL column reported "No changes detected.", --fail-on-breaking exited 0, and the fluid forge --refine preview said "No changes vs the existing contract." Columns now come from contract.schema when schema is absent, and a column is NOT NULL when required is true or nullable is false. * fix(diff): fluid diff --baseline sees the columns of a 0.7.x contract * fix(diff): a column is NOT NULL when required or nullable: false says so * test(diff): reuse the shared 0.7.6 fixture and pin which spelling names a nullability finding * test(streaming): say why the live fixture poll swallows OSError Answers the code-quality review on #784 (empty except): the fixture is still starting, so the poll retries. * fix(diff): a container child's own name no longer hides its change `_diff_container_children` built `{"name": key, **child}`, so a `name` inside an element/key/value mapping overrode the slot name. A non-string name dropped the child from the diff, and a string one moved its path. The slot name now wins. Found by the resolution review; inherited from #784. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
fas89
added a commit
that referenced
this pull request
Nov 4, 2026
…atch (#816) Python's $ also matches just before a final "\n", so re.match(r"^[a-z]+$", "orders\n") succeeds. #788 and #797 fixed the known validators with \Z or fullmatch, but nothing kept the class from coming back: no ruff, pylint, bandit, dlint or Semgrep CE rule covers it (astral-sh/ruff#15017 proposes one). tests/security/test_no_dollar_anchored_validators.py walks the AST of fluid_build/ and follows the pattern of every .match / .search call back to where it is written: a literal or constant, a dict or tuple, an import, a helper's return value, a property, a re.Pattern parameter, or a pattern compiled at run time. It reads each pattern with re's own parser, so alternatives, groups, escapes and VERBOSE count the way re reads them. Calls whose $ is deliberate are listed in _ACCEPTED with the reason. An entry that no longer matches a call fails as well, so the list only shrinks. On main before #797 it fails on 44 calls, including the shapes the sweep missed: a pattern held in a dict, schema patterns compiled at run time, and a pattern the caller passes in. It also caught the $ + .match sites merged since (#793, #801, #799). Each was read and is listed with its reason.
2 tasks done
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.
What was wrong
providers/_sql_safety.py::validate_identchecks a name against^[A-Za-z_][A-Za-z0-9_]*$with.match. Python's$also matches just before a trailing newline, sovalidate_ident("orders\n")returned"orders\n"unchanged. About 40 modules route contract-supplied names through it before the names reach DDL, an OpenTofu module, or a line-based file.A security review of #784 found where this matters most. That PR writes
expected.target=<namespace>.<table>into a Java properties file, so a table name that ends in\ngets past the check and splits the line. The gap predates #784, so this PR fixes it on its own.Reproduced through the real CLI on main f322f8b. I ran
fluid generate iaconexamples/aws-redshift-kinesis-streamingwithexternal_schema: "spectrum_events\n". It exits 0, andmain.tf.jsoncontainsCREATE EXTERNAL SCHEMA IF NOT EXISTS spectrum_events\n ....What changes
The four whole-string allowlists in
_sql_safety.pynow end in\Zinstead of$:_SAFE_IDENT_SAFE_EXPR_CHARS_SQL_TYPE_PARAM_PAYLOAD_SQL_TYPE_NAME\Zmatches only at the very end of the string, so the change can only tighten them. Call sites keep.match.Only
_SAFE_IDENTchanges behaviour.validate_sql_type_name,validate_sql_type_param_payloadandvalidate_sql_expression_allowlistalready.strip()their input before matching. For them,\Zstops a later edit that drops the strip from reopening the gap._SAFE_EXPR_CHARSadmits\sby design, newlines included. It changes only for consistency.With the fix, the same
fluid generate iacrun exits 1 withInvalid SQL identifier: 'spectrum_events\n'and writes no module.Behaviour change: a name that reaches
validate_identfrom an environment variable, or from a file that ends in a newline, is now refused too. Before, it went through with the newline. Two of those paths are{{ env.NAME }}values (build_runners/base.py:76, read withos.getenvand not stripped) and the dbt runner'senv_var('NAME')catalog/schema (build_runners/dbt/profiles.py:221). The error names the field. The fix is to strip the value where it is set.CHANGELOG.md: a### Securityentry..secrets.baseline: the twoSecret Keywordhits in_sql_safety.py(the DSNpassword=lines) sit four lines lower now, at 248/249 instead of 244/245. I regenerated the baseline with detect-secrets 1.5.0, the version CI uses.Borrow before build
\zas a synonym, but the package supports 3.10+, so this uses\Z. The alternative wasre.fullmatchat each call site. Anchoring the pattern itself also covers any.matchadded to these patterns later.re.fullmatch(modelcontextprotocol/python-sdk#3086, merged 2026-07-17). Seth Larson's write-up covers how$,\Zand\zbehave across regex engines.py/regex/unmatchable-dollarchecks something different.The same
^...$+.matchshape also appears outside this module, for example:providers/quality_engine.py:44exporters/sodacl.py:85copilot/catalog/_patterns.py:103cli/discover/_jdbc_introspect.py:544providers/_path_safety.py:49A sweep of those follows in a separate PR.
Validation
tests/providers/test_sql_safety_type_param.pygains 6 cases. 4 fail on main and pass here:validate_ident("orders\n")_SAFE_IDENT,_SQL_TYPE_NAMEand_SQL_TYPE_PARAM_PAYLOADmatched against its value plus a trailing\n"Main" here is b2d05c0;
_sql_safety.pyis unchanged from there through f322f8b. The other 2 cases,"orders\r\n"and"ord\ners", already raise on main. They stay as pins, so a later fix that strips the input or matches line by line fails them.Live run:
fluid generate iacas above exits 1 on the branch and 0 on main. The unmodified example emits a byte-identicalmain.tf.jsonon both.Whole-tree checks are clean:
ruff check fluid_build/ tests/,black==24.10.0 --check fluid_build/ tests/,scripts/check_license_headers.py,lint-imports(6 contracts kept) and the product guardrail.Tests:
_sql_safety, plustests/perf/test_startup_budget.py: 1215 passed.tests/providers tests/iac tests/output_ports tests/build_runners tests/engines tests/security tests/copilot: 9860 passed.--helpwall-clock backstop: 4.37 s on the branch, 4.50 s on main, against 2.0 stest_duckdb_sandbox_workspace_root.pytest_credentials_stay_out_of_files_and_diagnostics.pyReviews scoped to the diff:
str.splitlines()line break is refused in trailing, leading and embedded positions.[A-Za-z0-9_]characters.validate_sql_type_param_payloadcheckspayload.strip()but returns the rawpayload, and its\s*admits a newline around the comma. Nothing exploits it, because both callers (snowflake/util/types.py:132,iac/providers/databricks.py:355) discard the return value.Documentation
Invalid SQL identifier: '<name>\n'. The CHANGELOG Security entry records it.