Skip to content

Add tests for dolt_remote() operations#2906

Merged
fulghum merged 2 commits into
mainfrom
fulghum/remotes-testing
Jul 10, 2026
Merged

Add tests for dolt_remote() operations#2906
fulghum merged 2 commits into
mainfrom
fulghum/remotes-testing

Conversation

@fulghum

@fulghum fulghum commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor
Main PR
covering_index_scan_postgres 1869.31/s 1920.64/s +2.7%
groupby_scan_postgres 121.86/s 130.31/s +6.9%
index_join_postgres 665.99/s 668.12/s +0.3%
index_join_scan_postgres 850.97/s 842.66/s -1.0%
index_scan_postgres 27.34/s 26.86/s -1.8%
oltp_delete_insert_postgres 802.65/s 797.93/s -0.6%
oltp_insert 709.22/s 721.91/s +1.7%
oltp_point_select 3026.10/s 3026.15/s 0.0%
oltp_read_only 3009.76/s 2998.38/s -0.4%
oltp_read_write 2282.76/s 2302.64/s +0.8%
oltp_update_index 707.28/s 756.61/s +6.9%
oltp_update_non_index 764.22/s 777.24/s +1.7%
oltp_write_only 1724.03/s 1724.33/s 0.0%
select_random_points 1890.40/s 1886.76/s -0.2%
select_random_ranges 1087.33/s 1090.28/s +0.2%
table_scan_postgres 25.82/s 26.13/s +1.2%
types_delete_insert_postgres 776.03/s 770.82/s -0.7%
types_table_scan_postgres 11.23/s 11.08/s -1.4%

@fulghum fulghum force-pushed the fulghum/remotes-testing branch from 6320aa2 to 5cc8558 Compare July 8, 2026 21:57
@itoqa

itoqa Bot commented Jul 8, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: 5cc8558: 10 test cases ran, 10 passed ✅.

Summary

This run exercised core repository collaboration behaviors end to end, including cloning specific branches, fetching and pushing with both valid and invalid inputs, pulling through merge and conflict paths, and resolving conflicts cleanly without leftover state. It also covered schema continuity after replication, confirming sequence behavior, trigger and type-conversion runtime behavior, and isolation of remote scenarios all remained stable.

Safe to merge — the tested branch, remote sync, conflict-handling, and replicated-schema behaviors all remained healthy with no regressions observed in this PR. There are no failures attributable to this change, so current merge risk is low.

Tests run by Ito

View full run

Result Severity Type Description
Branch The non-default branch clone flow behaved correctly: cloning with --branch feature produced feature-branch data (a,b,c,feature-only) and the next insert returned id 5, confirming sequence continuity on the cloned branch.
Branch The non-fast-forward push was rejected with explicit diagnostics, and the immediate force-push retry completed successfully in the targeted remote-sync scenario without evidence of partial overwrite or corruption.
Config Calling select dolt_fetch('origin', 'garbage') returned an 'invalid ref spec' error and did not create unintended remote-tracking refs, so the malformed refspec rejection behavior worked as expected.
Config The fetch call with refspec refs/heads/main:refs/remotes/custom/main completed successfully and the observer repository contained exactly remotes/custom/main as asserted.
Harness The isolation checks passed: the run created unique temp-backed file:// remote and database directories per subtest with no cross-test state leakage; the single failing subtest was an environment dependency issue (missing pg_config), not an isolation defect.
Remote The pull conflict path behaved as expected: conflicts were surfaced with base/ours/theirs values, resolving with --ours cleared conflict metadata, and the local edit was preserved.
Remote Go subtest TestDoltRemote/pull_auto-merges_non-conflicting_divergent_history passed: dolt_pull('origin') returned merge successful and the merged table preserved both origin and clone non-overlapping rows.
Remote The conflict-resolution flow completed cleanly: conflict metadata and conflict table counts were zero after resolve, follow-up commit succeeded, and a subsequent pull returned a non-error success path.
Schema Both serial and identity sequence-preservation subtests passed after push and clone, including continuity of generated IDs and owned-sequence bindings.
Schema The trigger and custom cast replication checks passed: cloned metadata remained present and both runtime paths executed successfully after source-side objects were removed.

Tip

Reply with @itoqa to send us feedback on this test run.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor
Main PR
Total 42090 42090
Successful 18275 18275
Failures 23815 23815
Partial Successes1 5335 5335
Main PR
Successful 43.4189% 43.4189%
Failures 56.5811% 56.5811%

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.

@fulghum fulghum requested a review from zachmu July 8, 2026 22:57

@zachmu zachmu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general these look good, but it's a little disappointing that we test pull and clone only into the same server that we pushed from originally. Would be nice to have tests of cloning from a completely fresh server, updating the original, pushing those changes, and pulling them on the clone. Maybe best expressed as a bats test.

@itoqa

itoqa Bot commented Jul 9, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Report5cc855887911b8: 10 test cases ran, 9 passing ✅, 1 additional finding ⚠️.

Diff Summary

This run exercised core database behavior across startup resilience, remote sync flows (clone, push, pull), and schema/state continuity for enums, functions, and sequences, covering both normal operation and edge timing around restarts and warmup. It also included an adversarial configuration path scenario to probe filesystem safety boundaries for remote targets.

Safe to merge — the exercised behaviors remained stable for this PR, with no regressions or other failures attributed to the change. The only failure is a high-severity additional product finding that is not caused by this PR, so it is a flag-for-later issue rather than a merge blocker for this branch.

Tests run by Ito

View full run

Result State Severity Type Description
Passing Context Go test TestContext1StartupMetadataLoad passed, and post-test metrics confirmed the server stayed up while enum-backed metadata queries completed successfully.
Passing Context The recorded run for TestContext2DeferredUnresolvedType passed and showed stable enum type resolution before and after context-backed operations; the original blocked status came from a harness hang after completion, not an application defect.
Passing Context The test scenario passed: after restart with enum-backed metadata, startup completed without panic and type-dependent queries succeeded. The runner's blocked status came from a harness completion-signal hang, not an application defect.
Passing Harness The remotes file-system restart scenario passed end-to-end, confirming clone, push, and pull behavior stayed correct across stop/start boundaries with separate server data directories.
Passing Remote Server A committed and pushed seeded schema/data to a file remote, and a fresh Server B clone returned the expected rows (apple/happy, banana/sad) plus double_it(21)=42.
Passing Remote Server B successfully pulled Server A's second push and reflected the new row without recloning; dolt_pull returned "merge successful" and existing replicated objects remained usable.
Passing Remote The sequence state replicated correctly across remote sync phases: server B reported last_value 100 after clone, 150 after pull, and nextval returned 200 with the expected increment.
Passing Schema Go and bats coverage both passed, confirming the mood enum definition and enum-backed values (happy, sad, ok) persist across initial clone and subsequent pull from the file remote.
Passing Schema Go and bats coverage both passed, confirming post-pull sequence state is honored: after pull, nextval('counter') advanced from the pulled head (6 in Go and 200 in bats) instead of reusing a stale pre-pull counter.
⏸️ Skipped Branch The non-default branch clone flow behaved correctly: cloning with --branch feature produced feature-branch data (a,b,c,feature-only) and the next insert returned id 5, confirming sequence continuity on the cloned branch.
⏸️ Skipped Branch The non-fast-forward push was rejected with explicit diagnostics, and the immediate force-push retry completed successfully in the targeted remote-sync scenario without evidence of partial overwrite or corruption.
⏸️ Skipped Config Calling select dolt_fetch('origin', 'garbage') returned an 'invalid ref spec' error and did not create unintended remote-tracking refs, so the malformed refspec rejection behavior worked as expected.
⏸️ Skipped Config The fetch call with refspec refs/heads/main:refs/remotes/custom/main completed successfully and the observer repository contained exactly remotes/custom/main as asserted.
⏸️ Skipped Harness The isolation checks passed: the run created unique temp-backed file:// remote and database directories per subtest with no cross-test state leakage; the single failing subtest was an environment dependency issue (missing pg_config), not an isolation defect.
⏸️ Skipped Remote The pull conflict path behaved as expected: conflicts were surfaced with base/ours/theirs values, resolving with --ours cleared conflict metadata, and the local edit was preserved.
⏸️ Skipped Remote Go subtest TestDoltRemote/pull_auto-merges_non-conflicting_divergent_history passed: dolt_pull('origin') returned merge successful and the merged table preserved both origin and clone non-overlapping rows.
⏸️ Skipped Remote The conflict-resolution flow completed cleanly: conflict metadata and conflict table counts were zero after resolve, follow-up commit succeeded, and a subsequent pull returned a non-error success path.
⏸️ Skipped Schema Both serial and identity sequence-preservation subtests passed after push and clone, including continuity of generated IDs and owned-sequence bindings.
⏸️ Skipped Schema The trigger and custom cast replication checks passed: cloned metadata remained present and both runtime paths executed successfully after source-side objects were removed.
⚠️ Additional Finding High severity Config Expected behavior was rejection of malformed or inaccessible filesystem remotes before unsafe sync. Actual behavior accepted those remotes and persisted remote refs while creating repository files at attacker-controlled filesystem paths.
Additional Findings Details

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

🟠 File remote path validation allows arbitrary filesystem writes
  • Severity: High High severity
  • Description: Expected behavior was rejection of malformed or inaccessible filesystem remotes before unsafe sync. Actual behavior accepted those remotes and persisted remote refs while creating repository files at attacker-controlled filesystem paths.
  • Impact: Users can configure remotes that write repository data to attacker-chosen filesystem paths, including traversal-style locations, creating persistent files outside expected directories. This can compromise host integrity and store data in unintended locations without a safe rejection.
  • Steps to Reproduce:
    1. Create a commit in Doltgres, then run SELECT dolt_remote('add', 'origin', 'file:///this/path/definitely/does/not/exist/anywhere');.
    2. Run SELECT dolt_push('origin', 'main'); and observe success instead of a validation error.
    3. Inspect the target path and confirm Dolt repo files (manifest, LOCK, chunk files, oldgen/) were created; repeat with file:///../../tmp/evil_path_traversal and observe normalized writes under /tmp.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: server/functions/dolt_procedures.go registers and dispatches Dolt procedures from dprocedures.DoltProcedures and invokes them with user-supplied arguments, but this integration layer adds no file-path safety guard before execution. The observed repro aligns with this: dolt_remote('add', ...) accepted non-existent and traversal-form paths and dolt_push wrote remote state to those locations. PR-changed lines in core/context.go and server/types/serialization.go are nil-context handling only and are outside this remote path-validation flow.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

@fulghum fulghum merged commit ea8c709 into main Jul 10, 2026
25 of 26 checks passed
@fulghum fulghum deleted the fulghum/remotes-testing branch July 10, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants