feat(observability): P4-P5 enhancements - health/metrics + credentials - #555
Merged
Merged
Conversation
- Merged 26 upstream commits from lfnovo/main - Preserved PMOVES security (USER directive, --chown) - Added upstream improvements: standalone server, NPM_REGISTRY, performance fixes - Includes: i18n (Japanese, Portuguese), 20-30x source listing speedup, cascade deletion
- Merged 161 upstream commits from firefly-iii/main - Added PMOVES /metrics endpoint for Prometheus observability - Resolved conflicts in .env.example and release.yml - Includes: v6.4.16, SSL config, search enhancements, language updates
…urity - gateway: Add /healthz and /metrics endpoints for observability - gateway-agent: Add /metrics endpoint (already had /healthz) - agent-zero: Fix security - run as non-root user (pmoves:pmoves 65532) PMOVES.AI Edition-Hardened - P4 completion 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
📝 WalkthroughWalkthroughUpdates submodule references, makes Agent Zero Dockerfile run as non-root, adds /healthz and /metrics endpoints to gateway services, expands env_setup providers, and introduces a multi-source credential bootstrap script for docked and standalone modes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bootstrap as "bootstrap_credentials.sh"
participant Parent as "parent PMOVES repo"
participant GitHub as "GitHub Secrets / Actions"
participant Docker as "/run/secrets"
participant CHIT as "CHIT decode (python)"
participant Output as ".env.bootstrap"
User->>Bootstrap: run script
Bootstrap->>Bootstrap: detect mode (docked vs standalone)
alt Docked
Bootstrap->>Parent: find & load env.shared
Parent-->>Bootstrap: env values
Bootstrap->>Output: write merged values
else Standalone
Bootstrap->>GitHub: load_from_github_secrets
GitHub-->>Bootstrap: secrets (if present)
Bootstrap->>CHIT: load_from_chit (py module or json)
CHIT-->>Bootstrap: decoded secrets (if present)
Bootstrap->>Docker: load_from_docker_secrets
Docker-->>Bootstrap: secret files (if present)
Bootstrap->>Bootstrap: attempt git-crypt, filter empty keys
Bootstrap->>Output: write merged values
end
Bootstrap->>User: report results & guidance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pmoves/services/agent-zero/Dockerfile`:
- Around line 56-62: The comment incorrectly claims privileged setup occurs in
an init script before dropping to pmoves, but the Dockerfile's USER
pmoves:pmoves is set before ENTRYPOINT so the entrypoint (including
/ins/copy_A0.sh and /opt/venv-a0/bin/python /git/agent-zero/prepare.py) runs as
the non-root pmoves user; update the comment to state that ENTRYPOINT runs as
pmoves and cannot perform privileged actions, or if privileged setup is
required, move the USER directive after a root-run wrapper entrypoint (or
implement a root-owned entrypoint script that performs privileged setup then
explicitly drops to pmoves).
🧹 Nitpick comments (3)
pmoves/services/gateway/gateway/main.py (1)
113-127: MoveResponseimport to module level for consistency.The
Responseimport on line 120 is inside the function body. While functional, it's better to import at module level alongside the existingfastapi.responsesimports on line 13.Also, consider adding return type hints per the coding guidelines (Python 3.11+ with type hints).
♻️ Suggested refactor
Add to imports at line 13:
-from fastapi.responses import FileResponse, HTMLResponse +from fastapi.responses import FileResponse, HTMLResponse, ResponseThen update the function:
`@app.get`("/metrics") -async def metrics(): +async def metrics() -> Response: """Prometheus metrics endpoint placeholder. Note: Full metrics instrumentation to be added with prometheus_client. Currently returns basic service availability. """ - from fastapi import Response return Response( "# PMOVES Gateway metrics\n" "# HELP pmoves_gateway_up Service availability\n" "# TYPE pmoves_gateway_up gauge\n" "pmoves_gateway_up 1\n", media_type="text/plain", )As per coding guidelines: Use Python 3.11+ with 4-space indentation and prefer type hints in all Python code.
pmoves/services/gateway-agent/app.py (1)
424-435: MoveResponseimport to module level and add type hint.Same pattern issue as the gateway service - the
Responseimport is inside the function. For consistency with the existing code style (module-level imports), move it to the top.♻️ Suggested refactor
Add to imports near line 30:
from fastapi import FastAPI, HTTPException, BackgroundTasks, Depends, Security +from fastapi.responses import ResponseThen update the function:
`@app.get`("/metrics") -async def metrics(): +async def metrics() -> Response: """Prometheus metrics endpoint for observability.""" - from fastapi import Response return Response( "# PMOVES Gateway Agent metrics\n" "# HELP pmoves_gateway_agent_up Service availability\n" "# TYPE pmoves_gateway_agent_up gauge\n" "pmoves_gateway_agent_up 1\n", media_type="text/plain", )As per coding guidelines: Use Python 3.11+ with 4-space indentation and prefer type hints in all Python code.
pmoves/services/agent-zero/Dockerfile (1)
61-62: Error suppression with2>/dev/null || truemay hide legitimate failures.The ENTRYPOINT suppresses all errors from
copy_A0.shandprepare.py, making it difficult to diagnose startup issues. If these scripts are optional, consider logging a warning instead of silently swallowing errors. If they're required, failures should be visible.♻️ Alternative entrypoint with conditional logging
-ENTRYPOINT ["/bin/bash", "-lc", "/ins/copy_A0.sh 2>/dev/null || true && /opt/venv-a0/bin/python /git/agent-zero/prepare.py --dockerized=true 2>/dev/null || true && exec /opt/venv-a0/bin/python services/agent-zero/main.py"] +ENTRYPOINT ["/bin/bash", "-lc", "/ins/copy_A0.sh || echo '[WARN] copy_A0.sh skipped'; /opt/venv-a0/bin/python /git/agent-zero/prepare.py --dockerized=true || echo '[WARN] prepare.py skipped'; exec /opt/venv-a0/bin/python services/agent-zero/main.py"]This preserves the "continue on failure" behavior while providing visibility into what was skipped.
Comment on lines
+56
to
+62
| # PMOVES: Run as non-root user | ||
| # The init script will handle any privileged setup during container startup, | ||
| # then drop to pmoves user for the long-running service. | ||
| USER pmoves:pmoves | ||
|
|
||
| # Upstream Agent Zero preparation sets the root password (for SSH) and must run as root. | ||
| # Run preload as root, then drop privileges for the long-running service process. | ||
| CMD ["/bin/bash", "-lc", "/ins/copy_A0.sh && /opt/venv-a0/bin/python /git/agent-zero/prepare.py --dockerized=true && chown -R pmoves:pmoves /a0 && exec su -s /bin/bash -c '/opt/venv-a0/bin/python services/agent-zero/main.py' pmoves"] | ||
| # Entrypoint handles optional privileged setup then drops to pmoves user | ||
| ENTRYPOINT ["/bin/bash", "-lc", "/ins/copy_A0.sh 2>/dev/null || true && /opt/venv-a0/bin/python /git/agent-zero/prepare.py --dockerized=true 2>/dev/null || true && exec /opt/venv-a0/bin/python services/agent-zero/main.py"] |
Contributor
There was a problem hiding this comment.
Clarify comment: USER directive runs before ENTRYPOINT, so no privileged setup occurs.
The comment on lines 57-58 states "The init script will handle any privileged setup during container startup, then drop to pmoves user" — but the USER pmoves:pmoves directive (line 59) is set before the ENTRYPOINT, so the entire entrypoint runs as the non-root user. The init script cannot perform privileged setup unless the container is started with --user root externally.
Consider updating the comment to reflect the actual behavior, or if privileged setup is truly needed, restructure to use an entrypoint script that starts as root and drops privileges.
📝 Suggested comment fix
-# PMOVES: Run as non-root user
-# The init script will handle any privileged setup during container startup,
-# then drop to pmoves user for the long-running service.
+# PMOVES: Run as non-root user for the entire container lifecycle.
+# All setup scripts run unprivileged; privileged operations must be completed
+# during image build or via external orchestration.
USER pmoves:pmoves🤖 Prompt for AI Agents
In `@pmoves/services/agent-zero/Dockerfile` around lines 56 - 62, The comment
incorrectly claims privileged setup occurs in an init script before dropping to
pmoves, but the Dockerfile's USER pmoves:pmoves is set before ENTRYPOINT so the
entrypoint (including /ins/copy_A0.sh and /opt/venv-a0/bin/python
/git/agent-zero/prepare.py) runs as the non-root pmoves user; update the comment
to state that ENTRYPOINT runs as pmoves and cannot perform privileged actions,
or if privileged setup is required, move the USER directive after a root-run
wrapper entrypoint (or implement a root-owned entrypoint script that performs
privileged setup then explicitly drops to pmoves).
…support Restores scripts/bootstrap_credentials.sh from fix-post-merge-production branch which was never merged into PMOVES.AI-Edition-Hardened. Also updates PowerShell env_setup.ps1 with same provider support. Changes: - Restore scripts/bootstrap_credentials.sh (v4) with: - GitHub Secrets detection (GITHUB_ACTIONS, CODESPACES env vars) - CHIT Geometry Packet decoding - git-crypt encrypted file support - Docker Secrets (/run/secrets/*) support - Parent PMOVES.AI fallback - Empty value filtering for credential keys - Update pmoves/scripts/env_setup.ps1 to add: - 'github' provider - Load from environment variables - 'docker' provider - Load from /run/secrets/ - 'chit' provider - Load from CHIT Geometry Packet This enables submodules (PMOVES-ToKenism-Multi, PMOVES-DoX, PMOVES-BoTZ) to load credentials from multiple sources when run on different machines. Resolves: Credentials not loading in PowerShell scripts on other machines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@pmoves/scripts/env_setup.ps1`:
- Around line 133-151: The Docker-secrets import currently builds $envName using
ToTitleCase (producing Openai_Api_Key) and preserves trailing newlines from
Get-Content -Raw; change the $envName construction (from $name in the
Get-ChildItem loop) to strip the '^pmoves_' prefix, replace non-alphanumeric
chars with '_' and force uppercase (e.g. ($name -replace '^pmoves_','' -replace
'[^A-Za-z0-9]','_' ) .ToUpper()) so keys become OPENAI_API_KEY, and trim the
secret value (use $value = (Get-Content $_.FullName -Raw -ErrorAction
SilentlyContinue).Trim() or .TrimEnd()) before adding to $pairs so no extra
newlines are injected; leave the rest of the logic using $pairs and Set-Content
unchanged.
- Around line 159-205: The captured CHIT decode output ($decoded) currently
includes stderr because of the shell redirection "2>&1", which can corrupt the
generated .env file; remove the "2>&1" so python's stderr doesn't get mixed into
$decoded, then before calling Set-Content filter $decoded to only lines matching
the env pattern (e.g. '^[A-Z_]+='), write only those filtered lines to $OutPath,
update $count based on that filtered set, and (optionally) capture and log any
stderr separately so errors/warnings from the python decode of $cgpFile are not
written into the .env.generated file.
In `@scripts/bootstrap_credentials.sh`:
- Around line 282-299: The conditional that detects decrypted git-crypt files is
wrong: the pattern [[ "$first_line" == *"[A-Z_"* ]] matches the literal string
"[A-Z_" instead of a character class, so lines like "OPENAI_API_KEY=..." are
missed. Fix the check in the block that reads local first_line by replacing that
quoted glob with a proper character-class match (e.g. [[ "$first_line" ==
*[A-Z_]* ]] or use a regex test like [[ "$first_line" =~ ^[A-Z_]+= ]]) so
env-var style lines are recognized; keep the rest of the placeholder-filtering
logic that writes to "$output_file" and the log_success call unchanged.
- Around line 450-452: The script's main() currently appends to .env.bootstrap
causing stale values and leaves it world-readable; modify main() to truncate the
output_file at start (use the output_file variable) so each run starts fresh,
then after calling filter_empty_values finish by applying chmod 600 to
output_file to lock permissions; additionally add ".env.bootstrap" to .gitignore
to prevent committing secrets.
- Around line 124-135: The grep regexes that scan env files (used where
env_shared is read and where counts original_count/filtered_count are computed)
currently use patterns like '^[A-Z_]+=' and '^[A-Z_]=' which exclude variable
names with digits; update all those occurrences to use '^[A-Z_][A-Z0-9_]*=' so
names like E2B_API_KEY, E2E_TEST, K8S_CONFIG are preserved, ensuring you change
the grep -E patterns at the env_shared load block and the later count/filtering
blocks that set original_count and filtered_count to the new pattern.
Comment on lines
+133
to
+151
| 'docker' { | ||
| # PMOVES.AI: Load credentials from Docker Secrets (/run/secrets/) | ||
| $secretsDir = '/run/secrets' | ||
| if (Test-Path $secretsDir) { | ||
| $found = 0 | ||
| $pairs = @() | ||
| Get-ChildItem -Path $secretsDir -Filter 'pmoves_*' -ErrorAction SilentlyContinue | ForEach-Object { | ||
| $name = $_.Name | ||
| # Convert pmoves_openai_api_key -> OPENAI_API_KEY | ||
| $envName = $name -replace '^pmoves_', '' -replace '_', ' ' | ForEach-Object { (Get-Culture).TextInfo.ToTitleCase($_) } -replace ' ', '_' | ||
| $value = Get-Content $_.FullName -Raw -ErrorAction SilentlyContinue | ||
| if ($value) { | ||
| $pairs += "$envName=$value" | ||
| $found++ | ||
| } | ||
| } | ||
| if ($found -gt 0) { | ||
| $pairs -join "`n" | Set-Content -Encoding UTF8 -Path $OutPath | ||
| Write-Host "Imported $found secrets from Docker Secrets -> $OutPath" -ForegroundColor Green |
Contributor
There was a problem hiding this comment.
Normalize Docker secret names to expected ENV keys and trim newlines.
ToTitleCase yields keys like Openai_Api_Key, which won’t match typical OPENAI_API_KEY. Also Get-Content -Raw includes trailing newlines that can inject line breaks into the .env output.
🛠️ Proposed fix
- $envName = $name -replace '^pmoves_', '' -replace '_', ' ' | ForEach-Object { (Get-Culture).TextInfo.ToTitleCase($_) } -replace ' ', '_'
- $value = Get-Content $_.FullName -Raw -ErrorAction SilentlyContinue
+ $envName = (($name -replace '^pmoves_', '' -replace '_', ' ').ToUpperInvariant()) -replace ' ', '_'
+ $value = (Get-Content $_.FullName -Raw -ErrorAction SilentlyContinue).TrimEnd("`r","`n")🤖 Prompt for AI Agents
In `@pmoves/scripts/env_setup.ps1` around lines 133 - 151, The Docker-secrets
import currently builds $envName using ToTitleCase (producing Openai_Api_Key)
and preserves trailing newlines from Get-Content -Raw; change the $envName
construction (from $name in the Get-ChildItem loop) to strip the '^pmoves_'
prefix, replace non-alphanumeric chars with '_' and force uppercase (e.g. ($name
-replace '^pmoves_','' -replace '[^A-Za-z0-9]','_' ) .ToUpper()) so keys become
OPENAI_API_KEY, and trim the secret value (use $value = (Get-Content $_.FullName
-Raw -ErrorAction SilentlyContinue).Trim() or .TrimEnd()) before adding to
$pairs so no extra newlines are injected; leave the rest of the logic using
$pairs and Set-Content unchanged.
Comment on lines
+159
to
+205
| 'chit' { | ||
| # PMOVES.AI: Load credentials from CHIT Geometry Packet | ||
| $cgpPaths = @( | ||
| 'data/chit/env.cgp.json', | ||
| 'pmoves/data/chit/env.cgp.json', | ||
| '../pmoves/data/chit/env.cgp.json', | ||
| '../../pmoves/data/chit/env.cgp.json' | ||
| ) | ||
| $cgpFile = $null | ||
| foreach ($path in $cgpPaths) { | ||
| if (Test-Path $path) { | ||
| $cgpFile = $path | ||
| break | ||
| } | ||
| } | ||
| if ($cgpFile) { | ||
| try { | ||
| # Try Python CHIT decode | ||
| $decoded = python3 -c @" | ||
| import sys, json | ||
| from pathlib import Path | ||
| repo_root = Path('$pwd').resolve().parent | ||
| for parent in [repo_root] + list(repo_root.parents): | ||
| chit_path = parent / 'pmoves' / 'chit' | ||
| if chit_path.exists(): | ||
| sys.path.insert(0, str(parent)) | ||
| break | ||
| try: | ||
| from pmoves.chit import load_cgp, decode_secret_map | ||
| cgp = load_cgp('$cgpFile') | ||
| secrets = decode_secret_map(cgp) | ||
| for k, v in sorted(secrets.items()): | ||
| print(f'{k}={v}') | ||
| except ImportError: | ||
| with open('$cgpFile') as f: | ||
| cgp = json.load(f) | ||
| for point in cgp.get('points', []): | ||
| label = point['label'] | ||
| value = point.get('value', '') | ||
| encoding = point.get('encoding', 'cleartext') | ||
| if encoding == 'cleartext': | ||
| print(f'{label}={value}') | ||
| "@ 2>&1 | ||
| if ($decoded) { | ||
| $decoded | Set-Content -Encoding UTF8 -Path $OutPath | ||
| $count = ($decoded -split "`n").Where({ $_ -match '^[A-Z_]+=' }).Count | ||
| Write-Host "Decoded $count secrets from CHIT Geometry Packet -> $OutPath" -ForegroundColor Green |
Contributor
There was a problem hiding this comment.
Avoid mixing stderr into generated .env output for CHIT decode.
2>&1 captures warnings/errors into $decoded, which then get written to .env.generated, potentially corrupting it. Filter to env-formatted lines before writing.
🛠️ Proposed fix
- $decoded = python3 -c @"
+ $decoded = python3 -c @"
import sys, json
from pathlib import Path
repo_root = Path('$pwd').resolve().parent
@@
print(f'{label}={value}')
"@ 2>&1
- if ($decoded) {
- $decoded | Set-Content -Encoding UTF8 -Path $OutPath
- $count = ($decoded -split "`n").Where({ $_ -match '^[A-Z_]+=' }).Count
- Write-Host "Decoded $count secrets from CHIT Geometry Packet -> $OutPath" -ForegroundColor Green
- } else {
- Write-Warning "CHIT decode failed. Make sure pmoves.chit module is available."
- }
+ if ($decoded) {
+ $lines = $decoded -split "`r?`n"
+ $envLines = $lines | Where-Object { $_ -match '^[A-Z_][A-Z0-9_]*=' }
+ if ($envLines.Count -gt 0) {
+ $envLines | Set-Content -Encoding UTF8 -Path $OutPath
+ $count = $envLines.Count
+ Write-Host "Decoded $count secrets from CHIT Geometry Packet -> $OutPath" -ForegroundColor Green
+ } else {
+ Write-Warning "CHIT decode produced no env-formatted lines."
+ }
+ } else {
+ Write-Warning "CHIT decode failed. Make sure pmoves.chit module is available."
+ }🤖 Prompt for AI Agents
In `@pmoves/scripts/env_setup.ps1` around lines 159 - 205, The captured CHIT
decode output ($decoded) currently includes stderr because of the shell
redirection "2>&1", which can corrupt the generated .env file; remove the "2>&1"
so python's stderr doesn't get mixed into $decoded, then before calling
Set-Content filter $decoded to only lines matching the env pattern (e.g.
'^[A-Z_]+='), write only those filtered lines to $OutPath, update $count based
on that filtered set, and (optionally) capture and log any stderr separately so
errors/warnings from the python decode of $cgpFile are not written into the
.env.generated file.
Comment on lines
+124
to
+135
| if [ -f "$env_shared" ]; then | ||
| log_info "Loading env.shared structure..." | ||
| # Copy env.shared to output, filtering out: | ||
| # - comments and empty lines | ||
| # - placeholder values (any line ending with -here, -if-needed, descriptive placeholders) | ||
| grep -E '^[A-Z_]+=|^export ' "$env_shared" 2>/dev/null | \ | ||
| sed 's/^export //' | \ | ||
| grep -vE '-here$|-if-needed$|-when-needed$|-optional$' | \ | ||
| grep -vE '@your-' | \ | ||
| grep -vE '=TEMPLATE_' | \ | ||
| grep -vE '^TEMPLATE_' | \ | ||
| grep -vE '=super-secret-jwt-token-with-at-least' | \ |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and check if it exists
fd -t f "bootstrap_credentials.sh"Repository: POWERFULMOVES/PMOVES.AI
Length of output: 99
🏁 Script executed:
# Get the file size first
wc -l scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 103
🏁 Script executed:
# Read the file content around the mentioned line ranges
cat -n scripts/bootstrap_credentials.sh | sed -n '120,140p'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 1198
🏁 Script executed:
# Check the other mentioned line ranges
cat -n scripts/bootstrap_credentials.sh | sed -n '145,150p'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 422
🏁 Script executed:
# Check lines 428-435
cat -n scripts/bootstrap_credentials.sh | sed -n '425,440p'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 1055
🏁 Script executed:
# Check lines 538-539
cat -n scripts/bootstrap_credentials.sh | sed -n '535,545p'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 642
🏁 Script executed:
# Now let's test the regex patterns to verify the claims
echo "=== Testing current patterns vs fixed pattern ==="
echo ""
echo "Test case: E2B_API_KEY=1"
echo ""
echo "Current pattern: ^[A-Z_]+="
printf 'E2B_API_KEY=1\n' | grep -E '^[A-Z_]+=' && echo "✓ MATCHED" || echo "✗ NOT MATCHED (claim is FALSE)"
echo ""
echo "Fixed pattern: ^[A-Z_][A-Z0-9_]*="
printf 'E2B_API_KEY=1\n' | grep -E '^[A-Z_][A-Z0-9_]*=' && echo "✓ MATCHED" || echo "✗ NOT MATCHED"
echo ""
echo "=== Testing single letter pattern from line 538 ==="
echo "Test case: API_KEY=value (has digit-like suffix but underscore)"
printf 'API_KEY=value\n' | grep -E '^[A-Z_]=' && echo "✓ MATCHED by ^[A-Z_]=" || echo "✗ NOT MATCHED by ^[A-Z_]="
printf 'API_KEY=value\n' | grep -E '^[A-Z_][A-Z0-9_]*=' && echo "✓ MATCHED by ^[A-Z_][A-Z0-9_]*=" || echo "✗ NOT MATCHED by ^[A-Z_][A-Z0-9_]*="
echo ""
echo "=== Testing other variables ==="
printf 'OPENAI_API_KEY=secret\nMYVAR=val\nE2E_TEST=1\n' | grep -E '^[A-Z_]+='
echo "--- Above: matched by current pattern ^[A-Z_]+= ---"
echo ""
printf 'OPENAI_API_KEY=secret\nMYVAR=val\nE2E_TEST=1\n' | grep -E '^[A-Z_][A-Z0-9_]*='
echo "--- Above: matched by fixed pattern ^[A-Z_][A-Z0-9_]*= ---"Repository: POWERFULMOVES/PMOVES.AI
Length of output: 669
🏁 Script executed:
# Verify the exact pattern at line 538 more carefully
cat -n scripts/bootstrap_credentials.sh | sed -n '536,540p'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 366
🏁 Script executed:
# Final verification: check for any other uses of this pattern in the file
rg '\^\[A-Z_\]' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 581
🏁 Script executed:
# Find the line with enc_file to check if there are more locations
rg -n 'enc_file' scripts/bootstrap_credentials.sh | head -20Repository: POWERFULMOVES/PMOVES.AI
Length of output: 348
🏁 Script executed:
# Check the git-crypt loader section around line 289
cat -n scripts/bootstrap_credentials.sh | sed -n '285,295p'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 763
🏁 Script executed:
# Create a comprehensive test to demonstrate the impact
echo "=== Comprehensive test of impact ==="
echo ""
echo "Creating test env file with mixed variable names:"
cat > /tmp/test_env.txt << 'EOF'
API_KEY=value1
E2B_API_KEY=value2
E2E_TEST=value3
OPENAI_API_BASE=value4
SOME_VAR=value5
DB_NAME=value6
K8S_CONFIG=value7
EOF
echo "Test file content:"
cat /tmp/test_env.txt
echo ""
echo "Matched by current pattern '^[A-Z_]+=':"
grep -E '^[A-Z_]+=' /tmp/test_env.txt
echo ""
echo "Matched by fixed pattern '^[A-Z_][A-Z0-9_]*=':"
grep -E '^[A-Z_][A-Z0-9_]*=' /tmp/test_env.txt
echo ""
echo "Matched by line 538 pattern '^[A-Z_]=' (single char only):"
grep -E '^[A-Z_]=' /tmp/test_env.txt || echo "(none matched)"Repository: POWERFULMOVES/PMOVES.AI
Length of output: 650
Fix regex patterns to match environment variables with digits (E2B_API_KEY, K8S_CONFIG, etc.)
Current patterns ^[A-Z_]+= and ^[A-Z_]= exclude variables containing digits, causing critical credentials to be silently dropped. For example, E2B_API_KEY, E2E_TEST, and K8S_CONFIG are filtered out.
Use ^[A-Z_][A-Z0-9_]*= instead at all affected locations:
Locations requiring fix
- Line 129:
grep -E '^[A-Z_]+=|^export ' - Line 147:
grep -E '^[A-Z_]+=' - Line 289:
grep -E '^[A-Z_]+='(git-crypt loader) - Line 428:
grep -E '^[A-Z_]+=' - Line 434:
grep -c '^[A-Z_]+='(original_count) - Line 435:
grep -c '^[A-Z_]+='(filtered_count) - Line 538:
grep -c '^[A-Z_]='(counts zero variables)
Proposed fix
- grep -E '^[A-Z_]+=|^export ' "$env_shared" 2>/dev/null | \
+ grep -E '^[A-Z_][A-Z0-9_]*=|^export ' "$env_shared" 2>/dev/null | \
@@
- grep -E '^[A-Z_]+=' "$parent_env" 2>/dev/null >> "$output_file" || true
+ grep -E '^[A-Z_][A-Z0-9_]*=' "$parent_env" 2>/dev/null >> "$output_file" || true
@@
- grep -E '^[A-Z_]+=' "$enc_file" 2>/dev/null | \
+ grep -E '^[A-Z_][A-Z0-9_]*=' "$enc_file" 2>/dev/null | \
@@
- grep -E '^[A-Z_]+=' "$output_file" 2>/dev/null | \
+ grep -E '^[A-Z_][A-Z0-9_]*=' "$output_file" 2>/dev/null | \
@@
- local original_count=$(grep -c '^[A-Z_]+=' "$output_file" 2>/dev/null || echo "0")
- local filtered_count=$(grep -c '^[A-Z_]+=' "$temp_file" 2>/dev/null || echo "0")
+ local original_count=$(grep -c '^[A-Z_][A-Z0-9_]*=' "$output_file" 2>/dev/null || echo "0")
+ local filtered_count=$(grep -c '^[A-Z_][A-Z0-9_]*=' "$temp_file" 2>/dev/null || echo "0")
@@
- local var_count=$(grep -c '^[A-Z_]=' "$output_file" 2>/dev/null || echo "0")
+ local var_count=$(grep -c '^[A-Z_][A-Z0-9_]*=' "$output_file" 2>/dev/null || echo "0")🤖 Prompt for AI Agents
In `@scripts/bootstrap_credentials.sh` around lines 124 - 135, The grep regexes
that scan env files (used where env_shared is read and where counts
original_count/filtered_count are computed) currently use patterns like
'^[A-Z_]+=' and '^[A-Z_]=' which exclude variable names with digits; update all
those occurrences to use '^[A-Z_][A-Z0-9_]*=' so names like E2B_API_KEY,
E2E_TEST, K8S_CONFIG are preserved, ensuring you change the grep -E patterns at
the env_shared load block and the later count/filtering blocks that set
original_count and filtered_count to the new pattern.
Comment on lines
+282
to
+299
| # Check if file is decrypted (content is readable) | ||
| local first_line=$(head -1 "$enc_file" 2>/dev/null || echo "") | ||
|
|
||
| # git-crypt encrypted files start with specific bytes | ||
| # If we can read a normal-looking line, it's decrypted | ||
| if [[ "$first_line" == *"#"* ]] || [[ "$first_line" == *"[A-Z_"* ]] || [[ "$first_line" == *"PMOVES"* ]]; then | ||
| # File is decrypted, load it (filtering placeholders) | ||
| grep -E '^[A-Z_]+=' "$enc_file" 2>/dev/null | \ | ||
| grep -vE '-here$|-if-needed$|-when-needed$|-optional$' | \ | ||
| grep -vE '@your-' | \ | ||
| grep -vE '=TEMPLATE_' | \ | ||
| grep -vE '^TEMPLATE_' | \ | ||
| grep -vE '=Replace this with actual' \ | ||
| >> "$output_file" || true | ||
| local count=$(grep -c '^' "$output_file" 2>/dev/null || echo "0") | ||
| log_success " Loaded $count credentials from git-crypt (decrypted, placeholders filtered)" | ||
| return 0 | ||
| else |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Test the bash glob behavior with quoted vs unquoted patterns
echo "=== Testing quoted glob pattern ==="
first_line='OPENAI_API_KEY=foo'
[[ "$first_line" == *"[A-Z_"* ]] && echo "quoted glob: MATCHED" || echo "quoted glob: MISSED"
echo ""
echo "=== Testing if literal [A-Z_ substring exists ==="
[[ "$first_line" == *"["* ]] && echo "contains literal [: YES" || echo "contains literal [: NO"
echo ""
echo "=== Testing regex pattern (proposed fix) ==="
[[ "$first_line" =~ ^[A-Z_][A-Z0-9_]*= ]] && echo "regex: MATCHED" || echo "regex: MISSED"
echo ""
echo "=== Testing hash comment detection ==="
first_line='# This is a comment'
[[ "$first_line" == *"#"* ]] && echo "hash pattern: MATCHED" || echo "hash pattern: MISSED"
echo ""
echo "=== Testing unquoted glob for env var pattern ==="
first_line='OPENAI_API_KEY=foo'
[[ "$first_line" == [A-Z_]* ]] && echo "unquoted glob [A-Z_]*: MATCHED" || echo "unquoted glob [A-Z_]*: MISSED"Repository: POWERFULMOVES/PMOVES.AI
Length of output: 401
🏁 Script executed:
sed -n '280,305p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 1228
Fix git-crypt decrypted detection — quoted glob searches for literal [A-Z_ string, not env var pattern.
The pattern [[ "$first_line" == *"[A-Z_"* ]] searches for the literal substring [A-Z_, not a character class. A file starting with OPENAI_API_KEY=foo or any standard env var misses all three conditions (no #, no literal [A-Z_, no PMOVES), causing decrypted files to be incorrectly classified as encrypted and skipped.
🐛 Proposed fix
- if [[ "$first_line" == *"#"* ]] || [[ "$first_line" == *"[A-Z_"* ]] || [[ "$first_line" == *"PMOVES"* ]]; then
+ if [[ "$first_line" == \#* ]] || [[ "$first_line" == *PMOVES* ]] || [[ "$first_line" =~ ^[A-Z_][A-Z0-9_]*= ]]; then🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 283-283: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 296-296: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In `@scripts/bootstrap_credentials.sh` around lines 282 - 299, The conditional
that detects decrypted git-crypt files is wrong: the pattern [[ "$first_line" ==
*"[A-Z_"* ]] matches the literal string "[A-Z_" instead of a character class, so
lines like "OPENAI_API_KEY=..." are missed. Fix the check in the block that
reads local first_line by replacing that quoted glob with a proper
character-class match (e.g. [[ "$first_line" == *[A-Z_]* ]] or use a regex test
like [[ "$first_line" =~ ^[A-Z_]+= ]]) so env-var style lines are recognized;
keep the rest of the placeholder-filtering logic that writes to "$output_file"
and the log_success call unchanged.
Comment on lines
+450
to
+452
| main() { | ||
| local output_file=".env.bootstrap" | ||
| local parent_dir="" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate the script and check basic file info
find . -name "bootstrap_credentials.sh" -type fRepository: POWERFULMOVES/PMOVES.AI
Length of output: 101
🏁 Script executed:
# Check .gitignore to see if .env.bootstrap is ignored
cat .gitignore 2>/dev/null | grep -E "\.env|bootstrap"Repository: POWERFULMOVES/PMOVES.AI
Length of output: 308
🏁 Script executed:
# Read the script around the specified lines
if [ -f scripts/bootstrap_credentials.sh ]; then
wc -l scripts/bootstrap_credentials.sh
fiRepository: POWERFULMOVES/PMOVES.AI
Length of output: 103
🏁 Script executed:
# Read the main() function area and surrounding context
sed -n '440,460p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 648
🏁 Script executed:
# Read the other affected section around lines 533-539
sed -n '525,545p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 803
🏁 Script executed:
# Search for all uses of .env.bootstrap in the script
rg '\.env\.bootstrap' scripts/bootstrap_credentials.sh -nRepository: POWERFULMOVES/PMOVES.AI
Length of output: 478
🏁 Script executed:
# Search for how output is written to the file (>> for append, > for redirect)
rg '>>|>|\$output_file' scripts/bootstrap_credentials.sh -n -B2 -A2 | head -100Repository: POWERFULMOVES/PMOVES.AI
Length of output: 4628
🏁 Script executed:
# Look at filter_empty_values function to see how it processes the output file
sed -n '160,180p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 711
🏁 Script executed:
# Search for chmod operations in the script
rg 'chmod' scripts/bootstrap_credentials.sh -nRepository: POWERFULMOVES/PMOVES.AI
Length of output: 49
🏁 Script executed:
# Look at the logic flow to understand when line 137 (the truncate) happens
sed -n '115,155p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 1677
🏁 Script executed:
# Check if .env.bootstrap appears in .gitignore by any pattern
cat .gitignore | grep -E 'bootstrap|env\*'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 49
🏁 Script executed:
# See what happens at the start of main() before any writes
sed -n '451,480p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 1114
🏁 Script executed:
# See the full standalone mode section and how it handles output_file
sed -n '480,530p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 2211
🏁 Script executed:
# Check if there's any truncation or file initialization in standalone mode
rg 'output_file.*[>|:]|>\s*".*output' scripts/bootstrap_credentials.sh -n | head -20Repository: POWERFULMOVES/PMOVES.AI
Length of output: 1435
🏁 Script executed:
# Look at where filter_empty_values is called and what it does
sed -n '410,435p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 1126
🏁 Script executed:
# Check the filter_empty_values function completely (around lines 533-539 mentioned in review)
sed -n '418,450p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 1353
🏁 Script executed:
# Also verify what happens after filter_empty_values is called
sed -n '535,555p' scripts/bootstrap_credentials.shRepository: POWERFULMOVES/PMOVES.AI
Length of output: 1199
Add file truncation and permission locking to prevent stale values and world-readable secrets.
In standalone mode, the script appends to .env.bootstrap through multiple load_from_* functions without truncating first. Reruns mix stale values from previous executions. Additionally, no chmod is applied, leaving the secrets file potentially world-readable by default. Add > "$output_file" at the start of main() to truncate, and chmod 600 "$output_file" after filter_empty_values completes.
Also note: .env.bootstrap is not listed in .gitignore, risking accidental commit of secrets. Add it to the ignore list.
🔒 Proposed fix
main() {
local output_file=".env.bootstrap"
local parent_dir=""
+ : > "$output_file" # Filter out empty credential values
filter_empty_values "$output_file"
+ chmod 600 "$output_file" 2>/dev/null || trueAnd in .gitignore:
/.env
/.env.local
+/.env.bootstrap🤖 Prompt for AI Agents
In `@scripts/bootstrap_credentials.sh` around lines 450 - 452, The script's main()
currently appends to .env.bootstrap causing stale values and leaves it
world-readable; modify main() to truncate the output_file at start (use the
output_file variable) so each run starts fresh, then after calling
filter_empty_values finish by applying chmod 600 to output_file to lock
permissions; additionally add ".env.bootstrap" to .gitignore to prevent
committing secrets.
Adds /healthz endpoint to the custom HTTP server for health checks.
- Returns JSON: {"status": "healthy", "service": "pmoves-publisher"}
- Existing /metrics, /metrics.prom, /metrics.json endpoints unchanged
- Enables Kubernetes liveness/readiness probes
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
POWERFULMOVES
changed the title
4 tasks
4 tasks
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
1 participant
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.
P4-P5 observability enhancements and universal credentials restoration for PMOVES.AI.
Changes
Gateway Services (P4):
/healthzand/metricsendpoints topmoves/services/gateway/metricsendpoint topmoves/services/gateway-agentPublisher Service (P5):
/healthzendpoint topmoves/services/publisherSecurity Fixes:
agent-zeroto run as non-root user (pmoves:pmovesUID/GID 65532)Credentials Loading:
scripts/bootstrap_credentials.shwith GitHub Secrets supportgithub,docker,chitproviders topmoves/scripts/env_setup.ps1Upstream Sync:
/metricsendpoint added (Laravel, no composer deps)Test Plan
/healthzendpoints return JSON status/metricsendpoints return Prometheus formatMetrics Coverage Improvement
Services with Health/Metrics Endpoints (11+)
Credentials Loading
Supported Sources:
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com
Summary by CodeRabbit
New Features
Security
Chores
✏️ Tip: You can customize this high-level summary in your review settings.