Test that https://unpkg.com is used only in the connected mode #442
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| f"{nb_ipynb.read_text()}" | ||
| ) | ||
| assert ( | ||
| "https://unpkg.com" in nb_ipynb.read_text() |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To more robustly verify that the notebook is importing itables from unpkg.com, the test should look for a JavaScript/CSS import statement or a precise URL string within a code cell or output area that matches the expected pattern, rather than a simple substring match. This can be done by parsing the notebook's JSON, examining the cells for references to "https://unpkg.com/itables" (or whatever is the exact import string).
How to fix:
- Parse the notebook file (
nb_ipynb) as JSON. - Iterate through the notebook's code cells (and possibly outputs), looking for code or output containing an exact URL import from unpkg.com, e.g. as a regex or full string match.
- Make the assertion based on this precise check rather than substring matching.
What to change:
- In
test_connected_notebook_is_small, replace the substring test in line 36 with code that parses the notebook JSON and checks whether any cell source or output contains the exact expected import URL for itables from unpkg.com. - Import the
jsonmodule at the top of the file, if not already imported.
| @@ -1,7 +1,7 @@ | ||
| import pytest | ||
| from jupytext.cli import jupytext | ||
| import json | ||
|
|
||
|
|
||
| def text_notebook(connected, display_logo_when_loading=True): | ||
| return f"""# %% | ||
| import itables | ||
| @@ -32,9 +31,35 @@ | ||
| f"Notebook size is too large: {nb_ipynb.stat().st_size} bytes:\n" | ||
| f"{nb_ipynb.read_text()}" | ||
| ) | ||
| assert ( | ||
| "https://unpkg.com" in nb_ipynb.read_text() | ||
| ), "The connected notebook should import itables from unpkg.com" | ||
| notebook = json.loads(nb_ipynb.read_text()) | ||
| found = False | ||
| expected_domain = "https://unpkg.com/itables" | ||
| for cell in notebook.get("cells", []): | ||
| # Check source code of each cell for import from unpkg.com/itables | ||
| if any( | ||
| expected_domain in line | ||
| for line in (cell.get("source", "") if isinstance(cell.get("source", ""), list) else [cell.get("source", "")]) | ||
| ): | ||
| found = True | ||
| break | ||
| # Also check cell outputs (HTML/javascript might be injected here) | ||
| for output in cell.get("outputs", []): | ||
| # Output text may be a list of lines or a single string | ||
| data = output.get("data", {}) | ||
| for value in data.values(): | ||
| if isinstance(value, str): | ||
| if expected_domain in value: | ||
| found = True | ||
| break | ||
| elif isinstance(value, list): | ||
| if any(expected_domain in v for v in value if isinstance(v, str)): | ||
| found = True | ||
| break | ||
| if found: | ||
| break | ||
| if found: | ||
| break | ||
| assert found, "The connected notebook should import itables from unpkg.com" | ||
|
|
||
|
|
||
| def test_offline_notebook_is_not_too_large(tmp_path): |
|
Thank you for making this pull request. Did you know? You can try it on Binder: Also, the version of ITables developed in this PR is available as a wheel artifact 📦 for easy installation. |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (91.26%) is below the target coverage (93.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #442 +/- ##
==========================================
- Coverage 94.03% 93.98% -0.05%
==========================================
Files 43 43
Lines 1878 1880 +2
==========================================
+ Hits 1766 1767 +1
- Misses 112 113 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Relates to #441