Skip to content

Commit d3b069b

Browse files
fix: Validate DB/IMDb IDs and add tests
Prevent path-traversal/invalid identifier usage when writing item JSON files by adding regex constants (DATABASE_ITEM_ID_PATTERN, IMDB_ID_PATTERN) and a helper _build_database_json_path that validates an ID before building an absolute path. _write_item_files now uses the helper for the primary database id and the IMDb copy, raising ValueError for invalid IDs. New unit tests assert that unsafe database ids and unsafe IMDb ids are rejected and that no files are created when validation fails.
1 parent 8bcc1fb commit d3b069b

2 files changed

Lines changed: 55 additions & 2 deletions

File tree

src/updater.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@
109109
TOP_CONTRIBUTORS_FILENAME = f'{TOP_CONTRIBUTORS_BASENAME}.svg'
110110
JSON_EXTENSION = '.json'
111111
PNG_CONTENT_TYPE = 'image/png'
112+
DATABASE_ITEM_ID_PATTERN = re.compile(r'\d+')
113+
IMDB_ID_PATTERN = re.compile(r'tt\d+')
112114
APPROVE_THEME_LABEL = 'approve-theme'
113115
ISSUE_AUTHOR_BADGE_STYLE = 'for-the-badge'
114116
ISSUE_AUTHOR_BADGES = {
@@ -1310,14 +1312,37 @@ def _update_issue_audit_data(og_data: dict,
13101312
_write_auto_close_message(message=DUPLICATE_NO_REPLACEMENT_REASON_CLOSE_MESSAGE)
13111313

13121314

1315+
def _build_database_json_path(base_dir: str, item_id: Union[int, str], pattern: re.Pattern, label: str) -> str:
1316+
"""Build a database JSON path from an expected identifier format."""
1317+
item_id_text = str(item_id)
1318+
if not pattern.fullmatch(item_id_text):
1319+
raise ValueError(f'Invalid {label}: {item_id_text}')
1320+
1321+
return os.path.join(os.path.abspath(base_dir), f'{item_id_text}{JSON_EXTENSION}')
1322+
1323+
13131324
def _write_item_files(database_path: str, item_type: str, og_data: dict) -> None:
13141325
"""Write the updated database item files."""
1315-
destination_filenames = [os.path.join(database_path, f'{og_data["id"]}.json')]
1326+
destination_filenames = [
1327+
_build_database_json_path(
1328+
base_dir=database_path,
1329+
item_id=og_data['id'],
1330+
pattern=DATABASE_ITEM_ID_PATTERN,
1331+
label='database item id',
1332+
),
1333+
]
13161334

13171335
if item_type == 'movie':
13181336
try:
13191337
if og_data["imdb_id"]:
1320-
destination_filenames.append(os.path.join(imdb_path, f'{og_data["imdb_id"]}.json'))
1338+
destination_filenames.append(
1339+
_build_database_json_path(
1340+
base_dir=imdb_path,
1341+
item_id=og_data['imdb_id'],
1342+
pattern=IMDB_ID_PATTERN,
1343+
label='IMDb id',
1344+
),
1345+
)
13211346
except KeyError as e:
13221347
print_github_error(f'Error getting imdb_id: {e}')
13231348

tests/unit/test_issue_updater.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,34 @@ def test_write_item_files_writes_primary_and_imdb_copy(tmp_path, monkeypatch):
499499
assert json.loads((imdb_dir / 'tt0113189.json').read_text()) == item_data
500500

501501

502+
@pytest.mark.parametrize('unsafe_id', ['../710', '710/evil', '710.json'])
503+
def test_write_item_files_rejects_unsafe_database_id(tmp_path, unsafe_id):
504+
"""Test database item file writing rejects unsafe item identifiers."""
505+
database_path = tmp_path / 'database' / 'movies' / 'themoviedb'
506+
item_data = {'id': unsafe_id, 'title': 'GoldenEye'}
507+
508+
with pytest.raises(ValueError, match='Invalid database item id'):
509+
updater._write_item_files(database_path=str(database_path), item_type='movie_collection', og_data=item_data)
510+
511+
assert not database_path.exists()
512+
assert not (tmp_path / 'database' / 'movies' / '710.json').exists()
513+
514+
515+
@pytest.mark.parametrize('unsafe_imdb_id', ['../tt0113189', 'tt0113189/evil', 'nm0113189'])
516+
def test_write_item_files_rejects_unsafe_imdb_id(tmp_path, monkeypatch, unsafe_imdb_id):
517+
"""Test movie file writing rejects unsafe IMDb identifiers before writing files."""
518+
database_path = tmp_path / 'database' / 'movies' / 'themoviedb'
519+
imdb_dir = tmp_path / 'database' / 'movies' / 'imdb'
520+
item_data = {'id': 710, 'imdb_id': unsafe_imdb_id, 'title': 'GoldenEye'}
521+
monkeypatch.setattr(updater, 'imdb_path', str(imdb_dir))
522+
523+
with pytest.raises(ValueError, match='Invalid IMDb id'):
524+
updater._write_item_files(database_path=str(database_path), item_type='movie', og_data=item_data)
525+
526+
assert not database_path.exists()
527+
assert not imdb_dir.exists()
528+
529+
502530
def test_load_issue_submission_values_uses_supplied_values(monkeypatch):
503531
"""Test fully supplied issue-update values bypass submission loading but still validate YouTube."""
504532
monkeypatch.setattr(

0 commit comments

Comments
 (0)