Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -646,3 +646,17 @@ shell-check: ## Run ShellCheck on shell scripts.
shell-check-dev: ## Run ShellCheck inside the Nix dev shell (mirrors CI).
@echo "🔍 Running ShellCheck inside the Nix dev shell..."
@DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) shell-check

##@ Doppler

.PHONY: doppler-sync
doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file.
@echo "🔐 Syncing Doppler secrets to .env..."

Copilot AI Dec 13, 2025

Copy link

Choose a reason for hiding this comment

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

The doppler-sync target overwrites the .env file without any backup or warning. If a user has local changes in their .env file, they will be lost without notification. Consider adding a warning message or creating a backup before overwriting.

Suggested change
@echo "🔐 Syncing Doppler secrets to .env..."
@echo "🔐 Syncing Doppler secrets to .env..."
@if [ -f .env ]; then \
echo "⚠️ .env file exists. Creating backup as .env.bak before overwriting."; \
cp .env .env.bak; \
fi

Copilot uses AI. Check for mistakes.
@doppler secrets download --project dotfiles --config prd --format env --no-file > .env
Comment on lines +654 to +655

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The doppler-sync command overwrites the .env file without any warning. If a user has made local changes to .env that they haven't uploaded, running this target will cause them to lose those changes. It would be safer to update the informational message to warn the user that the file will be overwritten.

	@echo "🔐 Syncing Doppler secrets to .env (this will overwrite the existing file)..."
	@doppler secrets download --project dotfiles --config prd --format env --no-file > .env

@cubic-dev-ai cubic-dev-ai Bot Dec 13, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Using direct stdout redirection can cause data loss if the doppler command fails. Consider writing to a temp file first and moving on success:

@doppler secrets download --project dotfiles --config prd --format env --no-file > .env.tmp && mv .env.tmp .env
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Makefile, line 655:

<comment>Using direct stdout redirection can cause data loss if the doppler command fails. Consider writing to a temp file first and moving on success:
```makefile
@doppler secrets download --project dotfiles --config prd --format env --no-file &gt; .env.tmp &amp;&amp; mv .env.tmp .env
```</comment>

<file context>
@@ -646,3 +646,17 @@ shell-check: ## Run ShellCheck on shell scripts.
+.PHONY: doppler-sync
+doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file.
+	@echo &quot;🔐 Syncing Doppler secrets to .env...&quot;
+	@doppler secrets download --project dotfiles --config prd --format env --no-file &gt; .env
+	@echo &quot;✅ .env file updated from Doppler (dotfiles/prd)&quot;
+
</file context>
Suggested change
@doppler secrets download --project dotfiles --config prd --format env --no-file > .env
@doppler secrets download --project dotfiles --config prd --format env --no-file > .env.tmp && mv .env.tmp .env
Fix with Cubic

@echo "✅ .env file updated from Doppler (dotfiles/prd)"

.PHONY: doppler-upload
doppler-upload: ## Upload .env file to Doppler (dotfiles/prd).

Copilot AI Dec 13, 2025

Copy link

Choose a reason for hiding this comment

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

The doppler-upload target should verify that the .env file exists before attempting to upload to avoid misleading error messages. Consider adding a file existence check before the upload command.

Suggested change
doppler-upload: ## Upload .env file to Doppler (dotfiles/prd).
doppler-upload: ## Upload .env file to Doppler (dotfiles/prd).
@if [ ! -f .env ]; then \
echo "❌ .env file does not exist. Please create it before uploading."; \
exit 1; \
fi

Copilot uses AI. Check for mistakes.
@echo "🔐 Uploading .env to Doppler..."
@doppler secrets upload --project dotfiles --config prd .env
@echo "✅ .env uploaded to Doppler (dotfiles/prd)"
Comment on lines +659 to +662

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The doppler-upload target attempts to upload the .env file without first checking if it exists. If the file is missing (for example, if make doppler-sync hasn't been run), the command will fail with an error from the doppler CLI.

It would be more user-friendly and robust to add a check for the file's existence and provide a helpful error message if it's not found. This would guide the user on the correct workflow.

doppler-upload: ## Upload .env file to Doppler (dotfiles/prd).
	@if [ ! -f .env ]; then \
		echo "❌ .env file not found. Run 'make doppler-sync' first."; \
		exit 1; \
	fi
	@echo "🔐 Uploading .env to Doppler..."
	@doppler secrets upload --project dotfiles --config prd .env
	@echo "✅ .env uploaded to Doppler (dotfiles/prd)"

Comment on lines +650 to +662

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Prevent accidental secret leakage / clobbering when syncing Doppler to .env.
doppler-sync overwrites .env in the repo root and leaves secrets at rest; it’s easy to accidentally commit it. Consider: (1) ensure .env is in .gitignore, (2) write with restrictive permissions (e.g., umask 077), (3) optional backup/confirmation, and/or (4) fail early if doppler isn’t installed/authenticated.

Suggested tweak:

 doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file.
 	@echo "🔐 Syncing Doppler secrets to .env..."
-	@doppler secrets download --project dotfiles --config prd --format env --no-file > .env
+	@command -v doppler >/dev/null 2>&1 || (echo "❌ doppler CLI not found" && exit 1)
+	@umask 077; doppler secrets download --project dotfiles --config prd --format env --no-file > .env
 	@echo "✅ .env file updated from Doppler (dotfiles/prd)"
🧰 Tools
🪛 checkmake (0.2.2)

[warning] 658-658: Missing required phony target "all"

(minphony)


[warning] 658-658: Missing required phony target "clean"

(minphony)

🤖 Prompt for AI Agents
In Makefile around lines 650 to 662, the doppler-sync target currently
overwrites .env in the repo root which risks leaking or clobbering secrets;
update the target to (1) check doppler is installed and authenticated and fail
early if not, (2) ensure .env is listed in .gitignore (or at least warn and
refuse to proceed if not), (3) write secrets to a temporary file then set
restrictive permissions (e.g., create with umask 077 or chmod 600) and
atomically move it into place to avoid leaving secrets in world-readable temp
files, and (4) optionally backup an existing .env (e.g., rename to
.env.backup.TIMESTAMP) or require a confirmation flag before overwriting;
implement these checks and atomic/temp-file + chmod + move flow in the Makefile
recipe so doppler-sync is safe by default.

Comment on lines +652 to +662

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Address checkmake “missing phony all/clean” warnings if you want a clean lint run.
checkmake is asking for conventional all and clean phony targets; if your repo treats these as required, add lightweight aliases (even if they just point at existing targets).

🧰 Tools
🪛 checkmake (0.2.2)

[warning] 658-658: Missing required phony target "all"

(minphony)


[warning] 658-658: Missing required phony target "clean"

(minphony)

🤖 Prompt for AI Agents
In Makefile around lines 652 to 662, checkmake warns about missing conventional
phony targets `all` and `clean`; add lightweight .PHONY targets named `all` and
`clean` (placed near the doppler-* rules) and implement them as simple aliases
that invoke your real default/build target or perform a no-op/cleanup (e.g.,
`all` can depend on the main build target or echo a message; `clean` can remove
generated artifacts or be an empty-but-valid target) so the lint stops
complaining while preserving current behavior.

Loading
Loading