-
Notifications
You must be signed in to change notification settings - Fork 4
RORDEV-1629 reproduction #86
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.
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughKibana config updated to add trace-level logging; a new Bash script sets Elasticsearch disk watermarks via the cluster settings API; TLS certificate and private key files were added for the reverse proxy. Changes
Sequence Diagram(s)sequenceDiagram
participant Script as es-watermark-changes.sh
participant ES as Elasticsearch Cluster
Script->>ES: HTTPS PUT /_cluster/settings\nPayload: transient.cluster.routing.allocation.disk.watermark_* (5/10/15%)
Note right of Script: uses curl -k and -u <user:pass>
ES->>ES: Apply transient cluster settings
ES-->>Script: 200 OK / JSON response
rect rgba(100,200,100,0.12)
note over Script,ES: Watermarks updated (transient)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
xpack-docker-demo-cluster/conf/kibana.yml(1 hunks)xpack-docker-demo-cluster/es-watermark-changes.sh(1 hunks)
🔇 Additional comments (1)
xpack-docker-demo-cluster/conf/kibana.yml (1)
11-11: Trace-level logging enabled for debugging.Line 11 sets logging verbosity to
trace, which is appropriate for reproduction scenarios but generates high log volume. Ensure this is reverted before production deployment.
| AUTH="elastic:elastic" | ||
|
|
||
| # Set transient disk watermarks | ||
| curl -k -u $AUTH -X PUT "$ES_URL/_cluster/settings" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical security issue: Hardcoded credentials and insecure TLS verification.
This script contains multiple security vulnerabilities that must be addressed before use:
- Hardcoded credentials (Line 5): Credentials are embedded in plaintext and visible via process inspection (
ps aux), shell history, and logs. - Disabled SSL verification (Line 8): The
-kflag disables certificate verification, leaving the connection vulnerable to MITM attacks. - Exposed credentials in command line (Line 8): Using
-u $AUTHpasses credentials as a process argument, making them visible to other users on the system.
Refactor to use environment variables and proper credential handling:
-#!/bin/bash
-# Script to set Elasticsearch disk watermarks dynamically
-
-ES_URL="https://localhost:29200"
-AUTH="elastic:elastic"
-
-# Set transient disk watermarks
-curl -k -u $AUTH -X PUT "$ES_URL/_cluster/settings" \
+#!/bin/bash
+# Script to set Elasticsearch disk watermarks dynamically
+
+# Use environment variables for configuration
+ES_URL="${ES_URL:-https://localhost:9200}"
+ES_USERNAME="${ES_USERNAME:-elastic}"
+ES_PASSWORD="${ES_PASSWORD}"
+
+if [ -z "$ES_PASSWORD" ]; then
+ echo "Error: ES_PASSWORD environment variable is not set" >&2
+ exit 1
+fi
+
+# Set transient disk watermarks
+# Use --user for secure credential passing and verify SSL certificates
+curl -u "${ES_USERNAME}:${ES_PASSWORD}" -X PUT "$ES_URL/_cluster/settings" \
-H "Content-Type: application/json" \
-d '{
"transient": {
"cluster.routing.allocation.disk.watermark.low": "5%",
"cluster.routing.allocation.disk.watermark.high": "10%",
"cluster.routing.allocation.disk.watermark.flood_stage": "15%"
}
}'
+
+if [ $? -ne 0 ]; then
+ echo "Error: Failed to update Elasticsearch disk watermarks" >&2
+ exit 1
+fi
+
+echo "Successfully updated Elasticsearch disk watermarks"Alternatively, consider using a .netrc file or secrets management tooling (e.g., HashiCorp Vault, AWS Secrets Manager) for credential handling.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In xpack-docker-demo-cluster/es-watermark-changes.sh around lines 5 to 8, remove
the hardcoded AUTH variable and the use of curl -k -u $AUTH which exposes
credentials and disables TLS verification; instead read credentials from a
secure source (environment variables, a mounted secrets file, or a secrets
manager) and load them at runtime without embedding them in the command line,
and enable proper TLS verification by removing -k and ensuring a valid
CA/certificate is present or by pointing curl to a trusted --cacert/CA bundle;
if command-line auth must be avoided, use curl --netrc-file or pass an
Authorization header from a file/secure variable so credentials are never
visible in process listings or shell history.
| curl -k -u $AUTH -X PUT "$ES_URL/_cluster/settings" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{ | ||
| "transient": { | ||
| "cluster.routing.allocation.disk.watermark.low": "5%", | ||
| "cluster.routing.allocation.disk.watermark.high": "10%", | ||
| "cluster.routing.allocation.disk.watermark.flood_stage": "15%" | ||
| } | ||
| }' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling and input validation.
The script lacks error checking and does not validate whether the API call succeeded, potentially masking failures silently.
Apply this diff to add error handling:
# Set transient disk watermarks
-curl -k -u $AUTH -X PUT "$ES_URL/_cluster/settings" \
+RESPONSE=$(curl -s -w "\n%{http_code}" -u "${ES_USERNAME}:${ES_PASSWORD}" \
+ -X PUT "$ES_URL/_cluster/settings" \
-H "Content-Type: application/json" \
-d '{
"transient": {
"cluster.routing.allocation.disk.watermark.low": "5%",
"cluster.routing.allocation.disk.watermark.high": "10%",
"cluster.routing.allocation.disk.watermark.flood_stage": "15%"
}
- }'
+ }')
+
+HTTP_CODE=$(echo "$RESPONSE" | tail -n1)
+BODY=$(echo "$RESPONSE" | head -n-1)
+
+if [ "$HTTP_CODE" != "200" ]; then
+ echo "Error: API request failed with HTTP $HTTP_CODE" >&2
+ echo "Response: $BODY" >&2
+ exit 1
+fi
+
+echo "Successfully updated disk watermarks: $BODY"Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ror-demo-cluster/conf/reverse-proxy/certs/server.crt(1 hunks)ror-demo-cluster/conf/reverse-proxy/certs/server.key(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ror-demo-cluster/conf/reverse-proxy/certs/server.crt
🧰 Additional context used
🪛 Gitleaks (8.29.0)
ror-demo-cluster/conf/reverse-proxy/certs/server.key
[high] 1-28: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
| -----BEGIN PRIVATE KEY----- | ||
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC2QFVd4jCXP90e | ||
| xNQp+Fpzx2AQxUqj+q85yrkueqX5NjAAl/l+o3bK002TJyPIEFsJzyD5tpughQKB | ||
| KiNe7vqhlE5mUtD4faplixT0dfM8l/m6Sy5qDQc0Y2SXHtxFC1anc0Ur16v2kpzZ | ||
| 9FWwZOjwN1pn/KXbomJjCkk+GWM/ZcQD9Ylmuhpqi4vGVBFm0Wok3X7MjBPx62C4 | ||
| 3YVK0xztIRzxCLBwqxyWB9yfTyai+gSV7kzhImSskCQcV284fCgHpjlNYDXZtMdf | ||
| TwyfZ+BrYYkgzjdLgRBEyLMDBE8sLHRQQwX/56VmMhbEmMCChFUw7IGE4INdMrwj | ||
| HElmWhS1AgMBAAECggEAAaVzH/X7GmKpTK3afMaRipoyc/RUSEbrbko2ggT5mtay | ||
| eE7nIg239P0TplCkMhpzuBL26UqM/VY2P5Rx3VmrSepdCu+Uk6oO7/vhpJOsLs/w | ||
| oY4sTSjw97guIG9W7gi8L6cK6Op50zBf2lgqrf07XXAikO3nUaSV3u8o2jbAfsIY | ||
| pjT/XLWsYF3XYXfH8RMRKv+tQNhCAch2Un/tug3N1tXSTcbjGwIay97Ytj8pYPcd | ||
| +1hZay9t5MXa5CAVOusrwbfShQ+sXpWenH13DqvvQX8GA/zd8bA3UV5oaDvQpw3v | ||
| HTLMr7EiFsAExc7vPr6E8X8hLfju2BdKTu0i8z7fwQKBgQDw9eyfui3jSI2EE08H | ||
| wLvLT6pZSrJ2TIQd75c9mdL04YkCwxTMCufvZwYj7PG0srRdNNlO1UTxEQNoAJ03 | ||
| U2C1qjjOQG9X71+Fj1+mtv8V8KllpZFxiItySZOpLYJe4GbqXkef/dMG+/JZ6KwY | ||
| gJuEQeiXc+LBFAYWrx9goXoQRwKBgQDBoFmIEidr3020+JJEFvIOXzZ/OkcV2Do+ | ||
| C8K4/wqUvectYVRgHlH2tTcXW+7ngVrdqnPMFeBxylxymnD7yUIS2GuV6WXeqzPg | ||
| CPuzr8OiE634qmxemj3UpbRFO3ctU00/8SeOcbnnHTrBQhrnSBC8K4Y3bXutFhba | ||
| Hb3T2NfNIwKBgQCqPxdQQ1romvphtsK/14zXuRHCxOQScT1naUCSZXyHSFJlgS/Q | ||
| emQk4cWU3HRqF1kYAZ8H8+ch68NcWBK8ZEyQDhTUpPIGTzpOQ15xjBnuhnspNjHs | ||
| 5Wyg8xtBDMZwAly0eqhgghX3eUth+uKc7UDz0R1k5JvxjxQ+Mr0YqP51QQKBgCVm | ||
| hsS81PaGPKlPNlmGoRzbkVhD9oUmriFb/jHjubR/dg8S9MxYLvbbjBer/1qiZt3Y | ||
| VeO++gqgzvioEljgSC4Btc5QNggrw6prscrEkaJV8M3OhX5EhtrzYv3Yfa24oHxQ | ||
| ZZajKwiUYcwAq9eu2OSv943vTOtjD+jzw74O7ahHAoGAS3T27W6SkloZi9t+SFSY | ||
| BeINYvV3ciRNGN48k00v28+5sFJ7uaqkxkd+t8RGn7nGaA7n+nJNXexyiJzVVKjS | ||
| zL5WaQuduTDJHpW5HbAQp3bMpCrYhI6YL9EzXEedWkLaTh+Ox9yVc3nDBNx0lGw3 | ||
| yRdF/RqO80s0HbQhbJMtcMQ= | ||
| -----END PRIVATE KEY----- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🚨 Critical: Remove private key from version control immediately.
Committing private keys to any repository—even demo environments—is a critical security vulnerability. Once exposed in version control (especially public repositories), the key is compromised and should be rotated.
Actions required:
-
Remove this file from the repository:
git rm ror-demo-cluster/conf/reverse-proxy/certs/server.key
-
Add certificate/key files to
.gitignore:ror-demo-cluster/conf/reverse-proxy/certs/*.key ror-demo-cluster/conf/reverse-proxy/certs/*.pem -
Generate the private key locally during deployment (e.g., via a setup script):
openssl genrsa -out ror-demo-cluster/conf/reverse-proxy/certs/server.key 2048 openssl req -new -x509 -key ror-demo-cluster/conf/reverse-proxy/certs/server.key \ -out ror-demo-cluster/conf/reverse-proxy/certs/server.crt -days 365
-
Document the local generation process in setup instructions.
-
If this key was ever used in production, rotate it immediately.
Verify that the related certificate file (server.crt) is also not committed to the repository, or if it is, that it doesn't contain sensitive data.
🏁 Script executed:
#!/bin/bash
# Check if the private key file exists
if [ -f "ror-demo-cluster/conf/reverse-proxy/certs/server.key" ]; then
echo "=== Private key file EXISTS ==="
head -1 "ror-demo-cluster/conf/reverse-proxy/certs/server.key"
tail -1 "ror-demo-cluster/conf/reverse-proxy/certs/server.key"
else
echo "=== Private key file NOT FOUND ==="
fi
# Check if related certificate file exists
if [ -f "ror-demo-cluster/conf/reverse-proxy/certs/server.crt" ]; then
echo "=== Certificate file EXISTS ==="
head -1 "ror-demo-cluster/conf/reverse-proxy/certs/server.crt"
else
echo "=== Certificate file NOT FOUND ==="
fi
# Check .gitignore for certificate/key patterns
echo -e "\n=== Checking .gitignore for cert/key patterns ==="
if [ -f ".gitignore" ]; then
echo "Patterns matching 'key', 'crt', 'pem', or 'cert':"
grep -i -E '\.(key|crt|pem|cert)|certs/' .gitignore || echo "(no matches found)"
else
echo ".gitignore not found"
fi
# Check if certs directory exists
echo -e "\n=== Contents of certs directory ==="
if [ -d "ror-demo-cluster/conf/reverse-proxy/certs" ]; then
ls -la "ror-demo-cluster/conf/reverse-proxy/certs/" || echo "Unable to list"
else
echo "Certs directory not found"
fiLength of output: 533
🚨 Critical: Remove private key and certificate from version control immediately.
Committing private keys to any repository—even demo environments—is a critical security vulnerability. Once exposed in version control (especially public repositories), the key is compromised and must be rotated. The related certificate file is also present.
Actions required:
-
Remove both files from the repository:
git rm ror-demo-cluster/conf/reverse-proxy/certs/server.key git rm ror-demo-cluster/conf/reverse-proxy/certs/server.crt
-
Add certificate/key files to
.gitignore:ror-demo-cluster/conf/reverse-proxy/certs/*.key ror-demo-cluster/conf/reverse-proxy/certs/*.crt ror-demo-cluster/conf/reverse-proxy/certs/*.pem -
Generate the key and certificate locally during deployment:
openssl genrsa -out ror-demo-cluster/conf/reverse-proxy/certs/server.key 2048 openssl req -new -x509 -key ror-demo-cluster/conf/reverse-proxy/certs/server.key \ -out ror-demo-cluster/conf/reverse-proxy/certs/server.crt -days 365
-
Document the local generation process in setup instructions.
-
If this key was ever used in production, rotate it immediately.
🧰 Tools
🪛 Gitleaks (8.29.0)
[high] 1-28: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🤖 Prompt for AI Agents
In ror-demo-cluster/conf/reverse-proxy/certs/server.key lines 1-28: this file is
a private key committed to the repo; remove it and the matching certificate from
version control, add the cert/key patterns in
ror-demo-cluster/conf/reverse-proxy/certs to .gitignore, replace committed files
with locally/generated certs during deployment (generate with openssl or your
PKI tooling), document the local generation steps in the project setup docs, and
if this key was ever used in production rotate/revoke it immediately.
xpack-docker-demo-cluster/run.shxpack-docker-demo-cluster/es-watermark-changes.shSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.