Skip to content

🛡️ Sentinel: [HIGH] Fix TOCTOU vulnerability in SSH key generation #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kidchenko wants to merge 1 commit into from
Closed

🛡️ Sentinel: [HIGH] Fix TOCTOU vulnerability in SSH key generation #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-04-08 - [TOCTOU in SSH Key Generation]

Check failure on line 1 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

First line in a file should be a top-level heading 

.jules/sentinel.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "## 2024-04-08 - [TOCTOU in SSH..."] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md041.md

Check failure on line 1 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

Headings should be surrounded by blank lines 

.jules/sentinel.md:1 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "## 2024-04-08 - [TOCTOU in SSH Key Generation]"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix markdown structure to pass linting.

The static analysis flagged several issues:

  1. First line should be a top-level heading (# not ##)
  2. Heading should have a blank line below it
Proposed fix 
-## 2024-04-08 - [TOCTOU in SSH Key Generation]
-**Vulnerability:** SSH private keys were temporarily written to disk with default permissions before `chmod 600` was applied, creating a Time-of-Check to Time-of-Use (TOCTOU) vulnerability where other users could theoretically read the key in the brief window between creation and permission change.
+# Sentinel Security Learnings
+
+## 2026-04-08 - TOCTOU in SSH Key Generation
+
+**Vulnerability:** SSH private keys were temporarily written to disk with
+default permissions before `chmod 600` was applied, creating a TOCTOU
+vulnerability where other users could read the key in the brief window
+between creation and permission change.
🧰 Tools
🪛 GitHub Check: Lint Documentation

[failure] 1-1: First line in a file should be a top-level heading
.jules/sentinel.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "## 2024-04-08 - [TOCTOU in SSH..."] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md041.md

[failure] 1-1: Headings should be surrounded by blank lines
.jules/sentinel.md:1 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "## 2024-04-08 - [TOCTOU in SSH Key Generation]"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.jules/sentinel.md at line 1, Change the first line from a level-2 heading
to a top-level heading and ensure there is a blank line immediately after it;
specifically update the line containing "## 2024-04-08 - [TOCTOU in SSH Key
Generation]" to "# 2024-04-08 - [TOCTOU in SSH Key Generation]" and add a single
empty line following that heading so the Markdown linter passes.

**Vulnerability:** SSH private keys were temporarily written to disk with default permissions before `chmod 600` was applied, creating a Time-of-Check to Time-of-Use (TOCTOU) vulnerability where other users could theoretically read the key in the brief window between creation and permission change.

Check failure on line 2 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

Line length 

.jules/sentinel.md:2:81 MD013/line-length Line length [Expected: 80; Actual: 299] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Learning:** Shell redirection (`>`) creates files with default umask permissions. Applying `chmod` immediately after still leaves a race condition.

Check failure on line 3 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

Line length 

.jules/sentinel.md:3:81 MD013/line-length Line length [Expected: 80; Actual: 149] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Prevention:** Shell scripts handling sensitive data must enforce strict access control using `umask 077` (globally or in a subshell) before file creation to prevent TOCTOU vulnerabilities.

Check failure on line 4 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

Line length 

.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 190] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
Comment on lines +2 to +4
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Wrap long lines to fix lint failures.

Lines 2-4 exceed the 80-character limit (MD013), causing the "Lint Documentation" check to fail. Consider wrapping these lines for readability and CI compliance.

Proposed fix with wrapped lines 
-**Vulnerability:** SSH private keys were temporarily written to disk with default permissions before `chmod 600` was applied, creating a Time-of-Check to Time-of-Use (TOCTOU) vulnerability where other users could theoretically read the key in the brief window between creation and permission change.
-**Learning:** Shell redirection (`>`) creates files with default umask permissions. Applying `chmod` immediately after still leaves a race condition.
-**Prevention:** Shell scripts handling sensitive data must enforce strict access control using `umask 077` (globally or in a subshell) before file creation to prevent TOCTOU vulnerabilities.
+**Vulnerability:** SSH private keys were temporarily written to disk with
+default permissions before `chmod 600` was applied, creating a TOCTOU
+vulnerability where other users could read the key in the brief window
+between creation and permission change.
+
+**Learning:** Shell redirection (`>`) creates files with default umask
+permissions. Applying `chmod` immediately after still leaves a race condition.
+
+**Prevention:** Shell scripts handling sensitive data must enforce strict
+access control using `umask 077` (globally or in a subshell) before file
+creation to prevent TOCTOU vulnerabilities.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
**Vulnerability:** SSH private keys were temporarily written to disk with default permissions before `chmod 600` was applied, creating a Time-of-Check to Time-of-Use (TOCTOU) vulnerability where other users could theoretically read the key in the brief window between creation and permission change.
**Learning:** Shell redirection (`>`) creates files with default umask permissions. Applying `chmod` immediately after still leaves a race condition.
**Prevention:** Shell scripts handling sensitive data must enforce strict access control using `umask 077` (globally or in a subshell) before file creation to prevent TOCTOU vulnerabilities.
**Vulnerability:** SSH private keys were temporarily written to disk with
default permissions before `chmod 600` was applied, creating a TOCTOU
vulnerability where other users could read the key in the brief window
between creation and permission change.
**Learning:** Shell redirection (`>`) creates files with default umask
permissions. Applying `chmod` immediately after still leaves a race condition.
**Prevention:** Shell scripts handling sensitive data must enforce strict
access control using `umask 077` (globally or in a subshell) before file
creation to prevent TOCTOU vulnerabilities.
🧰 Tools
🪛 GitHub Check: Lint Documentation

[failure] 4-4: Line length
.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 190] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md

[failure] 3-3: Line length
.jules/sentinel.md:3:81 MD013/line-length Line length [Expected: 80; Actual: 149] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md

[failure] 2-2: Line length
.jules/sentinel.md:2:81 MD013/line-length Line length [Expected: 80; Actual: 299] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.jules/sentinel.md around lines 2 - 4, Wrap the long Markdown lines in the
sentinel.md block so each line is ≤80 characters to satisfy MD013; specifically
break the content under the three bullet lines starting with
"**Vulnerability:**", "**Learning:**", and "**Prevention:**" into multiple
shorter lines (preserve the exact wording and punctuation, splitting between
clauses or after commas) so the text reads the same but no single line exceeds
80 columns.

14 changes: 9 additions & 5 deletions tools/setup-ssh-keys.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,17 @@ cmd_restore() {
mkdir -p "$SSH_DIR"
chmod 700 "$SSH_DIR"

# Read private key from 1Password and save locally
op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE"
chmod 600 "$PRIVATE_KEY_FILE"
# Read private key from 1Password and save locally (prevent TOCTOU)
(
umask 077
op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE"
)

# Read public key from 1Password and save locally
op read "op://$VAULT/$KEY_NAME/public_key" > "$PUBLIC_KEY_FILE"
chmod 644 "$PUBLIC_KEY_FILE"
(
umask 022
op read "op://$VAULT/$KEY_NAME/public_key" > "$PUBLIC_KEY_FILE"
)

say "SSH key restored to $SSH_DIR"
echo ""
Expand Down
Loading