Skip to content

Commit c567326

Browse files
authored
Merge pull request #153 from Firstp1ck/feat/priviliage-improvements
feat: add sudo timestamp setup and security hardening
2 parents b98a7ef + 738eb8c commit c567326

80 files changed

Lines changed: 4051 additions & 881 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ cargo fmt --all
3434
cargo clippy --all-targets --all-features -- -D warnings
3535
cargo test -- --test-threads=1
3636
RUST_LOG=pacsea=debug cargo run -- --dry-run
37+
38+
# Security (mirrors .github/workflows/security.yml where tools are installed)
39+
./dev/scripts/security-check.sh
40+
# Individual checks if needed:
41+
# cargo audit
42+
# cargo deny check
43+
# gitleaks detect --source . --no-banner
3744
```
3845

3946
## Screenshots / recordings (if UI changes)
@@ -69,6 +76,12 @@ Include before/after images or a short GIF. Update files in `Images/` if relevan
6976
- [ ] Code degrades gracefully if `pacman`/`paru`/`yay` are unavailable
7077
- [ ] No breaking changes (or clearly documented if intentional)
7178

79+
**Security (CI):**
80+
- [ ] Ran `./dev/scripts/security-check.sh` before opening the PR (local mirror of the Security workflow: rustfmt, clippy, `cargo audit`, `cargo deny check`, gitleaks — skipped steps print install hints)
81+
- [ ] After adding or updating dependencies: `cargo audit` and `cargo deny check` pass (`deny.toml`); no new high-severity or license issues that would fail GitHub **dependency review** on this PR
82+
- [ ] No secrets or credential-like placeholders that would trip **gitleaks** (see `.gitleaks.toml` for allowlisted paths if you must add test fixtures)
83+
- [ ] Follows secure coding rules in `AGENTS.md` / `CLAUDE.md` (shell quoting, credentials, HTTP, paths) — see `dev/SECURITY_REMEDIATION_GUIDE.md` for remediation patterns
84+
7285
**Other:**
7386
- [ ] Not a packaging change for AUR (otherwise propose in `pacsea-bin` or `pacsea-git` repos)
7487

.github/workflows/security.yml

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
name: Security
2+
3+
on:
4+
push:
5+
branches: [main]
6+
pull_request:
7+
schedule:
8+
# Weekly Monday 06:00 UTC — catch new advisories between PRs
9+
- cron: "0 6 * * 1"
10+
11+
permissions:
12+
contents: read
13+
14+
env:
15+
CARGO_TERM_COLOR: always
16+
17+
jobs:
18+
19+
# ── Dependency vulnerability scan ──────────────────────────────────────
20+
# Checks Cargo.lock against the RustSec advisory database.
21+
# Fails when cargo-audit reports known advisories in dependencies.
22+
cargo-audit:
23+
name: cargo audit
24+
runs-on: ubuntu-latest
25+
steps:
26+
- uses: actions/checkout@v4
27+
28+
- name: Install cargo-audit
29+
run: cargo install cargo-audit --locked
30+
31+
- name: Run cargo audit
32+
run: cargo audit
33+
34+
# ── Clippy with security-relevant lints ────────────────────────────────
35+
# The main rust.yml only runs build + test. Clippy catches:
36+
# - unwrap_used (deny) — panic vectors
37+
# - pedantic / nursery (deny) — broad safety net
38+
# - cognitive_complexity (warn → error via -D warnings)
39+
# - missing_docs / missing_docs_in_private_items (warn → error)
40+
clippy:
41+
name: clippy
42+
runs-on: ubuntu-latest
43+
steps:
44+
- uses: actions/checkout@v4
45+
46+
- uses: dtolnay/rust-toolchain@stable
47+
with:
48+
components: clippy
49+
50+
- name: Run clippy
51+
run: cargo clippy --all-targets --all-features -- -D warnings
52+
53+
# ── Format check ──────────────────────────────────────────────────────
54+
# Ensures no unformatted code slips through.
55+
fmt:
56+
name: rustfmt
57+
runs-on: ubuntu-latest
58+
steps:
59+
- uses: actions/checkout@v4
60+
61+
- uses: dtolnay/rust-toolchain@stable
62+
with:
63+
components: rustfmt
64+
65+
- name: Check formatting
66+
run: cargo fmt --all -- --check
67+
68+
# ── Dependency diff review on PRs ──────────────────────────────────────
69+
# Flags new/changed dependencies with known vulnerabilities or
70+
# restrictive licenses before they land in main.
71+
dependency-review:
72+
name: dependency review
73+
runs-on: ubuntu-latest
74+
if: github.event_name == 'pull_request'
75+
permissions:
76+
contents: read
77+
pull-requests: write
78+
steps:
79+
- uses: actions/checkout@v4
80+
81+
- name: Dependency Review
82+
uses: actions/dependency-review-action@v4
83+
with:
84+
fail-on-severity: high
85+
deny-licenses: AGPL-3.0-only, GPL-3.0-only
86+
comment-summary-in-pr: on-failure
87+
88+
# ── Secret scanning ────────────────────────────────────────────────────
89+
# Scans commits for accidentally committed secrets (API keys, tokens,
90+
# private keys, passwords). Complements GitHub's built-in secret
91+
# scanning with broader pattern coverage.
92+
secrets:
93+
name: secret scan
94+
runs-on: ubuntu-latest
95+
steps:
96+
- uses: actions/checkout@v4
97+
with:
98+
fetch-depth: 0
99+
100+
- name: Run gitleaks
101+
uses: gitleaks/gitleaks-action@v2
102+
env:
103+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
104+
105+
# ── Cargo deny (advisories + license + ban) ────────────────────────────
106+
# More comprehensive than cargo-audit alone: also checks licenses and
107+
# can ban specific crates. Uses deny.toml for configuration.
108+
cargo-deny:
109+
name: cargo deny
110+
runs-on: ubuntu-latest
111+
steps:
112+
- uses: actions/checkout@v4
113+
114+
- name: Run cargo deny
115+
uses: EmbarkStudios/cargo-deny-action@v2
116+
with:
117+
command: check

.gitignore

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,14 @@ __pycache__/
5050
dev/COMMIT/*
5151
dev/PR/*
5252

53-
pacsea.log
53+
pacsea.log
54+
55+
dev/SECURITY_AUDIT_REPORT.md
56+
57+
# Security: prevent accidental secret commits
58+
.env
59+
.env.*
60+
*.pem
61+
*.key
62+
credentials.json
63+
service-account.json

.gitleaks.toml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# gitleaks configuration — suppress false positives
2+
# https://github.com/gitleaks/gitleaks#configuration
3+
4+
[allowlist]
5+
description = "Pacsea false positive suppressions"
6+
7+
# Generated rustdoc JS files contain Rust constant names (KEYCTL_CAPS0_*, FSCRYPT_*)
8+
# that trigger generic-api-key due to entropy. These are not secrets.
9+
paths = [
10+
'''doc/.*\.js$''',
11+
]
12+
13+
# Test files with intentionally fake API keys / patterns
14+
commits = [
15+
"c2bc699bf3dddd75dd4d0c64f5b9084fe8806d82",
16+
"8c8e90760b773a70d3a6f5358cf901f6123a3db9",
17+
]
18+
19+
# Placeholder/example VirusTotal key in shipped config template
20+
regexTarget = "match"
21+
regexes = [
22+
'''abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890''',
23+
'''b83e91a6ddb14ce20edddd2ad056abac5bd63338eb371becadd08b88fb1b20f4''',
24+
]

AGENTS.md

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,60 @@ If config keys or schema change:
159159
| Function length | Clippy `too_many_lines` + `clippy.toml` | 150 lines |
160160
| Data flow / coupling | Manual design review; no lint — match existing module patterns | N/A |
161161

162+
## Security rules
163+
164+
These rules exist to prevent specific vulnerability classes identified in `dev/SECURITY_AUDIT_REPORT.md`. They are **mandatory** — not suggestions.
165+
166+
### Shell command construction
167+
168+
- **Never interpolate package names, file paths, or user input directly into shell command strings.** Always pass them through `shell_single_quote()` from `src/install/utils.rs` first.
169+
- When building shell commands with multiple package names, quote **each name individually** before joining:
170+
```rust
171+
let quoted: Vec<String> = names.iter().map(|n| shell_single_quote(n)).collect();
172+
let names_str = quoted.join(" ");
173+
```
174+
- When adding a new function in `src/install/` that constructs shell commands, verify that **every** variable fragment interpolated into `format!()` is either:
175+
1. A constant/static string, **or**
176+
2. Passed through `shell_single_quote()`, **or**
177+
3. Validated against `^[a-z\d@._+-]+$` before use.
178+
- **Never use `format!()` with `sh -c` and unsanitized user data.** Prefer `Command::new().arg()` argument passing over string interpolation when possible — `Command` arguments are not interpreted by a shell.
179+
- On Windows, never pass unescaped strings into `cmd /C` or `cmd /K`. Use PowerShell with proper quoting or escape `&`, `|`, `>`, `<`, `^` for cmd.exe.
180+
181+
### Password and credential handling
182+
183+
- **Never log passwords.** Any function that writes command strings to disk (log files, temp scripts) must redact password pipe patterns (`printf '%s\n' '...' | sudo -S`) before writing. Replace the password with `[REDACTED]`.
184+
- **Never store passwords in plain `String`.** Use `SecureString` (zeroize-backed) from `src/state/types.rs` for all password fields. If `SecureString` does not exist yet, use `String` but add a `// TODO: migrate to SecureString` comment and a `zeroize` call on the containing struct's `Drop`.
185+
- When creating files that might contain sensitive data (log files, temp scripts), use `OpenOptions::mode(0o600)` on Unix to prevent world-readable permissions.
186+
- Never include passwords or credentials in `tracing::info!`, `tracing::debug!`, or `tracing::warn!` output. If you need to log that a password was provided, log `password_provided = true` — never the value.
187+
188+
### Network and HTTP
189+
190+
- All curl invocations must go through `curl_args()` in `src/util/mod.rs`. Do not construct raw curl commands.
191+
- `curl_args()` must include `--max-filesize 10485760` (10 MB) to prevent memory exhaustion from oversized responses. If this is not yet present, add it.
192+
- Never disable TLS certificate verification (`-k` / `--insecure`) on Linux builds. The Windows `-k` flag is a known issue tracked for removal.
193+
- URLs constructed from user input or API data must be validated to start with `http://` or `https://` before passing to curl. See `looks_like_http_url()` in `src/logic/repos/apply_plan.rs` for the pattern.
194+
195+
### File system safety
196+
197+
- **Validate all paths before writing.** Use or extend `is_safe_abs_path()` from `src/logic/repos/apply_plan.rs` for any privileged file write. Reject paths containing `..`.
198+
- **Create temp files atomically.** Use `OpenOptions::create_new(true).mode(0o700)` instead of `fs::write()` followed by `set_permissions()`. The `create_new` flag prevents symlink-following TOCTOU races.
199+
- When writing files under `~/.config/pacsea/`, create parent directories with `create_dir_all` but do not set overly permissive modes. Default umask-derived permissions are acceptable for non-sensitive files; use `0o600` for anything that could contain credentials.
200+
201+
### Test environment overrides
202+
203+
- Functions that check `PACSEA_INTEGRATION_TEST` or `PACSEA_TEST_*` environment variables must **not** bypass security checks in release builds. Gate them with `#[cfg(debug_assertions)]` or `#[cfg(test)]` so they compile to no-ops in release mode.
204+
- Never add new environment-variable-based test overrides that skip authentication, privilege checks, or input validation without a `#[cfg(debug_assertions)]` guard.
205+
206+
### Dependency management
207+
208+
- Run `cargo audit` after adding or updating dependencies. Resolve all **critical** and **high** advisories before merging.
209+
- Prefer direct dependencies over transitive ones for security-sensitive functionality. If a transitive dependency has an advisory, check if the parent crate can be updated to pull a fixed version.
210+
- Do not add dependencies that require `unsafe` for their core functionality unless there is no safe alternative and the crate is well-maintained.
211+
212+
### Audit reference
213+
214+
Full findings and remediation steps: `dev/SECURITY_AUDIT_REPORT.md` and `dev/SECURITY_REMEDIATION_GUIDE.md`.
215+
162216
## General rules
163217
- No unsolicited `*.md` / wiki / README edits.
164218
- Preserve dry-run semantics and graceful handling of missing external tools.

CLAUDE.md

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,60 @@ If config keys or schema change:
159159
| Function length | Clippy `too_many_lines` + `clippy.toml` | 150 lines |
160160
| Data flow / coupling | Manual design review; no lint — match existing module patterns | N/A |
161161

162+
## Security rules
163+
164+
These rules exist to prevent specific vulnerability classes identified in `dev/SECURITY_AUDIT_REPORT.md`. They are **mandatory** — not suggestions.
165+
166+
### Shell command construction
167+
168+
- **Never interpolate package names, file paths, or user input directly into shell command strings.** Always pass them through `shell_single_quote()` from `src/install/utils.rs` first.
169+
- When building shell commands with multiple package names, quote **each name individually** before joining:
170+
```rust
171+
let quoted: Vec<String> = names.iter().map(|n| shell_single_quote(n)).collect();
172+
let names_str = quoted.join(" ");
173+
```
174+
- When adding a new function in `src/install/` that constructs shell commands, verify that **every** variable fragment interpolated into `format!()` is either:
175+
1. A constant/static string, **or**
176+
2. Passed through `shell_single_quote()`, **or**
177+
3. Validated against `^[a-z\d@._+-]+$` before use.
178+
- **Never use `format!()` with `sh -c` and unsanitized user data.** Prefer `Command::new().arg()` argument passing over string interpolation when possible — `Command` arguments are not interpreted by a shell.
179+
- On Windows, never pass unescaped strings into `cmd /C` or `cmd /K`. Use PowerShell with proper quoting or escape `&`, `|`, `>`, `<`, `^` for cmd.exe.
180+
181+
### Password and credential handling
182+
183+
- **Never log passwords.** Any function that writes command strings to disk (log files, temp scripts) must redact password pipe patterns (`printf '%s\n' '...' | sudo -S`) before writing. Replace the password with `[REDACTED]`.
184+
- **Never store passwords in plain `String`.** Use `SecureString` (zeroize-backed) from `src/state/types.rs` for all password fields. If `SecureString` does not exist yet, use `String` but add a `// TODO: migrate to SecureString` comment and a `zeroize` call on the containing struct's `Drop`.
185+
- When creating files that might contain sensitive data (log files, temp scripts), use `OpenOptions::mode(0o600)` on Unix to prevent world-readable permissions.
186+
- Never include passwords or credentials in `tracing::info!`, `tracing::debug!`, or `tracing::warn!` output. If you need to log that a password was provided, log `password_provided = true` — never the value.
187+
188+
### Network and HTTP
189+
190+
- All curl invocations must go through `curl_args()` in `src/util/mod.rs`. Do not construct raw curl commands.
191+
- `curl_args()` must include `--max-filesize 10485760` (10 MB) to prevent memory exhaustion from oversized responses. If this is not yet present, add it.
192+
- Never disable TLS certificate verification (`-k` / `--insecure`) on Linux builds. The Windows `-k` flag is a known issue tracked for removal.
193+
- URLs constructed from user input or API data must be validated to start with `http://` or `https://` before passing to curl. See `looks_like_http_url()` in `src/logic/repos/apply_plan.rs` for the pattern.
194+
195+
### File system safety
196+
197+
- **Validate all paths before writing.** Use or extend `is_safe_abs_path()` from `src/logic/repos/apply_plan.rs` for any privileged file write. Reject paths containing `..`.
198+
- **Create temp files atomically.** Use `OpenOptions::create_new(true).mode(0o700)` instead of `fs::write()` followed by `set_permissions()`. The `create_new` flag prevents symlink-following TOCTOU races.
199+
- When writing files under `~/.config/pacsea/`, create parent directories with `create_dir_all` but do not set overly permissive modes. Default umask-derived permissions are acceptable for non-sensitive files; use `0o600` for anything that could contain credentials.
200+
201+
### Test environment overrides
202+
203+
- Functions that check `PACSEA_INTEGRATION_TEST` or `PACSEA_TEST_*` environment variables must **not** bypass security checks in release builds. Gate them with `#[cfg(debug_assertions)]` or `#[cfg(test)]` so they compile to no-ops in release mode.
204+
- Never add new environment-variable-based test overrides that skip authentication, privilege checks, or input validation without a `#[cfg(debug_assertions)]` guard.
205+
206+
### Dependency management
207+
208+
- Run `cargo audit` after adding or updating dependencies. Resolve all **critical** and **high** advisories before merging.
209+
- Prefer direct dependencies over transitive ones for security-sensitive functionality. If a transitive dependency has an advisory, check if the parent crate can be updated to pull a fixed version.
210+
- Do not add dependencies that require `unsafe` for their core functionality unless there is no safe alternative and the crate is well-maintained.
211+
212+
### Audit reference
213+
214+
Full findings and remediation steps: `dev/SECURITY_AUDIT_REPORT.md` and `dev/SECURITY_REMEDIATION_GUIDE.md`.
215+
162216
## General rules
163217
- No unsolicited `*.md` / wiki / README edits.
164218
- Preserve dry-run semantics and graceful handling of missing external tools.

CONTRIBUTING.md

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,13 @@ Before committing, ensure all of the following pass:
8989
cargo test -- --test-threads=1
9090
```
9191

92-
5. **Check complexity (for new code):**
92+
5. **Run security checks (if tools installed):**
93+
```bash
94+
./dev/scripts/security-check.sh
95+
```
96+
This runs `cargo audit`, `cargo deny`, `gitleaks`, and the fmt/clippy checks in one pass. Missing tools are skipped with install instructions.
97+
98+
6. **Check complexity (for new code):**
9399
```bash
94100
# Run complexity tests to ensure new functions meet thresholds
95101
cargo test complexity -- --nocapture
@@ -274,6 +280,8 @@ Step-by-step testing instructions.
274280
- [ ] Updated docs if behavior, options, or keybinds changed
275281
- [ ] For UI changes: included screenshots and updated `Images/` if applicable
276282
- [ ] Changes respect `--dry-run` and degrade gracefully if `pacman`/`paru`/`yay` are unavailable
283+
- [ ] `cargo audit` clean (no new high/critical advisories introduced)
284+
- [ ] Shell commands use `shell_single_quote()` for external data (package names, paths)
277285
- [ ] If config keys changed: updated README/wiki sections for `settings.conf`, `theme.conf`, and `keybinds.conf`
278286
- [ ] Not a packaging change for AUR (otherwise propose in `pacsea-bin` or `pacsea-git` repos)
279287

@@ -308,6 +316,13 @@ If new config keys were added, document them here.
308316
- Update README if it's a major feature
309317
- Ensure backward compatibility when possible
310318

319+
### Security
320+
- **Shell commands**: Never interpolate external data (package names, paths, user input) directly into shell strings. Use `shell_single_quote()` from `src/install/utils.rs` or pass arguments via `Command::new().arg()`.
321+
- **Credentials**: Never log passwords or tokens. Redact sensitive values before writing to disk. Use `0o600` permissions for files that could contain credential traces.
322+
- **Network**: All HTTP requests go through `curl_args()`. Never disable TLS verification on Linux. Validate URLs start with `http://` or `https://` before fetching.
323+
- **Dependencies**: Run `cargo audit` after adding or updating crates. High/critical advisories block merges.
324+
- Full rules and rationale: [`CLAUDE.md` → Security rules](CLAUDE.md) and [`dev/SECURITY_REMEDIATION_GUIDE.md`](dev/SECURITY_REMEDIATION_GUIDE.md).
325+
311326
### Platform behavior
312327
- **Dry-run**: All commands must respect `--dry-run` flag
313328
- **Graceful degradation**: Commands must degrade gracefully if `pacman`/`paru`/`yay` are unavailable

0 commit comments

Comments
 (0)