mirror of
https://github.com/MacRimi/ProxMenux.git
synced 2026-11-06 12:52:05 +00:00
fix(monitor): serve Next.js static files only from their folder
The /_next route joined the requested name to its folder and served it with send_file, so a path with parent components could read a file outside it without signing in. It now uses send_from_directory, like the other static routes, and a regression test covers plain, encoded and absolute escapes on the three of them.
This commit is contained in:
@@ -32,6 +32,7 @@ on:
|
|||||||
- 'AppImage/scripts/flask_proxmenux_routes.py'
|
- 'AppImage/scripts/flask_proxmenux_routes.py'
|
||||||
- 'AppImage/scripts/guest_backup_archives.py'
|
- 'AppImage/scripts/guest_backup_archives.py'
|
||||||
- 'AppImage/scripts/flask_server.py'
|
- 'AppImage/scripts/flask_server.py'
|
||||||
|
- 'AppImage/scripts/tests/test_static_path_containment.py'
|
||||||
- 'AppImage/scripts/ovs_topology.py'
|
- 'AppImage/scripts/ovs_topology.py'
|
||||||
- 'AppImage/scripts/tests/test_ovs_topology.py'
|
- 'AppImage/scripts/tests/test_ovs_topology.py'
|
||||||
- 'AppImage/components/network-flow.tsx'
|
- 'AppImage/components/network-flow.tsx'
|
||||||
@@ -98,6 +99,7 @@ on:
|
|||||||
- 'AppImage/scripts/flask_proxmenux_routes.py'
|
- 'AppImage/scripts/flask_proxmenux_routes.py'
|
||||||
- 'AppImage/scripts/guest_backup_archives.py'
|
- 'AppImage/scripts/guest_backup_archives.py'
|
||||||
- 'AppImage/scripts/flask_server.py'
|
- 'AppImage/scripts/flask_server.py'
|
||||||
|
- 'AppImage/scripts/tests/test_static_path_containment.py'
|
||||||
- 'AppImage/scripts/ovs_topology.py'
|
- 'AppImage/scripts/ovs_topology.py'
|
||||||
- 'AppImage/scripts/tests/test_ovs_topology.py'
|
- 'AppImage/scripts/tests/test_ovs_topology.py'
|
||||||
- 'AppImage/components/network-flow.tsx'
|
- 'AppImage/components/network-flow.tsx'
|
||||||
@@ -189,6 +191,21 @@ jobs:
|
|||||||
NODE_PATH: ${{ github.workspace }}/.github/ci/offline-node/node_modules
|
NODE_PATH: ${{ github.workspace }}/.github/ci/offline-node/node_modules
|
||||||
run: node tests/test_backup_storage_retention.cjs
|
run: node tests/test_backup_storage_retention.cjs
|
||||||
|
|
||||||
|
static-files:
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
timeout-minutes: 5
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
with:
|
||||||
|
persist-credentials: false
|
||||||
|
- uses: actions/setup-python@v5
|
||||||
|
with:
|
||||||
|
python-version: '3.11'
|
||||||
|
- name: Install Flask
|
||||||
|
run: python -m pip install 'Flask==3.1.2'
|
||||||
|
- name: Test that the Monitor's own files never leave their folder
|
||||||
|
run: python -B -m unittest discover -s AppImage/scripts/tests -p test_static_path_containment.py -v
|
||||||
|
|
||||||
nvidia-installer:
|
nvidia-installer:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
timeout-minutes: 5
|
timeout-minutes: 5
|
||||||
|
|||||||
@@ -3850,14 +3850,8 @@ def serve_next_static(filename):
|
|||||||
appimage_root = os.path.dirname(base_dir)
|
appimage_root = os.path.dirname(base_dir)
|
||||||
|
|
||||||
static_dir = os.path.join(appimage_root, 'web', '_next')
|
static_dir = os.path.join(appimage_root, 'web', '_next')
|
||||||
file_path = os.path.join(static_dir, filename)
|
# send_from_directory refuses any path that leaves static_dir.
|
||||||
|
return send_from_directory(static_dir, filename)
|
||||||
if os.path.exists(file_path):
|
|
||||||
return send_file(file_path)
|
|
||||||
|
|
||||||
# print(f"[v0] ❌ Next.js static file not found: {file_path}")
|
|
||||||
pass
|
|
||||||
return '', 404
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
# print(f"Error serving Next.js static file {filename}: {e}")
|
# print(f"Error serving Next.js static file {filename}: {e}")
|
||||||
pass
|
pass
|
||||||
|
|||||||
@@ -0,0 +1,83 @@
|
|||||||
|
"""The routes that serve the Monitor's own files never return a file from
|
||||||
|
outside their folder, whatever the request path says."""
|
||||||
|
import ast
|
||||||
|
import os
|
||||||
|
from pathlib import Path
|
||||||
|
import tempfile
|
||||||
|
import unittest
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
from flask import Flask, jsonify, send_file, send_from_directory
|
||||||
|
|
||||||
|
SCRIPTS = Path(__file__).resolve().parents[1]
|
||||||
|
HANDLERS = {'serve_next_static', 'serve_static_files', 'serve_images'}
|
||||||
|
ESCAPES = (
|
||||||
|
'../../secret.txt', '..%2f..%2fsecret.txt', '..%2F..%2Fsecret.txt',
|
||||||
|
'%2e%2e/%2e%2e/secret.txt', '....//....//secret.txt', '/../../secret.txt',
|
||||||
|
'..\\..\\secret.txt', '%2e%2e%2f%2e%2e%2fsecret.txt',
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def app_with_handlers():
|
||||||
|
"""The real route functions of flask_server.py on a bare Flask app."""
|
||||||
|
tree = ast.parse((SCRIPTS / 'flask_server.py').read_text())
|
||||||
|
app = Flask(__name__)
|
||||||
|
namespace = {'os': os, 'app': app, 'send_file': send_file,
|
||||||
|
'send_from_directory': send_from_directory, 'jsonify': jsonify}
|
||||||
|
nodes = [n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name in HANDLERS]
|
||||||
|
if {n.name for n in nodes} != HANDLERS:
|
||||||
|
raise AssertionError('static handlers not found')
|
||||||
|
exec(compile(ast.Module(body=nodes, type_ignores=[]), 'flask_server.py', 'exec'), namespace)
|
||||||
|
for node in nodes:
|
||||||
|
for decorator in node.decorator_list:
|
||||||
|
rule = decorator.args[0].value
|
||||||
|
app.add_url_rule(rule, node.name, namespace[node.name])
|
||||||
|
return app
|
||||||
|
|
||||||
|
|
||||||
|
class StaticPathContainment(unittest.TestCase):
|
||||||
|
def setUp(self):
|
||||||
|
temp = tempfile.TemporaryDirectory()
|
||||||
|
self.addCleanup(temp.cleanup)
|
||||||
|
root = Path(temp.name)
|
||||||
|
self.appdir = root / 'appdir'
|
||||||
|
(self.appdir / 'web/_next/static').mkdir(parents=True)
|
||||||
|
(self.appdir / 'web/images').mkdir(parents=True)
|
||||||
|
(self.appdir / 'web/_next/static/app.js').write_text('next-asset')
|
||||||
|
(self.appdir / 'web/images/logo.png').write_text('image-asset')
|
||||||
|
(self.appdir / 'web/index.txt').write_text('web-asset')
|
||||||
|
# Harmless markers just outside each served folder and at the root.
|
||||||
|
for marker in (root / 'secret.txt', self.appdir / 'secret.txt', self.appdir / 'web/secret.txt'):
|
||||||
|
marker.write_text('outside-marker')
|
||||||
|
patcher = patch.dict(os.environ, {'APPDIR': str(self.appdir)})
|
||||||
|
patcher.start()
|
||||||
|
self.addCleanup(patcher.stop)
|
||||||
|
self.client = app_with_handlers().test_client()
|
||||||
|
|
||||||
|
def test_files_inside_the_folders_are_served(self):
|
||||||
|
for path, body in (('/_next/static/app.js', b'next-asset'), ('/images/logo.png', b'image-asset'),
|
||||||
|
('/index.txt', b'web-asset')):
|
||||||
|
with self.subTest(path=path):
|
||||||
|
response = self.client.get(path)
|
||||||
|
self.assertEqual((response.status_code, response.get_data()), (200, body))
|
||||||
|
response.close()
|
||||||
|
|
||||||
|
def test_no_path_leaves_the_folder(self):
|
||||||
|
for prefix in ('/_next/', '/images/', '/'):
|
||||||
|
for escape in ESCAPES:
|
||||||
|
if prefix == '/' and escape.startswith('/'):
|
||||||
|
continue # '//' would be read as a host by the test client
|
||||||
|
with self.subTest(path=prefix + escape):
|
||||||
|
response = self.client.get(prefix + escape)
|
||||||
|
self.assertNotIn(b'outside-marker', response.get_data())
|
||||||
|
self.assertNotEqual(response.status_code, 200)
|
||||||
|
response.close()
|
||||||
|
|
||||||
|
def test_an_absolute_path_is_not_served(self):
|
||||||
|
response = self.client.get('/_next/' + str(self.appdir / 'secret.txt'))
|
||||||
|
self.assertNotIn(b'outside-marker', response.get_data())
|
||||||
|
response.close()
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == '__main__':
|
||||||
|
unittest.main()
|
||||||
Reference in New Issue
Block a user