feat(auth): complete security and authorization architecture - #33
Merged
Conversation
…ting, and build integration - Add initial Vue 3 dashboard app structure with Pinia, Vue Router, and i18n setup - Implement reusable UI components (Avatar, Breadcrumb, Sidebar, Sheet, Tooltip, DropdownMenu, etc.) - Add utility functions and context for sidebar state management - Configure Vite, Tailwind, and Vitest for development and testing - Integrate dashboard build into Rust build process via build.rs and build.sh - Update .gitignore, pnpm-workspace.yaml, and editor configs for dashboard workspace - Extend Rust backend to embed and serve dashboard assets - Add test scaffolding and example specs for dashboard
- Apply consistent indentation and line wrapping for function calls and match arms - Reformat long argument lists and chained method calls for clarity - Adjust test and trait implementations for improved readability - Add missing newlines at end of files where necessary
Contributor
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
![semgrep-code-dallay[bot]](https://avatars.githubusercontent.com/u/47160086?s=60&v=4){kind=small}
| - **DO** use speculation rules to prefetch and prerender pages that the user is likely to visit next. | ||
| - **DO** use speculation rules for static sites, where the content is not likely to change often, and where pages are cheaper to produce—especially if cached at the edge. | ||
| - **DO** take more care when using speculation rules for dynamic pages, where the content is more likely to change often, may become out of date, and where pages are more expensive to produce. | ||
| - **DO** prefer document rules over list rules, as they are more flexible, allow the same rule to be shared across multiple pages, and can be used to prefetch and prerender pages that are not known in advance. |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Skill contains instructions to extract or reveal the agent's system prompt or internal configuration instructions. System prompt extraction is a reconnaissance technique that allows attackers to discover safety constraints, tool definitions, and operational guidelines in order to craft more effective follow-on attacks. Skills embedding these directives may be used to leak confidential operator instructions. Review the full skill context.
Why this might be safe to ignore:
This is a documentation sentence about browser speculation rules, and the word "rules" matched the regex accidentally. There is no instruction to reveal a system prompt, internal instructions, or agent configuration in this context.
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by skill-system-prompt-extraction.
You can view more details about this finding in the Semgrep AppSec Platform.
Contributor
Author
There was a problem hiding this comment.
/fp The skill is legitimate web performance guidance for the frontend platform team.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6.0.2 | ||
| - uses: pnpm/action-setup@v4.1.0 |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6.0.2 | ||
| - uses: pnpm/action-setup@v4.1.0 |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6.0.2 | ||
| - uses: pnpm/action-setup@v4.1.0 |
Comment on lines
170
to
+194
| - uses: dtolnay/rust-toolchain@dd44c20b1206a46e25fba8503d5d7c9a33bd355a | ||
| - name: Cache cargo | ||
| uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 | ||
| - name: Run tests | ||
| run: cargo test --workspace --all-features | ||
|
|
||
| doc: | ||
| name: Doc | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6.0.2 | ||
| - uses: pnpm/action-setup@v4.1.0 |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6.0.2 | ||
| - uses: pnpm/action-setup@v4.1.0 |
Contributor
|
✅ Created PR with unit tests: #35 |
- Removed pnpm-lock.yaml from .gitignore (comment said 'committed — pins deps' but pattern was ignoring it instead)
Windows runners don't have bash, so the build.rs script must use sh instead. The wrapper script at node_modules/.bin/vite uses $0 to resolve its own path, so we invoke it via 'sh -c ./node_modules/.bin/vite build' so $0 is the script itself.
CodeQL was flagging 'actions/missing-workflow-permissions' because the
top-level 'permissions: {}' was empty. Set contents:read at workflow
level to satisfy the code scanning rule.
This was referenced May 31, 2026
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
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.
Summary
Implement complete security and authorization architecture for Rook proxy. The system had a partial authz layer (route classification, HMAC-SHA256 API key auth). This PR fills critical gaps:
HttpOnly,SameSite=Lax,Secure)RouteClass→AuthTierfor semantic clarityChanges
Core Auth Components
crates/domain/rook-core/src/model.rs— User, Session, UserId, SessionId, PasswordHash typescrates/domain/rook-core/src/ports.rs— UserRepositoryPort, SessionRepositoryPort, PasswordHasher traitscrates/infrastructure/auth-sqlite/src/lib.rs— SqliteUserRepository, SqliteSessionRepository + migrationscrates/infrastructure/encryption-inmemory/src/password.rs— Argon2idHasher (64MB, 3 iterations, 1 parallelism)Use Cases
crates/application/rook-usecases/src/auth/ensure_admin_user.rs— First-boot admin creationcrates/application/rook-usecases/src/auth/set_admin_password.rs— TUI password settercrates/application/rook-usecases/src/auth/login.rs— Login with session creationcrates/application/rook-usecases/src/auth/logout.rs— Session revocationcrates/application/rook-usecases/src/auth/validate_session.rs— Session validationTransport Layer
crates/infrastructure/transport-axum/src/authz.rs— AuthTier enum, session validation middlewarecrates/infrastructure/transport-axum/src/handlers/auth.rs— POST /login, GET /logout handlerscrates/infrastructure/transport-axum/src/middleware/login_rate_limiter.rs— Per-IP rate limitercrates/infrastructure/transport-axum/src/middleware/csrf_guard.rs— CSRF double-submit validationcrates/infrastructure/transport-axum/src/middleware/api_key_rate_limiter.rs— Per-key rate limitercrates/infrastructure/transport-axum/src/routes.rs— Route wiring with rate limiterscrates/infrastructure/transport-axum/tests/auth_integration_tests.rs— 21 integration testsDI & Bootstrap
apps/rook/src/di.rs— All auth components wired, EnsureAdminUser called at startupArchitecture Docs
openspec/ARCHITECTURE.md— Security architecture documentedopenspec/DOMAIN.md— Auth domain models documentedTesting
cargo clippy --workspace --all-targets -- -D warningscleancargo audit— no CVEsKnown Gaps (Non-Blocking)
POST /logoutreturns 501rook admin set-passwordCLIChecklist
cargo clippy --workspace --all-targets -- -D warningspassescargo test --workspace --all-featurespassescargo auditpasses with no CVEsRelated Issues
Closes implementation of security-authz-architecture-notes SDD change