You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test(artifacts): gate the protected-file refusal test on security-module - #8561
Stacked on #8560. Merge #8560 first; this PR then carries one commit (6786d69b07). Draft until #8560 lands.
Summary
artifacts::tiny_store::tests::…::a_protected_file_inside_the_working_folder_is_refused fails in a build without security-module. It asserts TinySecurity's protected-path floor, and that floor does not exist when security is disabled.
This PR gates the test on security-module, like its sibling the_guarded_roots_refuse_to_read_a_protected_file.
Base: stacked on #8560 (the tiny_store_tests.rs split), because the test lives in that PR's new tiny_store_guard_tests.rs. On main it is still in tiny_store_tests.rs, so a PR off main would conflict with #8560. Merge #8560 first; this then carries one commit.
Problem
Per CLAUDE.md ("With security disabled"): a build without security-module has no path policy at all, and file tools reach ~/.ssh like any other path.
So in that build the artifact create succeeds, and the test's unwrap_err() panics on Ok(Saved { … }). This is identical on main 6043171.
tiny_store_tests.rs had grown to 784 lines, over rust:layout's 750-line
limit. The trailing file-root and protected-file tests move unchanged into
tiny_store_guard_tests.rs, a child module of the same tests module, so they
keep the shared helpers through super::*.
a_protected_file_inside_the_working_folder_is_refused asserts TinySecurity's
protected-path floor. Without security-module, security is disabled and
there is no floor, so the create succeeds and the test failed in that
build. It now runs only where the floor exists, like its sibling
the_guarded_roots_refuse_to_read_a_protected_file.
No actionable comments were generated in the recent review.
Walkthrough
The pull request moves artifact file-guard tests into a dedicated test module and registers it in the existing test module. The tests cover protected-file handling, trusted file roots, artifact revision, and stored file-key resolution.
Changes: Artifact file-guard tests
Layer / File(s)
Summary
Guard test coverage crates/openhuman-core/src/artifacts/tiny_store_guard_tests.rs, crates/openhuman-core/src/artifacts/tiny_store_tests.rs
The dedicated module contains tests for protected-file rejection, guarded-root reads, trusted-root selection, artifact revision, and stored file-key resolution. The existing test module registers the new module and removes the prior test definitions.
tinyhumansai/openhuman#8560: Moves guarded-file tests without changing their bodies; this PR includes the test relocation.
tinyhumansai/openhuman#7888: Introduces the versioned artifact service and file-root integration, including artifact revision tests moved here.
tinyhumansai/openhuman#8115: Adds stored_file_path and a test for relative and absolute FileRef keys, which this PR moves into the guarded-file test module.
Suggested labels:rust-core
Suggested reviewers:senamakel
Merge Risk: Minimal · up to 6786d
The change preserves the moved tests and limits the protected-path assertion to builds that provide that protection. No actionable merge-blocking issue remains, subject to normal checks.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly describes the feature gate added to the protected-file refusal test.
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks each guarded way, And tests where trusted files may stay. A key path lands beneath its home, While absolute paths still roam. The test files hop to a new burrow.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: dae87b53-2f05-4797-9d5b-3841b3494127
Commits
Reviewing files that changed from the base of the PR and between 6043171 and 6786d69.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
Verdict: Ready for maintainer review · Priority: none Coverage: Reviewed 2 of 2 files.
Pull request purpose (title): test(artifacts): gat…
Diagnostics and supporting details
Full summary
Pull request purpose (title): test(artifacts): gate the protected-file refusal test on security-module. This PR gates the artifacts protected-file refusal test on the `security-module` feature, matching its sibling test, because TinySecurity's protected-path floor does not exist when security is disabled. The change is test-only: it moves five existing tests into a new `tiny_store_guard_tests.rs` module declared via `#[path]` from `tiny_store_tests.rs`, and widens `a_protected_file_inside_the_working_folder_is_refused` to `#[cfg(feature = "security-module")]` with an explanatory comment. No push delta was available for review.
Review snapshot
Change surface
Files
Review signal
Count
Production
0
Active findings
0
Tests
2
Noted findings
0
Documentation
0
Resolved findings
0
Configuration
0
Pending checks/questions
0
Completeness: Complete Test assessment: Test coverage is assessed from changed tests and lane evidence; execution is not claimed without trusted check data.
What changed
The pull-request diff adds `crates/openhuman-core/src/artifacts/tiny_store_guard_tests.rs` containing five tests relocated from `tiny_store_tests.rs`, and registers it in `crates/openhuman-core/src/artifacts/tiny_store_tests.rs` via `#[path = "tiny_store_guard_tests.rs"] mod guard_tests;`. The protected-file refusal test now carries `#[cfg(feature = "security-module")]`, joined by a comment explaining that the refusal is TinySecurity's protected-path floor and that a build without the security module has no floor. Test bodies and assertions are otherwise carried over unchanged, so there is no runtime behavior change.
Features
Internal refactor — Guard tests split into a dedicated sibling test module: Five existing tiny_store tests (create/revise under an earlier trusted files folder, protected-path refusal at create, guarded-root read refusal, working-folder containment, stored-key resolution) now live in `tiny_store_guard_tests.rs`, declared from `tiny_store_tests.rs` with `#[path]` and beginning with `use super::*;`, matching the repository's sibling-test-file layout rule. Assertions are preserved. (crates/openhuman-core/src/artifacts/tiny_store_guard_tests.rs, crates/openhuman-core/src/artifacts/tiny_store_tests.rsuse tiny_artifacts::{)
Tests
moved_unit_tests — The four other relocated tests continue to exercise create/revise against an earlier trusted files folder, refusal to read a protected file via guarded roots while still reading an ordinary file, non-trust of a working folder containing the workspace, and stored-key resolution under the version files folder versus an absolute key.: The tests lane states the test bodies are unchanged apart from the widened attribute and that they still exercise these paths; no findings were raised. (crates/openhuman-core/src/artifacts/tiny_store_guard_tests.rs, crates/openhuman-core/src/artifacts/tiny_store_tests.rs)
Findings
No active actionable findings.
Before merge
None.
Agent review details
critique
Conclusion: Success
Scope reviewed: all assigned evidence
Positive: Critique lane answered, covering both changed files, with zero findings.
Lane summary: Reviewed 2 files; 0 findings. _The code index is behind this pull request (indexed at `90c5199a04d5`), so retrieved context may be out of date._
security
Conclusion: Success
Scope reviewed: all assigned evidence
Positive: Security lane answered with zero findings; the change is test-only and does not alter the protected-path enforcement itself.
Lane summary: Reviewed 2 files; 0 findings. _The code index is behind this pull request (indexed at `90c5199a04d5`), so retrieved context may be out of date._
tests
Conclusion: Success
Scope reviewed: all assigned evidence
Lane summary: This change only reorganises existing tests: it moves five already-written tests out of `tiny_store_tests.rs` into a new `tiny_store_guard_tests.rs` module declared via `#[path]`, which is exactly the layout the repository's test-file rules require (`use super::*;` on the first line, no inline `#[cfg(test)] mod tests`, no `_test.rs` suffix). The test bodies are unchanged apart from one attribute being widened to `#[cfg(feature = "security-module")]`, and that widening is consistent with the test's own comment. No production behaviour changes, so no new behavioural test is owed; the moved tests retain their assertions and still exercise create/revise, protected-path refusal, root containment, and stored-key resolution. _The code index is behind this pull request (indexed at `90c5199a04d5`), so retrieved context may be out of date._
commits
Conclusion: Success
Scope reviewed: all assigned evidence
Positive: Commits lane reports no sensitive content in committed files.
Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `90c5199a04d5`), so retrieved context may be out of date._
description
Conclusion: Success
Scope reviewed: all assigned evidence
Positive: Description lane confirms the diff matches the stated intent: the attribute widens to `security-module` to match its sibling, and the comment naming TinySecurity's protected-path floor explains why.
Lane summary: The diff does what the description says: the protected-file refusal test now carries `#[cfg(feature = "security-module")]`, matching its sibling, and the comment naming TinySecurity's protected-path floor explains why. The test move into `tiny_store_guard_tests.rs` comes from the stacked base and matches the repository's sibling-test-file rule. No defects found. _The code index is behind this pull request (indexed at `90c5199a04d5`), so retrieved context may be out of date._
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
priority: p3Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.rust-coreCore Rust runtime in src/: CLI, core_server, shared infrastructure.
2 participants
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.
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
tinysweeper
Bot
added
the
priority: p3
coderabbitai
Bot
added
the
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
artifacts::tiny_store::tests::…::a_protected_file_inside_the_working_folder_is_refusedfails in a build withoutsecurity-module. It asserts TinySecurity's protected-path floor, and that floor does not exist when security is disabled.security-module, like its siblingthe_guarded_roots_refuse_to_read_a_protected_file.Base: stacked on #8560 (the
tiny_store_tests.rssplit), because the test lives in that PR's newtiny_store_guard_tests.rs. On main it is still intiny_store_tests.rs, so a PR off main would conflict with #8560. Merge #8560 first; this then carries one commit.Problem
security-modulehas no path policy at all, and file tools reach~/.sshlike any other path.unwrap_err()panics onOk(Saved { … }). This is identical on main 6043171.Solution
#[cfg(feature = "security-module")]on the test, with a comment naming the floor.Submission Checklist
docs/TEST-COVERAGE-MATRIX.md.docs/RELEASE-MANUAL-SMOKE.md).Validation (cuttlefish, CF-W1-fix-029-1)
cargo fmt --checkpnpm rust:layoutsecurity_mode.rs:93), pre-existingcargo check --locked, core ± security-moduleartifacts::tiny_store::tests::with security-moduleguard_tests::)artifacts::tiny_store::tests::without security-module:68)Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/ts-e2e-029-gate-protected-file-test6786d69b0729325e93494935697d78d4f4ffb8e2Validation Run
pnpm --filter openhuman-app format:check, no frontend changepnpm typecheck, no TypeScript changeValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit