Skip to content

🛡️ Sentinel: Fix SSH key creation race condition #20

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

Open
kidchenko wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🛡️ Sentinel: Fix SSH key creation race condition #20

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-10-24 - Shell Script Race Conditions

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-10-24 - Shell Script R..."] 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-10-24 - Shell Script Race Conditions"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
**Vulnerability:** Found a Time-of-Check Time-of-Use (TOCTOU) race condition in `tools/setup-ssh-keys.sh` where a private key was written to disk with default permissions before being restricted with `chmod`.

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: 208] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Learning:** Shell scripts often default to permissive `umask` (e.g., 022), making files world-readable for a brief window during creation.

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: 140] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Prevention:** Always use `umask 077` in a subshell before writing sensitive files to ensure they are created with restricted permissions (0600) atomically.

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: 157] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
Comment on lines +1 to +4
Copy link

@coderabbitai coderabbitai bot Feb 4, 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 linting failures flagged by CI.

The documentation content is accurate, but the linter reports several issues:

  • MD041: First line should be a top-level heading (# not ##)
  • MD022: Heading needs a blank line after it
  • MD013: Lines 2-4 exceed 80 characters
📝 Proposed fix to address linting issues 
-## 2024-10-24 - Shell Script Race Conditions
-**Vulnerability:** Found a Time-of-Check Time-of-Use (TOCTOU) race condition in `tools/setup-ssh-keys.sh` where a private key was written to disk with default permissions before being restricted with `chmod`.
-**Learning:** Shell scripts often default to permissive `umask` (e.g., 022), making files world-readable for a brief window during creation.
-**Prevention:** Always use `umask 077` in a subshell before writing sensitive files to ensure they are created with restricted permissions (0600) atomically.
+# Sentinel Security Learnings
+
+## 2024-10-24 - Shell Script Race Conditions
+
+**Vulnerability:** Found a Time-of-Check Time-of-Use (TOCTOU) race condition
+in `tools/setup-ssh-keys.sh` where a private key was written to disk with
+default permissions before being restricted with `chmod`.
+
+**Learning:** Shell scripts often default to permissive `umask` (e.g., 022),
+making files world-readable for a brief window during creation.
+
+**Prevention:** Always use `umask 077` in a subshell before writing sensitive
+files to ensure they are created with restricted permissions (0600) atomically.
📝 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
## 2024-10-24 - Shell Script Race Conditions
**Vulnerability:** Found a Time-of-Check Time-of-Use (TOCTOU) race condition in `tools/setup-ssh-keys.sh` where a private key was written to disk with default permissions before being restricted with `chmod`.
**Learning:** Shell scripts often default to permissive `umask` (e.g., 022), making files world-readable for a brief window during creation.
**Prevention:** Always use `umask 077` in a subshell before writing sensitive files to ensure they are created with restricted permissions (0600) atomically.
# Sentinel Security Learnings
## 2024-10-24 - Shell Script Race Conditions
**Vulnerability:** Found a Time-of-Check Time-of-Use (TOCTOU) race condition
in `tools/setup-ssh-keys.sh` where a private key was written to disk with
default permissions before being restricted with `chmod`.
**Learning:** Shell scripts often default to permissive `umask` (e.g., 022),
making files world-readable for a brief window during creation.
**Prevention:** Always use `umask 077` in a subshell before writing sensitive
files to ensure they are created with restricted permissions (0600) atomically.
🧰 Tools
🪛 GitHub Check: Lint Documentation

[failure] 4-4: Line length
.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 157] 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: 140] 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: 208] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md

[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-10-24 - Shell Script R..."] 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-10-24 - Shell Script Race Conditions"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md

🤖 Prompt for AI Agents 
In @.jules/sentinel.md around lines 1 - 4, Update the markdown to satisfy the
linter: change the first heading from "## 2024-10-24 - Shell Script Race
Conditions" to a top-level heading using "#" and add a blank line after it, then
wrap or break long lines (lines describing Vulnerability, Learning, and
Prevention) so they are <=80 characters; keep the same content but reflow text
or split into multiple short lines and ensure the code reference
`tools/setup-ssh-keys.sh` and the recommendation `umask 077`/`chmod` remain
intact.

8 changes: 5 additions & 3 deletions tools/setup-ssh-keys.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,11 @@ 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 (with secure permissions)
(
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"
Expand Down
Loading