Skip to content

[auto-bump] [no-release-notes] dependency by zachmu#2704

Open
coffeegoddd wants to merge 1 commit into
mainfrom
zachmu-476b8c07
Open

[auto-bump] [no-release-notes] dependency by zachmu#2704
coffeegoddd wants to merge 1 commit into
mainfrom
zachmu-476b8c07

Conversation

@coffeegoddd
Copy link
Copy Markdown
Contributor

An Automated Dependency Version Bump PR 👑

Initial Changes

The changes contained in this PR were produced by `go get`ing the dependency.

```bash
go get github.com/dolthub/[dependency]/go@[commit]
```

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Main PR
covering_index_scan_postgres 1336.35/s 1373.82/s +2.8%
index_join_postgres 190.82/s 190.20/s -0.4%
index_join_scan_postgres 198.57/s 197.40/s -0.6%
index_scan_postgres 12.23/s 12.21/s -0.2%
oltp_point_select 2519.30/s 2509.66/s -0.4%
oltp_read_only 1868.15/s 1862.67/s -0.3%
select_random_points 128.35/s 130.52/s +1.6%
select_random_ranges 1084.72/s 1080.22/s -0.5%
table_scan_postgres 11.94/s 11.91/s -0.3%
types_table_scan_postgres 5.47/s 5.51/s +0.7%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Main PR
Total 42090 42090
Successful 18017 18017
Failures 24073 24073
Partial Successes1 5372 5372
Main PR
Successful 42.8059% 42.8059%
Failures 57.1941% 57.1941%

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

@itoqa
Copy link
Copy Markdown

itoqa Bot commented May 7, 2026

Ito Test Report ✅

16 test cases ran. 3 additional findings, 13 passed.

Across 16 scenarios, 13 passed and 3 failed, showing strong coverage success for core query contracts, prepared-statement Parse/Bind behavior, replication startup/recovery, strict config validation, DDL hook execution, and first-boot/concurrency startup paths. The key confirmed production issues were a critical COPY FROM file-path security flaw enabling arbitrary host file import, a high-severity first-boot auth credential-source mismatch (environment bootstrap creds vs default listener config) that can prevent startup, and a medium-severity incomplete CancelRequest/backend key implementation that makes PID-based query cancellation unreliable, with all three marked as pre-existing rather than introduced by this PR.

✅ Passed (13)
Category Summary Screenshot
Binding Parse with empty parameter OIDs executed successfully and returned the expected typed result. N/A
Binding A short multi-code result format array was rejected, and the same connection remained usable afterward. N/A
Binding An invalid first bind failed cleanly, and a subsequent valid rebind executed successfully on the same prepared statement. N/A
Config Using the running valid-config server in the nested container, CREATE TABLE, ALTER TABLE (rename/add column), and DROP TABLE all succeeded, and information_schema checks matched expected schema transitions. CONFIG-1
Config Launching doltgres with a config containing unknown_top_level_field failed fast with strict YAML unmarshal error and non-zero exit (1), confirming startup is blocked on unknown top-level keys. CONFIG-2
Config Parallel ALTER/DROP on the same table produced an explicit schema conflict on one session while the other completed, and metadata remained consistent with no partial table/column state left behind. CONFIG-3
Engine Ordered SELECT over qa_rows returned 1..5 in sequence with no query execution errors. N/A
Engine Mixed plain SELECT, DML RETURNING, and command-style SET executed coherently with expected row payloads and command completions. N/A
Replica Startup with missing slot_name failed fast with an explicit validation error naming the missing replication field. N/A
Replica Missing upstream replication slot produced actionable retry errors, and restart succeeded after recreating the slot. N/A
Replica Bad replication credentials failed cleanly and a corrected restart recovered without leaving an orphan listener. N/A
Startup First boot from a clean data directory created auth state and accepted authenticated SQL with default credentials. STARTUP-2
Startup Dual first-boot startup showed explicit exclusive-init behavior: one process succeeded, one failed on lock, and auth remained healthy on the surviving process. STARTUP-3
ℹ️ Additional Findings (3)

These findings are unrelated to the current changes but were observed during testing.

Category Summary Screenshot
Engine 🚨 Confirmed real bug: COPY FROM accepts arbitrary absolute filesystem paths and imports host files (e.g., /etc/passwd) instead of rejecting/constraint-checking unsafe paths. ENGINE-2
Engine 🟠 Confirmed real bug: backend-PID cancellation path is not correctly implemented, so expected in-flight cancellation semantics are not reliably supported. ENGINE-3
Startup ⚠️ Setting only environment bootstrap credentials causes startup auth failure during default database creation and server abort. STARTUP-1
🚨 COPY FROM allows unsafe host file imports
  • What failed: The server reads an arbitrary absolute host path and imports rows, while expected behavior is to reject or strictly constrain unsafe file paths.
  • Impact: Any user who can run COPY FROM file-path statements can read server-host files into queryable tables. This is a direct data exposure vulnerability that can leak sensitive host information.
  • Steps to reproduce:
    1. Start the local server and create a target table.
    2. Issue COPY FROM '/etc/passwd' WITH (FORMAT csv, DELIMITER ':').
    3. Query the target table and observe imported host-file rows instead of a safe rejection.
    4. Stub / mock context: Authentication was intentionally bypassed by disabling SCRAM enforcement in server/authentication_scram.go so the SQL behavior checks could run deterministically without real login flows.
    5. Code analysis: I inspected the COPY execution path in the connection handler and found that file-based COPY operations call os.Open(stmt.File) directly, with an inline TODO noting missing security and privilege checks.
    6. Why this is likely a bug: The production COPY file-path path has no guardrails and explicitly acknowledges missing security checks, which directly explains unsafe absolute-path imports.
    7. Relevant code:

      server/connection_handler.go (lines 501-509)

      case *node.CopyFrom:
      	// When copying data from STDIN, the data is sent to the server as CopyData messages
      	// We send endOfMessages=false since the server will be in COPY DATA mode and won't
      	// be ready for more queries util COPY DATA mode is completed.
      	if injectedStmt.Stdin {
      		return true, false, h.handleCopyFromStdinQuery(injectedStmt, h.Conn())
      	} else {
      		// copying from a file is handled in a single message
      		return true, true, h.copyFromFileQuery(injectedStmt)
      	}

      server/connection_handler.go (lines 727-733)

      // TODO: security check for file path
      // TODO: Privilege Checking: https://www.postgresql.org/docs/15/sql-copy.html
      f, err := os.Open(stmt.File)
      if err != nil {
      	return err
      }
      defer f.Close()
      🟠 Backend PID cancellation protocol is incomplete
      • What failed: CancelRequest startup packets are not handled, and the backend secret key used for cancellation is hardcoded to 0, so expected backend-PID cancellation semantics are not reliably supported.
      • Impact: Query cancellation workflows are unreliable for long-running operations. Users may need to wait for completion or terminate sessions instead of performing targeted cancel operations.
      • Steps to reproduce:
        1. Open one session running a long query and a second session intended to cancel it.
        2. Attempt backend PID cancellation through the PostgreSQL cancel-request path.
        3. Observe cancellation does not execute with expected semantics and then verify baseline queries still run.
      • Stub / mock context: Authentication was intentionally bypassed by disabling SCRAM enforcement in server/authentication_scram.go so the SQL behavior checks could run deterministically without real login flows.
      • Code analysis: I inspected startup message handling and backend key generation in the server connection handler; the cancel-request packet type is missing from the startup switch, and the connection key is explicitly marked as non-unique.
      • Why this is likely a bug: The server omits CancelRequest startup handling and emits a non-usable cancellation key by design, which makes protocol-level cancellation incomplete in production behavior.

      Relevant code:

      server/connection_handler.go (lines 220-262)

      switch sm := startupMessage.(type) {
      case *pgproto3.StartupMessage:
      	if err = h.handleAuthentication(sm); err != nil {
      		return false, err
      	}
      	if err = h.sendClientStartupMessages(); err != nil {
      		return false, err
      	}
      	if err = h.chooseInitialParameters(sm); err != nil {
      		return false, err
      	}
      	return true, h.send(&pgproto3.ReadyForQuery{TxStatus: byte(ReadyForQueryTransactionIndicator_Idle)})
      default:
      	return false, errors.Errorf("terminating connection: unexpected start message: %#v", startupMessage)
      }

      server/connection_handler.go (lines 291-294)

      return h.send(&pgproto3.BackendKeyData{
      	ProcessID: processID,
      	SecretKey: 0, // TODO: this should represent an ID that can uniquely identify this connection, so that CancelRequest will work
      })
      ⚠️ Environment override credentials fail during bootstrap
      • What failed: Expected behavior is that environment override credentials fully replace default bootstrap login and allow startup completion; instead, startup aborts with auth failure when creating the default database.
      • Impact: New deployments that rely on environment-based bootstrap credential overrides can fail to start, leaving the service unavailable until config and env credentials are manually aligned.
      • Steps to reproduce:
        1. Set DOLTGRES_USER and DOLTGRES_PASSWORD to non-default values.
        2. Start the server from a clean first-boot state.
        3. Observe default database creation attempting auth with override credentials and startup exiting on authentication failure.
      • Stub / mock context: No stubs, mocks, or bypasses were applied for this test in the recorded run.
      • Code analysis: I traced startup auth/bootstrap code and found that auth initialization and default DB creation read superuser credentials from environment variables, while the server listener config still defaults to postgres/password unless explicit config user/password is set. This creates a production-code credential mismatch in the startup path.
      • Why this is likely a bug: Production startup code uses one credential source (env) for internal bootstrap connection while the active listener auth defaults to another source (config defaults), which can deterministically break startup.

      Relevant code:

      server/auth/init.go (lines 43-53)

      // GetSuperUserAndPassword returns the superuser and password for the server to use, as defined in the environment
      func GetSuperUserAndPassword() (string, string) {
          user := "postgres"
          if envUser := os.Getenv(doltgresUserEnvVar); envUser != "" {
              user = envUser
          }
      
          password := "password"
          if envPassword := os.Getenv(doltgresPasswordEnvVar); envPassword != "" {
              password = envPassword
          }

      server/server.go (lines 199-214)

      func createDefaultDatabase(cfg doltservercfg.ServerConfig) error {
          user, password := auth.GetSuperUserAndPassword()
          dbName := getDefaultDatabaseName(user)
      
          dsn := fmt.Sprintf("postgres://%s:%s@localhost:%d", user, password, cfg.Port())
      
          ctx := context.Background()
          conn, err := pgx.Connect(ctx, dsn)
          if err != nil {
              return err
          }
          defer conn.Close(ctx)

      servercfg/cfgdetails/config.go (lines 247-264)

      func (cfg *DoltgresConfig) User() string {
          if cfg.UserConfig == nil || cfg.UserConfig.Name == nil {
              return "postgres"
          }
          return *cfg.UserConfig.Name
      }
      
      func (cfg *DoltgresConfig) Password() string {
          if cfg.UserConfig == nil || cfg.UserConfig.Password == nil {
              return "password"
          }
          return *cfg.UserConfig.Password
      }

      Commit: df1fb9b

      View Full Run


      Tell us how we did: Give Ito Feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants