Proof: закрепить безопасные нативные координаты - #509
Closed
lemone112 wants to merge 9 commits into
Closed
Conversation
Collaborator
Author
|
@coderabbitai review Проверь exact head |
WalkthroughИзменения добавляют descriptor-relative проверки Docker CLI и cgroup с ChangesГраницы безопасности путей
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ArbController as SourceBoundArbControllerV1
participant DockerTransport as docker_command_coordinate_v1
participant LinuxExecutor as canonical_cgroup_parent_v1
participant Cgroup as enter_observer_cgroup_v1
ArbController->>DockerTransport: validate and store Docker coordinate
ArbController->>LinuxExecutor: canonicalize cgroup parent
ArbController->>Cgroup: enter observer cgroup during execution
Cgroup-->>ArbController: success or typed placement failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Commit identity =='
git show -s --format='commit=%H%nparents=%P%nsubject=%s' b950f3e
echo
echo '== Changed paths =='
git diff-tree --no-commit-id --name-status -r b950f3e
echo
echo '== Diff stat =='
git diff --stat b950f3e^ b950f3e
echo
echo '== Diff =='
git diff --find-renames --find-copies --no-ext-diff b950f3e^ b950f3e
echo
echo '== Candidate symbols and module references =='
rg -n -C 3 --hidden \
-g '!**/.git/**' \
-e '\benter_observer_cgroup_v1\b' \
-e '\bO_NOFOLLOW\b' \
-e '\bobserver[_ -]?cgroup\b' \
-e '\bArb\b|\bMPFI\b|\breceipt\b|\bRUN\b' \
.Length of output: 180830 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== executor import surface =='
sed -n '1,105p' proof/region/v1/executor.py
echo
echo '== executor placement implementation and adjacent cgroup code =='
sed -n '1375,1460p' proof/region/v1/executor.py
echo
echo '== receipt imports and complete execute ordering =='
sed -n '1,85p' proof/region/v1/arb/receipt.py
sed -n '590,735p' proof/region/v1/arb/receipt.py
echo
echo '== Structural definitions and direct call sites =='
ast-grep run --lang python --pattern 'def enter_observer_cgroup_v1($$$): $$$' proof/region/v1
ast-grep run --lang python --pattern '$_MODULE.enter_observer_cgroup_v1($$$)' proof/region/v1
echo
echo '== Duplicate protocol operations outside executor =='
rg -n -C 2 --glob '*.py' \
-e 'def\s+_?enter_observer_cgroup_v1\b' \
-e 'parent\s*/\s*["'\'']observer["'\'']' \
-e 'b?["'\'']cgroup\.procs["'\'']' \
proof/region/v1 \
| grep -vE '^(proof/region/v1/executor\.py|proof/region/v1/tests/|proof/region/v1/arb/tests/)' \
|| trueLength of output: 16382 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from __future__ import annotations
import ast
from pathlib import Path
root = Path("proof/region/v1")
for path in sorted(root.rglob("*.py")):
if "tests" in path.parts:
continue
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == "enter_observer_cgroup_v1":
print(f"DEFINITION {path}:{node.lineno}")
if (
isinstance(node, ast.Call)
and isinstance(node.func, ast.Attribute)
and node.func.attr == "enter_observer_cgroup_v1"
):
receiver = (
node.func.value.id
if isinstance(node.func.value, ast.Name)
else ast.unparse(node.func.value)
)
print(f"CALL {path}:{node.lineno} receiver={receiver}")
if (
isinstance(node, ast.Call)
and isinstance(node.func, ast.Attribute)
and node.func.attr == "open"
and any(
isinstance(argument, ast.Constant) and argument.value == b"cgroup.procs"
for argument in node.args
)
):
print(f"CGROUP_PROCS_OPEN {path}:{node.lineno}")
PY
echo
echo '== Executor engine-neutrality tokens, excluding tests and workflows =='
rg -n -i --glob '*.py' \
-e '\barb\b' -e '\bmpfi\b' -e '\breceipt\b' -e '\bformula\b' -e '\bcomparator\b' \
proof/region/v1/executor.py \
|| true
echo
echo '== Placement test body =='
sed -n '1500,1582p' proof/region/v1/tests/test_executor.pyLength of output: 5605
Замечаний в указанной области нет.
Я выполнил только статический анализ. Я не запускал тесты, Actions или runner infrastructure. ✅ Action performedReview finished.
|
lemone112
force-pushed
the
agent/mpfi-observer-placement
branch
from
August 1, 2026 08:02
b950f3e to
c63b3d5
Compare
Collaborator
Author
|
@coderabbitai review Stack rebased safely onto #508; exact new head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head_commit="$(git rev-parse c63b3d5)"
parent_commit="$(git rev-parse "${head_commit}^")"
echo "== Commit identity =="
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$head_commit"
echo
echo "== Changed paths =="
git diff-tree --no-commit-id --name-status -r "$head_commit"
echo
echo "== Focused diff: observer placement and inventory/reference pins =="
git diff --find-renames --find-copies --no-ext-diff "$parent_commit" "$head_commit" -- \
'proof/region/v1/executor.py' \
'proof/region/v1/arb/receipt.py' \
'proof/region/v1/reference.py' \
'proof/region/v1/**/inventory*' \
'proof/region/v1/**/manifest*' \
'proof/region/v1/**/test*' \
|| true
echo
echo "== Locate candidate source files =="
fd -t f -e py . proof/region/v1 | sort
echo
echo "== Definitions, call sites, and cgroup protocol operations outside tests =="
python3 - <<'PY'
import ast
from pathlib import Path
root = Path("proof/region/v1")
for path in sorted(root.rglob("*.py")):
if "tests" in path.parts:
continue
try:
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
except (OSError, SyntaxError) as exc:
print(f"PARSE_ERROR {path}: {exc}")
continue
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == "enter_observer_cgroup_v1":
print(f"DEFINITION {path}:{node.lineno}")
elif isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute):
if node.func.attr == "enter_observer_cgroup_v1":
receiver = ast.unparse(node.func.value)
print(f"CALL {path}:{node.lineno} receiver={receiver}")
if node.func.attr == "open" and any(
isinstance(arg, ast.Constant) and arg.value == b"cgroup.procs"
for arg in node.args
):
print(f"CGROUP_PROCS_OPEN {path}:{node.lineno}")
PY
echo
echo "== Engine-semantic imports/references in executor =="
rg -n -i -C 2 --glob 'executor.py' \
'\b(arb|mpfi|receipt|formula|comparator|run backend)\b' \
proof/region/v1 || true
echo
echo "== Reference and inventory pin candidates =="
rg -n -C 3 --glob '*.py' --glob '*.md' --glob '*.txt' \
'\benter_observer_cgroup_v1\b|\bobserver cgroup\b|\bobserver_cgroup\b|\bO_NOFOLLOW\b' \
proof/region/v1 \
| head -n 1200Length of output: 35380 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Gate inventory algorithm and pins =="
sed -n '1,180p' proof/region/v1/arb/tests/gate.py
echo
echo "== Shared build inventory pin validation =="
sed -n '1,125p' proof/region/v1/tests/test_build.py
echo
echo "== Arb imports and placement-to-RUN ordering =="
sed -n '1,90p' proof/region/v1/arb/receipt.py
sed -n '630,690p' proof/region/v1/arb/receipt.py
echo
echo "== Protocol ownership claim =="
sed -n '205,232p' proof/region/v1/PROTOCOL.md
echo
echo "== Independently calculate Arb test inventory and order candidates =="
python3 - <<'PY'
from __future__ import annotations
import ast
import hashlib
from pathlib import Path
test_dir = Path("proof/region/v1/arb/tests")
tests = sorted(
path.relative_to(test_dir).as_posix()
for path in test_dir.glob("test_*.py")
)
print("TEST_FILES")
print("\n".join(tests))
print("FILE_INVENTORY_SHA256", hashlib.sha256("\n".join(tests).encode()).hexdigest())
# Extract unittest-style methods statically. Do not import or execute project code.
methods: list[str] = []
for path in sorted(test_dir.glob("test_*.py")):
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
for cls in (node for node in tree.body if isinstance(node, ast.ClassDef)):
if any(
(isinstance(base, ast.Attribute) and base.attr == "TestCase")
or (isinstance(base, ast.Name) and base.id == "TestCase")
for base in cls.bases
):
for item in cls.body:
if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) and item.name.startswith("test"):
methods.append(f"{path.name}:{cls.name}.{item.name}")
methods.sort()
payload = "\n".join(methods)
print("STATIC_TEST_METHOD_COUNT", len(methods))
print("STATIC_TEST_METHOD_ORDER_SHA256", hashlib.sha256(payload.encode()).hexdigest())
print("STATIC_TEST_METHOD_INVENTORY_SHA256", hashlib.sha256("\n".join(methods).encode()).hexdigest())
PY
echo
echo "== Cross-module imports from executor, excluding tests =="
rg -n -C 2 --glob '*.py' \
'^\s*(from|import)\s+.*\b(arb|receipt|mpfi)\b' \
proof/region/v1/executor.py || trueLength of output: 19080 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Outer-oracle inventory assertions =="
rg -n -A18 -B8 \
'ARB_(INVENTORY_SHA256_V1|ORDER_SHA256_V1|TEST_COUNT_V1)|test_inventory_sha256_v1|full_suite_v1' \
proof/region/v1/tests/test_build.py
echo
echo "== Static reconstruction of gate test IDs =="
python3 - <<'PY'
from __future__ import annotations
import ast
import hashlib
from pathlib import Path
sources = [Path("proof/region/v1/tests/test_executor.py")]
sources.extend(sorted(Path("proof/region/v1/arb/tests").glob("test_*.py")))
def dotted_module(path: Path) -> str:
return path.stem
def test_case_classes(tree: ast.Module) -> set[str]:
classes = {
node.name: node
for node in ast.walk(tree)
if isinstance(node, ast.ClassDef)
}
derived: set[str] = set()
changed = True
while changed:
changed = False
for name, node in classes.items():
if name in derived:
continue
bases = {
base.id for base in node.bases if isinstance(base, ast.Name)
} | {
base.attr for base in node.bases if isinstance(base, ast.Attribute)
}
if "TestCase" in bases or bases & derived:
derived.add(name)
changed = True
return derived
test_ids: list[str] = []
for path in sources:
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
classes = {
node.name: node
for node in ast.walk(tree)
if isinstance(node, ast.ClassDef)
}
for class_name in sorted(test_case_classes(tree)):
node = classes[class_name]
for member in node.body:
if isinstance(member, (ast.FunctionDef, ast.AsyncFunctionDef)) and member.name.startswith("test"):
test_ids.append(f"{dotted_module(path)}.{class_name}.{member.name}")
test_ids.sort()
preimage = b"".join(test_id.encode("utf-8") + b"\n" for test_id in test_ids)
print("COUNT", len(test_ids))
print("INVENTORY_SHA256", hashlib.sha256(preimage).hexdigest())
print("ORDER_SHA256", hashlib.sha256("\n".join(test_ids).encode("utf-8")).hexdigest())
print("FIRST_IDS")
print("\n".join(test_ids[:8]))
print("LAST_IDS")
print("\n".join(test_ids[-8:]))
PY
echo
echo "== Exact pin literals =="
rg -n -C 2 \
'2571be2b0998ed9347fd2f51efd5eeedcd8283ccf8f0bc9f24817d62c9505653|8f74e87ece46cb8411f0cb1f545dd7feb31bb00294f1d4f7fa6bf2c386c57e33|ARB_TEST_COUNT_V1 = 172' \
proof/region/v1/arb/tests/gate.py proof/region/v1/tests/test_build.pyLength of output: 8650 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from __future__ import annotations
import ast
import hashlib
from pathlib import Path
sources = [Path("proof/region/v1/tests/test_executor.py")]
sources.extend(sorted(Path("proof/region/v1/arb/tests").glob("test_*.py")))
def module_test_case_classes(tree: ast.Module) -> dict[str, ast.ClassDef]:
classes = {
node.name: node for node in tree.body if isinstance(node, ast.ClassDef)
}
derived: set[str] = set()
changed = True
while changed:
changed = False
for name, node in classes.items():
if name in derived:
continue
bases = {
base.id for base in node.bases if isinstance(base, ast.Name)
} | {
base.attr for base in node.bases if isinstance(base, ast.Attribute)
}
if "TestCase" in bases or bases & derived:
derived.add(name)
changed = True
return {name: classes[name] for name in derived}
test_ids: list[str] = []
nested_candidates: list[str] = []
for path in sources:
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
module_classes = module_test_case_classes(tree)
for class_name, node in module_classes.items():
for member in node.body:
if isinstance(member, (ast.FunctionDef, ast.AsyncFunctionDef)) and member.name.startswith("test"):
test_ids.append(f"{path.stem}.{class_name}.{member.name}")
for node in ast.walk(tree):
if isinstance(node, ast.ClassDef) and node not in module_classes.values():
for member in node.body:
if isinstance(member, (ast.FunctionDef, ast.AsyncFunctionDef)) and member.name.startswith("test"):
nested_candidates.append(f"{path}:{node.name}.{member.name}")
test_ids.sort()
sorted_preimage = b"".join(item.encode("utf-8") + b"\n" for item in test_ids)
# unittest discovery is lexicographic by method name within each class, then
# module attributes are traversed in sorted name order. This is modeled here.
ordered_ids: list[str] = []
for path in sources:
for class_name in sorted(module_test_case_classes(
ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
)):
node = module_test_case_classes(
ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
)[class_name]
for member in sorted(
(
item for item in node.body
if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef))
and item.name.startswith("test")
),
key=lambda item: item.name,
):
ordered_ids.append(f"{path.stem}.{class_name}.{member.name}")
print("MODULE_LEVEL_COUNT", len(test_ids))
print("SORTED_INVENTORY_SHA256", hashlib.sha256(sorted_preimage).hexdigest())
print("DISCOVERY_ORDER_SHA256", hashlib.sha256(
b"".join(item.encode("utf-8") + b"\n" for item in ordered_ids)
).hexdigest())
print("NESTED_EXCLUDED")
print("\n".join(sorted(nested_candidates)) or "(none)")
PYLength of output: 3707
Замечаний в указанной области нет.
Я выполнил только статический анализ. Я не запускал тесты, Actions или runner infrastructure. ✅ Action performedReview finished.
|
lemone112
force-pushed
the
agent/mpfi-observer-placement
branch
from
August 1, 2026 12:23
c63b3d5 to
abb8a02
Compare
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@proof/region/v1/arb/tests/test_receipt.py`:
- Around line 304-322: Исправьте тест
test_controller_rejects_hostile_native_coordinates_on_construction: поскольку
SourceBoundArbControllerV1.__init__ отклоняет подклассы по точному типу до
вызова path-методов, переименуйте ExplodingPath и ExplodingFilesystemPath в
названия, отражающие проверку отказа подкласса, например PathSubclass; либо
перенесите проверку в прямые вызовы build_transport.docker_command_coordinate_v1
и executor.canonical_cgroup_parent_v1, если требуется тестировать враждебный
path-протокол.
In `@proof/region/v1/build/transport.py`:
- Around line 1106-1115: Update _open_docker_command_v1 to accept the already
validated command coordinate rather than a Path, and encode only that
coordinate’s path without calling docker_command_coordinate_v1 again. Update the
caller around self._command_coordinate to pass the coordinate object directly,
preserving the existing probe behavior and validation source.
In `@proof/region/v1/executor.py`:
- Around line 1491-1502: Update enter_observer_cgroup_v1 to construct both
OSError instances with explicit errno values, using the appropriate errno_module
code for the non-directory validation and short cgroup placement write,
consistent with the module’s existing cgroup error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f048bde2-7330-4092-a509-4192f74af317
📒 Files selected for processing (9)
proof/region/v1/PROTOCOL.mdproof/region/v1/arb/receipt.pyproof/region/v1/arb/tests/gate.pyproof/region/v1/arb/tests/test_receipt.pyproof/region/v1/arb/tests/test_transport.pyproof/region/v1/build/transport.pyproof/region/v1/executor.pyproof/region/v1/tests/test_build.pyproof/region/v1/tests/test_executor.py
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
lemone112
force-pushed
the
agent/build-transport
branch
from
August 1, 2026 12:55
7a82231 to
284ee16
Compare
lemone112
force-pushed
the
agent/mpfi-observer-placement
branch
from
August 1, 2026 13:32
d3b1533 to
de5b427
Compare
lemone112
changed the title
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@proof/region/v1/tests/test_executor.py`:
- Around line 1799-1931: Разделите сценарии из
test_observer_cgroup_placement_is_exact_and_fail_closed на отдельные тестовые
методы или независимые subTest-блоки: успешное размещение, недопустимые типы,
hostile path-оператор, symlink parent, symlink-алиас компонента, symlink
observer, symlink cgroup.procs и короткая запись. Сохраните существующие
проверки и очистку ресурсов, но обеспечьте изоляцию каждого сценария, чтобы
падение одного не скрывало результаты остальных.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 51b3f029-f7f4-4475-82fd-65ad58a36b78
📒 Files selected for processing (9)
proof/region/v1/PROTOCOL.mdproof/region/v1/arb/receipt.pyproof/region/v1/arb/tests/gate.pyproof/region/v1/arb/tests/test_receipt.pyproof/region/v1/arb/tests/test_transport.pyproof/region/v1/build/transport.pyproof/region/v1/executor.pyproof/region/v1/tests/test_build.pyproof/region/v1/tests/test_executor.py
Comment on lines
+1799
to
+1931
| def test_observer_cgroup_placement_is_exact_and_fail_closed(self) -> None: | ||
| with tempfile.TemporaryDirectory() as temporary: | ||
| root = Path(temporary).resolve() | ||
| parent = root / "proof" | ||
| observer = parent / "observer" | ||
| observer.mkdir(parents=True) | ||
| procs = observer / "cgroup.procs" | ||
| procs.write_bytes(b"") | ||
|
|
||
| parent.chmod(0o111) | ||
| observer.chmod(0o111) | ||
| procs.chmod(0o222) | ||
| try: | ||
| executor.enter_observer_cgroup_v1(parent) | ||
| finally: | ||
| procs.chmod(0o644) | ||
| observer.chmod(0o755) | ||
| parent.chmod(0o755) | ||
|
|
||
| self.assertEqual(procs.read_bytes(), str(os.getpid()).encode("ascii")) | ||
|
|
||
| for invalid in ( | ||
| object(), | ||
| Path("relative"), | ||
| Path("/absolute\0"), | ||
| Path("/absolute\ud800"), | ||
| "/absolute", | ||
| ): | ||
| with self.subTest(invalid=invalid): | ||
| with self.assertRaises(TypeError): | ||
| executor.enter_observer_cgroup_v1(invalid) # type: ignore[arg-type] | ||
|
|
||
| class ExplodingPath(type(Path())): | ||
| def __truediv__(self, _other: object) -> Path: | ||
| raise RuntimeError("hostile path operator") | ||
|
|
||
| with tempfile.TemporaryDirectory() as temporary: | ||
| root = Path(temporary).resolve() | ||
| parent = root / "proof" | ||
| observer = parent / "observer" | ||
| observer.mkdir(parents=True) | ||
| procs = observer / "cgroup.procs" | ||
| procs.write_bytes(b"") | ||
|
|
||
| executor.enter_observer_cgroup_v1(ExplodingPath(str(parent))) | ||
|
|
||
| self.assertEqual(procs.read_bytes(), str(os.getpid()).encode("ascii")) | ||
|
|
||
| with tempfile.TemporaryDirectory() as temporary: | ||
| root = Path(temporary).resolve() | ||
| parent = root / "proof" | ||
| target = root / "target" | ||
| observer = target / "observer" | ||
| observer.mkdir(parents=True) | ||
| procs = observer / "cgroup.procs" | ||
| procs.write_bytes(b"") | ||
| parent.symlink_to(target, target_is_directory=True) | ||
|
|
||
| with self.assertRaises(OSError): | ||
| executor.enter_observer_cgroup_v1(parent) | ||
|
|
||
| self.assertEqual(procs.read_bytes(), b"") | ||
|
|
||
| with tempfile.TemporaryDirectory() as temporary: | ||
| root = Path(temporary).resolve() | ||
| target = root / "target" | ||
| parent = target / "proof" | ||
| observer = parent / "observer" | ||
| observer.mkdir(parents=True) | ||
| procs = observer / "cgroup.procs" | ||
| procs.write_bytes(b"") | ||
| alias = root / "alias" | ||
| alias.symlink_to(target, target_is_directory=True) | ||
|
|
||
| with self.assertRaises(OSError): | ||
| executor.enter_observer_cgroup_v1(alias / "proof") | ||
|
|
||
| self.assertEqual(procs.read_bytes(), b"") | ||
|
|
||
| with tempfile.TemporaryDirectory() as temporary: | ||
| root = Path(temporary).resolve() | ||
| parent = root / "proof" | ||
| target = root / "target" | ||
| parent.mkdir() | ||
| target.mkdir() | ||
| (target / "cgroup.procs").write_bytes(b"") | ||
| (parent / "observer").symlink_to(target, target_is_directory=True) | ||
|
|
||
| with self.assertRaises(OSError): | ||
| executor.enter_observer_cgroup_v1(parent) | ||
|
|
||
| with tempfile.TemporaryDirectory() as temporary: | ||
| root = Path(temporary).resolve() | ||
| parent = root / "proof" | ||
| observer = parent / "observer" | ||
| observer.mkdir(parents=True) | ||
| target = root / "foreign-procs" | ||
| target.write_bytes(b"") | ||
| (observer / "cgroup.procs").symlink_to(target) | ||
|
|
||
| with self.assertRaises(OSError): | ||
| executor.enter_observer_cgroup_v1(parent) | ||
|
|
||
| with tempfile.TemporaryDirectory() as temporary: | ||
| root = Path(temporary).resolve() | ||
| parent = root / "proof" | ||
| observer = parent / "observer" | ||
| observer.mkdir(parents=True) | ||
| (observer / "cgroup.procs").write_bytes(b"") | ||
| opened: list[int] = [] | ||
| real_open = os.open | ||
|
|
||
| def track_open(*args: object, **kwargs: object) -> int: | ||
| descriptor = real_open(*args, **kwargs) | ||
| opened.append(descriptor) | ||
| return descriptor | ||
|
|
||
| with mock.patch.object( | ||
| executor.os, | ||
| "open", | ||
| side_effect=track_open, | ||
| ), mock.patch.object(executor.os, "write", return_value=0): | ||
| with self.assertRaises(OSError) as caught: | ||
| executor.enter_observer_cgroup_v1(parent) | ||
| self.assertEqual(caught.exception.errno, errno.EIO) | ||
|
|
||
| self.assertEqual(len(opened), len(parent.parts) + 2) | ||
| for descriptor in opened: | ||
| with self.subTest(descriptor=descriptor): | ||
| with self.assertRaises(OSError) as caught: | ||
| os.fstat(descriptor) | ||
| self.assertEqual(caught.exception.errno, errno.EBADF) | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Разделите тест размещения observer на отдельные тесты.
Метод содержит семь независимых сценариев: успешное размещение, отказ по типу, hostile path-оператор, symlink на parent, symlink-алиас компонента, symlink на observer, symlink на cgroup.procs и короткая запись. Первый упавший assert скрывает остальные сценарии. Выделите каждый сценарий в отдельный тест или в subTest, чтобы отчёт указывал точную причину.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 1809-1809: chmod sets a world-writable permission bit; restrict the mode so other users cannot modify the file (e.g. 0o644 / 0o600).
Context: procs.chmod(0o222)
Note: [CWE-276] Incorrect Default Permissions.
(world-writable-chmod-python)
🪛 Ruff (0.16.0)
[warning] 1799-1799: Too many statements (96 > 50)
(PLR0915)
[warning] 1827-1828: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
[warning] 1828-1828: Use pytest.raises instead of unittest-style assertRaises
Replace assertRaises with pytest.raises
(PT027)
[warning] 1833-1833: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 1857-1857: Use pytest.raises instead of unittest-style assertRaises
Replace assertRaises with pytest.raises
(PT027)
[warning] 1873-1873: Use pytest.raises instead of unittest-style assertRaises
Replace assertRaises with pytest.raises
(PT027)
[warning] 1887-1887: Use pytest.raises instead of unittest-style assertRaises
Replace assertRaises with pytest.raises
(PT027)
[warning] 1899-1899: Use pytest.raises instead of unittest-style assertRaises
Replace assertRaises with pytest.raises
(PT027)
[warning] 1916-1921: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
[warning] 1921-1921: Use pytest.raises instead of unittest-style assertRaises
Replace assertRaises with pytest.raises
(PT027)
[warning] 1928-1928: Use pytest.raises instead of unittest-style assertRaises
Replace assertRaises with pytest.raises
(PT027)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@proof/region/v1/tests/test_executor.py` around lines 1799 - 1931, Разделите
сценарии из test_observer_cgroup_placement_is_exact_and_fail_closed на отдельные
тестовые методы или независимые subTest-блоки: успешное размещение, недопустимые
типы, hostile path-оператор, symlink parent, symlink-алиас компонента, symlink
observer, symlink cgroup.procs и короткая запись. Сохраните существующие
проверки и очистку ресурсов, но обеспечьте изоляцию каждого сценария, чтобы
падение одного не скрывало результаты остальных.
Source: Linters/SAST tools
lemone112
force-pushed
the
agent/mpfi-observer-placement
branch
from
August 1, 2026 14:04
de5b427 to
2ff10ee
Compare
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…t/mpfi-observer-placement # Conflicts: # proof/region/v1/tests/test_build.py
This was referenced Aug 2, 2026
Collaborator
Author
|
Этот sibling-срез поглощён терминальным PR #514 (base |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Что меняется
O_PATHиO_NOFOLLOWна каждом сегменте пути.O_PATHне получает тихий fallback: probe возвращает typed unsupported path.Причина
O_RDONLYтребовал право чтения там, где нативная preflight-проверка читает только metadata. Это неверно отклоняло корректную executable-only/search-only координату и не соответствовало заявленной семантике точного descriptor-relative path.Границы
Docker capability V1 связывает pathname, а не неизменный file object между probe и BUILD. CLI и daemon остаются явной unsealed host boundary. Этот PR не заявляет выполненный Linux/VM integration gate.
Проверка на текущей голове
O_PATH=None/0.O_PATHс каждым фактическимopen; намеренные мутации кO_RDONLYи hard-codedO_EXECделают оба пути красными независимо от UID.-O: 193 tests, exact 11-skip manifest.test_receipt.pynormal и-O: 19 tests, 1 declared external-integration skip.test_build.pynormal и-O: 45 tests.-O: 156 tests, 1 declared external-integration skip.git show --checkчист.Stacked on
agent/build-transport(#508); merge только после parent chain и отдельного external VM gate.Join contract
Этот срез намеренно не является предком #510/#511. Перед началом M2b successor обязан явно объединить этот head с завершённым #511 и повторно прогнать общий executor/receipt gate; #511 сам по себе не содержит O_PATH/native execution hardening из этого PR.