Skip to content

fix(policy): deny uninspectable ancestors and verify configured releases - #4

Merged
senamakel merged 2 commits into
mainfrom
security-release-followup-7328
Oct 10, 2026
Merged

senamakel merged 2 commits into
mainfrom
security-release-followup-7328

Conversation

@senamakel

@senamakel senamakel commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Ancestor resolution treated non-NotFound metadata failures as missing components after canonicalization returned NotFound. It now propagates permission and other inspection errors instead of reconstructing an authorized prospective path. Canonical forbidden-root aliases containing the workspace also preserve the documented workspace precedence; narrower forbidden subtrees remain denied.

Both behavior regressions failed before the fixes and pass afterward. Validation: 152 Linux tests plus the constructor compile-fail doctest, full all-features build, Clippy with warnings denied, formatting, native loader verification, and every production source file above 90% line coverage.

The v0.2.2 release bundles passed every platform's native verifier, but the final generic loader invoked the module with {}, which is intentionally invalid initialization configuration. The release workflow now uses TinySecurity's existing configured verifier over the same GitHub archive/digest loader; it also exercises actual policy calls and SDK reinitialization. That verifier successfully loaded the published v0.2.2 archive, while the failing generic invocation is recorded in release run 38077126471. No configuration validation or policy checks are relaxed.

Follow-up to #3 and OpenHuman issue tinyhumansai/openhuman#7328. OpenHuman's migration remains in tinyhumansai/openhuman#7331; these policy fixes need merging and a new published release before the host ships them.

Summary by CodeRabbit

  • Bug Fixes
    • Workspace access is preserved when a forbidden root is an ancestor reached through a symlink. Forbidden subtrees within the workspace remain blocked.
    • Path checks now report errors encountered while inspecting path components instead of treating them as missing paths.
  • Documentation
    • Clarified how forbidden ancestor paths and symlink aliases affect workspace access.

…eleases

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3a03d450-e418-499c-81aa-576f06901449

📥 Commits

Reviewing files that changed from the base of the PR and between b196016 and cfa4791.


📒 Files selected for processing (4)
  • .github/workflows/release.yml
  • crates/tinysecurity-policy/src/path_scope.rs
  • crates/tinysecurity-policy/src/path_scope_contract_tests.rs
  • docs/specs/immutable-path-scopes.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

The release workflow now verifies the published module with tinysecurity-module’s verify_module example. Path-scope resolution now distinguishes missing paths from other inspection errors. Workspace access checks also account for forbidden roots that resolve through symlinks.

Changes

Path Scope Behavior

Layer / File(s) Summary
Ancestor resolution errors
crates/tinysecurity-policy/src/path_scope.rs, crates/tinysecurity-policy/src/path_scope_contract_tests.rs
Ancestor resolution uses canonicalization and metadata callbacks. Tests check that PermissionDenied, Interrupted, and Other errors are returned unchanged.
Workspace access for ancestor roots
crates/tinysecurity-policy/src/path_scope.rs, crates/tinysecurity-policy/src/path_scope_contract_tests.rs, docs/specs/immutable-path-scopes.md
Forbidden-root checks include the canonical-or-original root when checking whether it contains the workspace. A Unix test and the contract cover a symlink alias to an ancestor; forbidden subtrees inside the workspace remain denied.

Release Verification

Layer / File(s) Summary
Published-module verification
.github/workflows/release.yml
The release workflow invokes tinysecurity-module’s verify_module example instead of TinyBus’s github_module_host example. The release URL, archive name, and SHA-256 checksum remain the same.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix


Merge Risk | ⚪ Minimal · up to cfa47

Merge Risk: ⚪ Minimal · up to cfa47

The change makes path-scope checks fail closed on inspection errors and switches release verification to the module-specific verifier. No merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to cfa47

The inspected changes strengthen failure handling and preserve the documented workspace-access contract. No introduced security defect was established. Remaining uncertainty concerns host-side enforcement and module lifecycle behavior outside the available source.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed access semantics affect host filesystem paths evaluated under registered scopes, particularly workspace paths covered by a forbidden ancestor alias. Full validation bounds enabled scopes to the workspace or explicit trusted roots; the library itself grants no operating-system access. Effective exposure depends on the consuming host's authority and enforcement.

Trust Boundaries and Controls

  • observed — Caller authentication and protection of registration remain host responsibilities. The registry explicitly requires authenticated dispatch and a private module bus; policy identifiers alone do not establish caller ownership. External host callers were not available to verify these controls or enforcement before filesystem operations.

Resilience and Maintainability Implications

  • observed — PermissionDenied, Interrupted, and Other inspection failures now terminate ancestor resolution. Filesystem validation remains a non-atomic check: the documented requirement for an execution sandbox to contain subsequent filesystem races predates this PR.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies both main changes: failing closed for uninspectable ancestors and verifying configured releases.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (2 skipped: 2 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

I’m a rabbit with a checklist neat,
I hop through paths on careful feet.
Symlink roots now join the scope,
Missing paths still leave a trace of hope.
A release check gets a new way,
Then I nibble clover to end the day.

Comment @coderabbitai help to get the list of available commands.

@tinysweeper

tinysweeper Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Ready for maintainer review
Priority: none
Reviewed head: cfa4791aeb5d
Updated: 1791660653 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 1 Noted findings 0
Documentation 1 Resolved findings 3
Configuration 1 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Resolved this pass

  • Reject files under a forbidden aliased ancestor
  • Reject files under a forbidden aliased ancestor
  • Reject files under a forbidden aliased ancestor

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The specification now explicitly covers forbidden roots configured through symlink aliases to the workspace ancestor. The earlier concern is fixed, and this documentation change is safe to merge. _The code index for this repository is cold, so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

security

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No changed file has any attack surface. 1 file was not security-reviewed: docs/specs/immutable-path-scopes.md (prose or tabular data).

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The earlier finding (workspace access revoked when a forbidden root is a symlink alias resolving to the workspace's ancestor) is fixed: the denial check now also compares the canonicalized forbidden root, and a unix contract test exercises exactly that alias scenario. The error-propagation refactor of resolve_ancestor is covered by an injected-fault test plus the unix fixture. The change looks sound; no new findings. _The code index for this repository is cold, so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The earlier finding about forbidden aliased ancestors is fixed: forbidden roots are now also checked through their canonical form, with a Unix test covering the workspace-precedence case, and ancestor resolution now propagates uninspectable-component errors. The only new commit is a documentation tweak that accurately reflects that behavior. The change looks sound and safe to merge. _The code index for this repository is cold, so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: gpt-5.6-luna, glm-5.3-flash
  • Spend: $0.000544
  • Tokens: 24004 input · 2325 output · 3328 cached · 0 embedding
Head State Pass summary
6f7f9ac88895 changes requested 1 active finding(s), 0 resolved finding(s) (at 1791659500)
cfa4791aeb5d ready for maintainer review 0 active finding(s), 3 resolved finding(s) (at 1791660653)

tinysweeper 0.1.0

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: 1 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0107 · 137,514 in / 11,334 out · 18,004 cached (13%) · gpt-5.6-luna, glm-5.3-flash
critique:    $0.0068 · 79,548 in  / 6,807 out  · 10,864 cached (14%) · gpt-5.6-luna, glm-5.3-flash
security:    $0.0038 · 45,626 in  / 2,338 out  · 5,476 cached (12%)  · gpt-5.6-luna
tests:       $0.0001 · 4,751 in   / 1,111 out  · 64 cached (1%)      · glm-5.3-flash
description: $0.0000 · 4,390 in   / 108 out    · 1,472 cached (34%)  · glm-5.3-flash

Comment thread crates/tinysecurity-policy/src/path_scope_contract_tests.rs
Co-authored-by: Medulla <medulla@tinyhumans.ai>

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previously-blocking findings are resolved. Clearing the changes request.

             $0.0005 · 24,004 in / 2,325 out · 3,328 cached (14%) · gpt-5.6-luna, glm-5.3-flash
critique:    $0.0004 · 3,881 in  / 254 out   · 0 cached (0%)      · gpt-5.6-luna
tests:       $0.0001 · 11,527 in / 1,030 out · 3,136 cached (27%) · glm-5.3-flash
description: $0.0000 · 5,025 in  / 112 out   · 64 cached (1%)     · glm-5.3-flash

@senamakel
senamakel merged commit f2b7ebd into main Oct 10, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant