Repository navigation
test(send_allowlist): 16 behavioral tests for is_path_sendable - #1137
Closed
bassilkhilo-ag2 wants to merge 3 commits into
Closed
bassilkhilo-ag2 wants to merge 3 commits into
bassilkhilo-ag2 wants to merge 3 commits into
Conversation
Covers: missing results/ no-op, mtime cutoff (archive old, skip fresh),
non-.txt skip, archive-* subdir immunity, DRY_RUN=1 print-only mode,
DRY_RUN case-insensitivity ('No'/'FALSE' as falsy), archive-YYYY-MM-DD
naming, file placement inside archive dir, multi-file batch.
Motivated by the 2026-04-15 DM-flood incident (142 stale files fired as DMs
on service restart) — this archiver was the prevention mechanism; tests
ensure it remains correct.
12/12 tests passing.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- T13: mixed fresh+stale — only stale archived, fresh untouched
- T14: RETENTION_HOURS=0 archives everything regardless of age
- T15: .sending files (orphan proactive-delivery claims) are not
archived — only .txt suffix is in scope
Total: 15/15 passing.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Companion to send-allowlist-shared.test.py (architectural invariants). This suite exercises the runtime security properties that matter most: - Symlink-traversal: /tmp/sutando-<link>→outside is rejected via realpath - Path traversal: ../../../etc/passwd neutralised by realpath collapse - Intra-root escape: symlink within results/→outside results/ rejected - Fail-closed: nonexistent paths, directories, empty string all False - Trailing-sep guard: results-extra/ not matched by results/ root - Prefix correctness: /tmp/sutando-* and /tmp/echo-* accepted; others not No discord.py or workspace mutation — all root-based tests use monkeypatching of SEND_ALLOWED_ROOTS with tempdir fixtures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
liususan091219
left a comment
liususan091219
left a comment
Collaborator
There was a problem hiding this comment.
NON-BLOCKING — LGTM (bottom of stack)
Clean PR — adds tests/send-allowlist-is-path-sendable.test.py (+355, 16 tests for is_path_sendable). The other test file in the diff (archive-stale-results.test.py) is base from #1136. No build_log.md leak, no source edits.
This is the bottom of a 6-PR stack (#1137 → #1142). Each adds exactly one new test file on top of the previous. Stack pattern looks correct.
Deferring the merge call to Susan.
This was referenced May 26, 2026
VasiliyRad
approved these changes
May 26, 2026
VasiliyRad
left a comment
VasiliyRad
left a comment
Contributor
There was a problem hiding this comment.
All 16 assertions verified against the is_path_sendable source. Test-by-test:
- T1-T3: fail-closed (nonexistent, empty, directory) — correct, all hit
os.path.isfileguard. - T4-T6: prefix allow/reject — correct, realpath on macOS resolves
/tmp→/private/tmpand the prefixes list covers both forms. - T7-T9: symlink traversal + path-traversal
..— correct, realpath collapses before the check runs. - T10-T12: root-based matching via
_with_patched_rootsmonkeypatch — clean isolation, assertions matchstartswith(root_real + os.sep)logic. - T13-T14: substring/sibling false-prefix guards — correct, validates the trailing-sep invariant.
- T15-T16: intra-root symlink accept vs escape-symlink reject — correct.
No non-test files modified. No false-positive risks. LGTM.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
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
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.
Summary
tests/send-allowlist-is-path-sendable.test.py— behavioral coverage forsrc/send_allowlist.py::is_path_sendablesend-allowlist-shared.test.py(architectural invariants); that suite only checks imports and constant presence, never the function's runtime behaviorSecurity properties tested (16 tests)
/tmp/sutando-*and/tmp/echo-*accepted/tmp/notallowed-*rejected/etc/passwdrejected via realpath collapse/tmp/sutando-<link>→ outside area: name passes, realpath defeats it../../../etc/passwdneutralised by realpath/tmp/notallowed-*/with "sutando" in nameresults-extra/not matched byresults/rootTest plan
python3 tests/send-allowlist-is-path-sendable.test.py→ 16/16 PASSSEND_ALLOWED_ROOTSwith tempdir fixtures; no real workspace mutations🤖 Generated with Claude Code