From 7e140153a3e53655411d4afe2313981eda2c1c3e Mon Sep 17 00:00:00 2001 From: sunliqiang Date: Sat, 8 Aug 2026 13:10:21 +0800 Subject: [PATCH 1/3] Add tests for external file path validation Two tests covering the DDL-time check of external table file paths against the ExternalFileAccess policy: - test_external_file_access_denied: CREATE TABLE with a path outside the restricted directory list must fail with SQLSTATE 28000 at CREATE TABLE time. - test_external_file_allowed_dir: a table inside the allowed directory still works, guarding against regressions. Both tests skip when ExternalFileAccess is not configured as a restricted list. --- .../gtcs/test_external_file_access_denied.py | 62 +++++++++++++++++++ .../gtcs/test_external_file_allowed_dir.py | 54 ++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 tests/functional/gtcs/test_external_file_access_denied.py create mode 100644 tests/functional/gtcs/test_external_file_allowed_dir.py diff --git a/tests/functional/gtcs/test_external_file_access_denied.py b/tests/functional/gtcs/test_external_file_access_denied.py new file mode 100644 index 00000000..041708d5 --- /dev/null +++ b/tests/functional/gtcs/test_external_file_access_denied.py @@ -0,0 +1,62 @@ +#coding:utf-8 + +""" +ID: gtcs.external-file-access-denied +FBTEST: functional.gtcs.external_file_access_denied +TITLE: External file path outside configured directories is rejected at DDL time +DESCRIPTION: + When ExternalFileAccess is set to a restricted directory list, creating an + external table whose file path escapes the allowed directories must be + rejected during CREATE TABLE, not silently stored in metadata and only + failing later on access. + + Without the fix, the CREATE TABLE succeeded and the malicious path was + stored in RDB$EXTERNAL_FILE; the access check happened only when the + table was actually opened. With the fix, the path is validated at + metadata load time via checkExternalFileAccess(). + +NOTES: + [08.08.2026] sunliqiang + Test requires ExternalFileAccess set to a restricted list (e.g. + 'Restrict '). If external file access is configured as 'None', + the test is skipped. +""" + +import pytest +from firebird.qa import * + +db = db_factory(user='SYSDBA', password='masterkey') + +act = isql_act('db') + +expected_stderr = """ +Statement failed, SQLSTATE = 28000 +Use of external file at location /etc/passwd is not allowed by server configuration +""" + + +@pytest.mark.version('>=4.0') +def test_1(act: Action): + # Create a table pointing outside the allowed directory list. + # With the fix this must fail at DDL time. + sql = ''' + create table ext_escape external file '/etc/passwd' (line varchar(200)); + exit; + ''' + + act.expected_stderr = expected_stderr + + act.isql(switches=['-q'], input=sql) + + if 'SQLSTATE = 28000' not in act.clean_stderr: + # ExternalFileAccess may be 'None' in the test environment, which + # rejects the path with a different message, or 'Full' which allows + # everything. Distinguish: None rejects at open time (not DDL), + # Full allows. Only skip when the configuration is not a + # restricted list at all. + if 'not allowed by server configuration' not in act.clean_stderr: + pytest.skip('ExternalFileAccess is not set to a restricted list') + + assert 'SQLSTATE = 28000' in act.clean_stderr + assert 'not allowed by server configuration' in act.clean_stderr + assert act.clean_stderr == act.clean_expected_stderr diff --git a/tests/functional/gtcs/test_external_file_allowed_dir.py b/tests/functional/gtcs/test_external_file_allowed_dir.py new file mode 100644 index 00000000..bc611b4a --- /dev/null +++ b/tests/functional/gtcs/test_external_file_allowed_dir.py @@ -0,0 +1,54 @@ +#coding:utf-8 + +""" +ID: gtcs.external-file-allowed-dir +FBTEST: functional.gtcs.external_file_allowed_dir +TITLE: External table in the allowed directory still works +DESCRIPTION: + Regression test for the external file path validation added at metadata + load time. A table pointing to a file inside the configured allowed + directory must still be usable. + +NOTES: + [08.08.2026] sunliqiang + Test requires ExternalFileAccess set to a restricted list that includes + the test temp directory (e.g. 'Restrict /tmp'). Skipped otherwise. +""" + +from pathlib import Path +import pytest +from firebird.qa import * + +db = db_factory(user='SYSDBA', password='masterkey') + +act = isql_act('db') + +tmp_ext_file = temp_file('ext_allowed_dir_test.dat') + +expected_stdout = """ +ID 42 +""" + + +@pytest.mark.version('>=4.0') +def test_1(act: Action, tmp_ext_file: Path): + sql = f''' + create table ext_ok external file '{tmp_ext_file}' (id int); + commit; + insert into ext_ok (id) values (42); + commit; + set list on; + select id from ext_ok; + exit; + ''' + + act.isql(switches=['-q'], input=sql) + + if 'not allowed by server configuration' in act.clean_stderr: + pytest.skip('ExternalFileAccess does not include the temp directory') + + act.expected_stdout = expected_stdout + act.expected_stderr = "" + + assert act.clean_stdout == act.clean_expected_stdout + assert act.clean_stderr == "" From ab99c71bdb5a7dd55cae41d4fd8c8405f3881cd2 Mon Sep 17 00:00:00 2001 From: sunliqiang Date: Sat, 8 Aug 2026 23:09:50 +0800 Subject: [PATCH 2/3] Make external file path tests self-contained The tests previously relied on the ExternalFileAccess setting of the test environment and skipped when it was not a restricted list, which made them ineffective in the default QA configuration. Add dedicated databases.conf aliases with ExternalFileAccess restricted to the database directory, and derive the denied/allowed file paths from the actual database location at runtime. The tests no longer skip; a missing fix now fails them instead. Verified: 2 passed with the fix, 1 failed without it. --- files/qa-databases.conf | 13 ++++ .../gtcs/test_external_file_access_denied.py | 77 +++++++++++-------- .../gtcs/test_external_file_allowed_dir.py | 64 +++++++++++---- 3 files changed, 103 insertions(+), 51 deletions(-) diff --git a/files/qa-databases.conf b/files/qa-databases.conf index 4a74abc4..c4fd3460 100644 --- a/files/qa-databases.conf +++ b/files/qa-databases.conf @@ -385,3 +385,16 @@ db_repl_8766_alias = $(dir_sampleDb)/qa_replication/db_repl_8766.fdb # db_sync_main_alias = $(dir_sampleDb)/qa_replication/db_sync_main.fdb db_sync_repl_alias = $(dir_sampleDb)/qa_replication/db_sync_repl.fdb + +# Tests for external table path validation. +# Companion tests for FirebirdSQL/firebird#9121. + +tmp_external_file_access_denied_alias = $(dir_sampleDb)/qa/tmp_external_file_access_denied.fdb +{ + ExternalFileAccess = Restrict $(dir_sampleDb)/qa +} + +tmp_external_file_allowed_alias = $(dir_sampleDb)/qa/tmp_external_file_allowed.fdb +{ + ExternalFileAccess = Restrict $(dir_sampleDb)/qa +} diff --git a/tests/functional/gtcs/test_external_file_access_denied.py b/tests/functional/gtcs/test_external_file_access_denied.py index 041708d5..a4175c42 100644 --- a/tests/functional/gtcs/test_external_file_access_denied.py +++ b/tests/functional/gtcs/test_external_file_access_denied.py @@ -5,58 +5,67 @@ FBTEST: functional.gtcs.external_file_access_denied TITLE: External file path outside configured directories is rejected at DDL time DESCRIPTION: - When ExternalFileAccess is set to a restricted directory list, creating an - external table whose file path escapes the allowed directories must be - rejected during CREATE TABLE, not silently stored in metadata and only - failing later on access. + When ExternalFileAccess is restricted, creating an external table whose + file path is outside the allowed directory must be rejected during + CREATE TABLE. - Without the fix, the CREATE TABLE succeeded and the malicious path was - stored in RDB$EXTERNAL_FILE; the access check happened only when the - table was actually opened. With the fix, the path is validated at - metadata load time via checkExternalFileAccess(). + Without the fix, CREATE TABLE succeeds and the invalid path is stored + in RDB$EXTERNAL_FILE. The access check is performed only when the + external file is opened. + + With the fix, the path is validated while relation metadata is loaded, + so CREATE TABLE fails immediately. NOTES: [08.08.2026] sunliqiang - Test requires ExternalFileAccess set to a restricted list (e.g. - 'Restrict '). If external file access is configured as 'None', - the test is skipped. + The test database uses a dedicated databases.conf alias with + ExternalFileAccess restricted to its database directory. """ +from pathlib import Path + import pytest from firebird.qa import * -db = db_factory(user='SYSDBA', password='masterkey') -act = isql_act('db') +REQUIRED_ALIAS = 'tmp_external_file_access_denied_alias' -expected_stderr = """ -Statement failed, SQLSTATE = 28000 -Use of external file at location /etc/passwd is not allowed by server configuration -""" +db = db_factory(filename='#' + REQUIRED_ALIAS, user='SYSDBA', password='masterkey', async_write=False, do_not_drop=True) + +act = isql_act('db') @pytest.mark.version('>=4.0') def test_1(act: Action): - # Create a table pointing outside the allowed directory list. - # With the fix this must fail at DDL time. - sql = ''' - create table ext_escape external file '/etc/passwd' (line varchar(200)); + + # Obtain the physical database location. The database is created inside + # the directory allowed by ExternalFileAccess. + with act.db.connect() as con: + cur = con.cursor() + cur.execute('select mon$database_name from mon$database') + db_path = Path(cur.fetchone()[0]) + + allowed_dir = db_path.parent + + # Use a file in the parent directory, which is deliberately outside + # ExternalFileAccess = Restrict . + denied_file = allowed_dir.parent / 'ext_access_denied_test.dat' + + external_file = str(denied_file).replace("'", "''") + + sql = f""" + create table ext_escape external file '{external_file}' + ( + line varchar(200) + ); exit; - ''' + """ - act.expected_stderr = expected_stderr + act.expected_stderr = f""" +Statement failed, SQLSTATE = 28000 +Use of external file at location {denied_file} is not allowed by server configuration +""" act.isql(switches=['-q'], input=sql) - if 'SQLSTATE = 28000' not in act.clean_stderr: - # ExternalFileAccess may be 'None' in the test environment, which - # rejects the path with a different message, or 'Full' which allows - # everything. Distinguish: None rejects at open time (not DDL), - # Full allows. Only skip when the configuration is not a - # restricted list at all. - if 'not allowed by server configuration' not in act.clean_stderr: - pytest.skip('ExternalFileAccess is not set to a restricted list') - - assert 'SQLSTATE = 28000' in act.clean_stderr - assert 'not allowed by server configuration' in act.clean_stderr assert act.clean_stderr == act.clean_expected_stderr diff --git a/tests/functional/gtcs/test_external_file_allowed_dir.py b/tests/functional/gtcs/test_external_file_allowed_dir.py index bc611b4a..bc161ba3 100644 --- a/tests/functional/gtcs/test_external_file_allowed_dir.py +++ b/tests/functional/gtcs/test_external_file_allowed_dir.py @@ -5,25 +5,30 @@ FBTEST: functional.gtcs.external_file_allowed_dir TITLE: External table in the allowed directory still works DESCRIPTION: - Regression test for the external file path validation added at metadata - load time. A table pointing to a file inside the configured allowed - directory must still be usable. + Regression test for external file path validation at metadata load time. + + When ExternalFileAccess is restricted to the directory containing the + test database, an external table whose file is located inside that + directory must still work normally. NOTES: [08.08.2026] sunliqiang - Test requires ExternalFileAccess set to a restricted list that includes - the test temp directory (e.g. 'Restrict /tmp'). Skipped otherwise. + The test database uses a dedicated databases.conf alias with + ExternalFileAccess restricted to its database directory. """ from pathlib import Path + import pytest from firebird.qa import * -db = db_factory(user='SYSDBA', password='masterkey') + +REQUIRED_ALIAS = 'tmp_external_file_allowed_alias' + +db = db_factory(filename='#' + REQUIRED_ALIAS, user='SYSDBA', password='masterkey', async_write=False, do_not_drop=True) act = isql_act('db') -tmp_ext_file = temp_file('ext_allowed_dir_test.dat') expected_stdout = """ ID 42 @@ -31,24 +36,49 @@ @pytest.mark.version('>=4.0') -def test_1(act: Action, tmp_ext_file: Path): - sql = f''' - create table ext_ok external file '{tmp_ext_file}' (id int); +def test_1(act: Action): + + # The database itself is located in the directory configured in + # ExternalFileAccess, so a sibling external file is guaranteed to + # be inside the allowed directory. + with act.db.connect() as con: + cur = con.cursor() + cur.execute('select mon$database_name from mon$database') + db_path = Path(cur.fetchone()[0]) + + ext_file = db_path.parent / 'ext_access_allowed_test.dat' + + # Remove leftovers from an interrupted/failed previous test run. + ext_file.unlink(missing_ok=True) + + external_file = str(ext_file).replace("'", "''") + + sql = f""" + create table ext_ok external file '{external_file}' + ( + id int + ); commit; + insert into ext_ok (id) values (42); commit; + set list on; select id from ext_ok; - exit; - ''' - act.isql(switches=['-q'], input=sql) + drop table ext_ok; + commit; - if 'not allowed by server configuration' in act.clean_stderr: - pytest.skip('ExternalFileAccess does not include the temp directory') + exit; + """ act.expected_stdout = expected_stdout act.expected_stderr = "" - assert act.clean_stdout == act.clean_expected_stdout - assert act.clean_stderr == "" + try: + act.isql(switches=['-q'], input=sql) + + assert act.clean_stdout == act.clean_expected_stdout + assert act.clean_stderr == act.clean_expected_stderr + finally: + ext_file.unlink(missing_ok=True) From dd3c960c4d4c7f7041e5a0f1d2abde68ae2c37b0 Mon Sep 17 00:00:00 2001 From: sunliqiang Date: Sat, 8 Aug 2026 23:28:05 +0800 Subject: [PATCH 3/3] Use standard db_factory defaults in external file tests Drop the explicit user, password, async_write and do_not_drop parameters. The QA harness supplies credentials from the server configuration, and the database is removed by the fixture teardown as usual, so no leftover database is left between runs. --- tests/functional/gtcs/test_external_file_access_denied.py | 2 +- tests/functional/gtcs/test_external_file_allowed_dir.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/gtcs/test_external_file_access_denied.py b/tests/functional/gtcs/test_external_file_access_denied.py index a4175c42..c9d14a85 100644 --- a/tests/functional/gtcs/test_external_file_access_denied.py +++ b/tests/functional/gtcs/test_external_file_access_denied.py @@ -30,7 +30,7 @@ REQUIRED_ALIAS = 'tmp_external_file_access_denied_alias' -db = db_factory(filename='#' + REQUIRED_ALIAS, user='SYSDBA', password='masterkey', async_write=False, do_not_drop=True) +db = db_factory(filename='#' + REQUIRED_ALIAS) act = isql_act('db') diff --git a/tests/functional/gtcs/test_external_file_allowed_dir.py b/tests/functional/gtcs/test_external_file_allowed_dir.py index bc161ba3..a57275a8 100644 --- a/tests/functional/gtcs/test_external_file_allowed_dir.py +++ b/tests/functional/gtcs/test_external_file_allowed_dir.py @@ -25,7 +25,7 @@ REQUIRED_ALIAS = 'tmp_external_file_allowed_alias' -db = db_factory(filename='#' + REQUIRED_ALIAS, user='SYSDBA', password='masterkey', async_write=False, do_not_drop=True) +db = db_factory(filename='#' + REQUIRED_ALIAS) act = isql_act('db')