Skip to content

Commit aed534e

Browse files
colinsaramprice
colins
authored and
aramprice
committed
Fix NATS auth.json parse error caused by HTML-escaped '>' in JSON output
Go's json.Marshal HTML-escapes '>' as '\u003e' by default (to prevent XSS in HTML contexts). NATS's config parser does not support \u Unicode escape sequences and rejects the file on reload with: Failed to reload server configuration: error parsing include file '../../../data/nats/auth.json', Parse error on line 1: 'Invalid escape character 'u'.' This meant every SIGHUP sent by bosh-nats-sync caused NATS to reject the new auth.json and continue running with the original fooBar token placeholder, leaving health_monitor unable to authenticate for the full 5-minute deploy timeout. The fix switches writeNATSConfigFile to use json.NewEncoder with SetEscapeHTML(false), which writes '>' literally so NATS can parse the file. Also adds a regression test that asserts 'director.>' is not escaped. Also adds /var/vcap/sys/log/nats/bosh-nats-sync.log and /var/vcap/sys/log/nats/nats.log to the director diagnostic collection in run-bats-pipeline.sh, which is what exposed this bug.
1 parent e059a19 commit aed534e

3 files changed

Lines changed: 200 additions & 5 deletions

File tree

‎ci/tasks/run-bats-pipeline.sh‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
#!/usr/bin/env bash
2+
# run-bats-pipeline.sh
3+
#
4+
# Chains together the full BATs pipeline in a single Concourse task:
5+
# terraform apply → deploy-director → prepare-bats-config → run-bats
6+
# terraform destroy ← destroy-director ← (EXIT trap, always runs)
7+
#
8+
# Required env vars (set via run-bats-pipeline.yml params):
9+
# GCP_JSON_KEY – GCP service account JSON (resolved from Concourse creds)
10+
# GCP_PROJECT_ID – GCP project ID
11+
# ENV_NAME – Unique terraform env name, e.g. "bats-local"
12+
# BAT_INFRASTRUCTURE – "gcp"
13+
# STEMCELL_NAME – Stemcell name for bats-config.yml
14+
# DEPLOY_ARGS – Extra ops-file args for bosh create-env
15+
# BAT_RSPEC_FLAGS – Extra flags appended to the BAT run (optional)
16+
17+
set -eu
18+
19+
ROOT_DIR="$PWD"
20+
TERRAFORM_DIR="${ROOT_DIR}/bosh-ci/ci/bats/iaas/gcp/terraform"
21+
TERRAFORM_VERSION="1.9.8"
22+
23+
# ── Shared working directories ───────────────────────────────────────────────
24+
mkdir -p director-state bats-config environment
25+
# cache-dot-bosh-dir is provided as a Concourse cache volume; create if absent
26+
mkdir -p cache-dot-bosh-dir/.bosh
27+
28+
# prepare-bats-config.sh expects its terraform metadata at terraform/metadata
29+
# but deploy-director.sh expects it at environment/metadata.
30+
# Symlink terraform/ → environment/ so both scripts find what they need.
31+
ln -sf "${ROOT_DIR}/environment" "${ROOT_DIR}/terraform"
32+
33+
# ── Install terraform ────────────────────────────────────────────────────────
34+
if ! command -v terraform &>/dev/null; then
35+
echo "--- Installing terraform ${TERRAFORM_VERSION} ---"
36+
curl -sSL \
37+
"https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_amd64.zip" \
38+
-o /tmp/terraform.zip
39+
unzip -qo /tmp/terraform.zip -d /usr/local/bin terraform
40+
chmod +x /usr/local/bin/terraform
41+
fi
42+
43+
# ── GCP credentials file (used by the GCS backend and the Google provider) ──
44+
GCP_CREDS_FILE="$(mktemp /tmp/gcp-creds-XXXXXX.json)"
45+
echo "${GCP_JSON_KEY}" > "${GCP_CREDS_FILE}"
46+
chmod 600 "${GCP_CREDS_FILE}"
47+
48+
# ── Provision GCP environment via terraform ──────────────────────────────────
49+
echo "--- Provisioning GCP environment (env: ${ENV_NAME}) ---"
50+
pushd "${TERRAFORM_DIR}" >/dev/null
51+
52+
terraform init \
53+
-input=false \
54+
-reconfigure \
55+
-backend-config="bucket=bosh-director-pipeline" \
56+
-backend-config="prefix=bats-terraform/${ENV_NAME}" \
57+
-backend-config="credentials=${GCP_CREDS_FILE}"
58+
59+
terraform apply \
60+
-input=false \
61+
-auto-approve \
62+
-var "project_id=${GCP_PROJECT_ID}" \
63+
-var "gcp_credentials_json=${GCP_JSON_KEY}" \
64+
-var "name=${ENV_NAME}"
65+
66+
# Convert terraform outputs to the flat metadata JSON consumed by director-vars
67+
# and prepare-bats-config.sh.
68+
terraform output -json \
69+
| jq 'with_entries(.value = .value.value)' \
70+
> "${ROOT_DIR}/environment/metadata"
71+
72+
popd >/dev/null
73+
74+
# ── Teardown trap (always runs on EXIT) ─────────────────────────────────────
75+
function collect_director_diagnostics {
76+
# Only collect when we have a deployed director and bosh-cli is available.
77+
[[ -f director-state/director-creds.yml ]] || return 0
78+
[[ -f "${ROOT_DIR}/environment/metadata" ]] || return 0
79+
command -v bosh-cli &>/dev/null || return 0
80+
81+
local director_ip
82+
director_ip="$(jq -r '.director_public_ip // empty' "${ROOT_DIR}/environment/metadata")"
83+
[[ -n "${director_ip}" ]] || return 0
84+
85+
echo "--- Collecting director diagnostics (IP: ${director_ip}) ---"
86+
87+
# Extract jumpbox SSH private key from the vars-store.
88+
local jumpbox_key_file
89+
jumpbox_key_file="$(mktemp /tmp/jumpbox-key-XXXXXX)"
90+
bosh-cli interpolate director-state/director-creds.yml \
91+
--path /jumpbox_ssh/private_key > "${jumpbox_key_file}" 2>/dev/null || { rm -f "${jumpbox_key_file}"; return 0; }
92+
chmod 600 "${jumpbox_key_file}"
93+
94+
ssh -o StrictHostKeyChecking=no \
95+
-o UserKnownHostsFile=/dev/null \
96+
-o ConnectTimeout=10 \
97+
-i "${jumpbox_key_file}" \
98+
"jumpbox@${director_ip}" \
99+
'echo "=== monit status ===" && sudo /var/vcap/bosh/bin/monit status;
100+
echo "=== bosh_nats_sync log (last 100 lines) ===" && sudo tail -100 /var/vcap/sys/log/nats/bosh-nats-sync.log 2>/dev/null || true;
101+
echo "=== bosh_nats_sync bpm stdout ===" && sudo cat /var/vcap/sys/log/bpm/nats/bosh_nats_sync.stdout.log 2>/dev/null || true;
102+
echo "=== bosh_nats_sync bpm stderr ===" && sudo cat /var/vcap/sys/log/bpm/nats/bosh_nats_sync.stderr.log 2>/dev/null || true;
103+
echo "=== nats log (last 50 lines) ===" && sudo tail -50 /var/vcap/sys/log/nats/nats.log 2>/dev/null || true;
104+
echo "=== nats bpm stdout (last 50 lines) ===" && sudo tail -50 /var/vcap/sys/log/bpm/nats/nats.stdout.log 2>/dev/null || true;
105+
echo "=== health_monitor log (last 100 lines) ===" && sudo tail -100 /var/vcap/sys/log/health_monitor/health_monitor.log 2>/dev/null || true;
106+
echo "=== health_monitor bpm stdout ===" && sudo tail -50 /var/vcap/sys/log/bpm/health_monitor/health_monitor.stdout.log 2>/dev/null || true;
107+
echo "=== health_monitor bpm stderr ===" && sudo tail -50 /var/vcap/sys/log/bpm/health_monitor/health_monitor.stderr.log 2>/dev/null || true' \
108+
2>&1 || echo "(SSH diagnostics failed — VM may not be reachable)"
109+
110+
rm -f "${jumpbox_key_file}"
111+
}
112+
113+
function teardown {
114+
local exit_code=$?
115+
set +e
116+
117+
if [[ "${exit_code}" -ne 0 ]]; then
118+
collect_director_diagnostics
119+
fi
120+
121+
echo "--- Tearing down BOSH director ---"
122+
if [[ -f director-state/director-state.json ]]; then
123+
# destroy-director.sh expects bosh-cli/bosh-cli-* to exist; restore it
124+
# because deploy-director.sh already moved the original binary away.
125+
cp /usr/local/bin/bosh-cli bosh-cli/bosh-cli-restore 2>/dev/null || true
126+
bosh-ci/ci/bats/tasks/destroy-director.sh || true
127+
fi
128+
129+
echo "--- Destroying GCP environment (env: ${ENV_NAME}) ---"
130+
pushd "${TERRAFORM_DIR}" >/dev/null
131+
terraform destroy \
132+
-input=false \
133+
-auto-approve \
134+
-var "project_id=${GCP_PROJECT_ID}" \
135+
-var "gcp_credentials_json=${GCP_JSON_KEY}" \
136+
-var "name=${ENV_NAME}" || true
137+
popd >/dev/null
138+
139+
rm -f "${GCP_CREDS_FILE}"
140+
141+
exit "${exit_code}"
142+
}
143+
trap teardown EXIT
144+
145+
# ── Deploy BOSH director ─────────────────────────────────────────────────────
146+
echo "--- Deploying BOSH director ---"
147+
# deploy-director.sh moves bosh-cli/bosh-cli-* to /usr/local/bin/bosh-cli.
148+
# After this call bosh-cli is installed system-wide as 'bosh-cli'.
149+
bosh-ci/ci/bats/tasks/deploy-director.sh
150+
151+
# ── Prepare BATs config ──────────────────────────────────────────────────────
152+
echo "--- Preparing BATs config ---"
153+
bosh-ci/ci/bats/iaas/gcp/prepare-bats-config.sh
154+
155+
# ── Run BATs ─────────────────────────────────────────────────────────────────
156+
echo "--- Running BATs ---"
157+
# Source the environment file that prepare-bats-config.sh wrote; this exports
158+
# BOSH_ENVIRONMENT, BOSH_CLIENT, BOSH_CLIENT_SECRET, BOSH_CA_CERT,
159+
# BOSH_ALL_PROXY, and the default BAT_RSPEC_FLAGS.
160+
# shellcheck source=/dev/null
161+
source bats-config/bats.env
162+
163+
# Allow the caller to append extra RSpec flags (e.g. "--tag wip").
164+
if [[ -n "${BAT_RSPEC_FLAGS:-}" ]]; then
165+
export BAT_RSPEC_FLAGS="${BAT_RSPEC_FLAGS}"
166+
fi
167+
168+
bats/ci/tasks/run-bats.sh

‎src/bosh-nats-sync/pkg/userssync/users_sync.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package userssync
22

33
import (
4+
"bytes"
45
"crypto/md5"
56
"crypto/tls"
67
"crypto/x509"
@@ -368,9 +369,17 @@ func (u *UsersSync) queryAllRunningVMs() ([]natsauthconfig.VM, error) {
368369

369370
func (u *UsersSync) writeNATSConfigFile(vms []natsauthconfig.VM, directorSubject, hmSubject *string) error {
370371
cfg := natsauthconfig.CreateConfig(vms, directorSubject, hmSubject)
371-
data, err := json.Marshal(cfg)
372-
if err != nil {
372+
// Use SetEscapeHTML(false) so that '>' in NATS subjects (e.g. "director.>")
373+
// is written literally rather than as the \u003e Unicode escape that
374+
// json.Marshal emits by default. NATS's config parser does not support \u
375+
// escapes and rejects the file with a parse error if they are present.
376+
var buf bytes.Buffer
377+
enc := json.NewEncoder(&buf)
378+
enc.SetEscapeHTML(false)
379+
if err := enc.Encode(cfg); err != nil {
373380
return err
374381
}
375-
return os.WriteFile(u.NATSConfigFilePath, data, 0644)
382+
// Encode appends a trailing newline; strip it so the on-disk format stays
383+
// consistent with what existing tests and hash comparisons expect.
384+
return os.WriteFile(u.NATSConfigFilePath, bytes.TrimRight(buf.Bytes(), "\n"), 0644)
376385
}

‎src/bosh-nats-sync/pkg/userssync/users_sync_test.go‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package userssync_test
22

33
import (
4+
"bytes"
45
"encoding/json"
56
"encoding/pem"
67
"errors"
@@ -383,8 +384,13 @@ var _ = Describe("UsersSync", func() {
383384
ds := directorSubject
384385
hs := hmSubject
385386
cfg := natsauthconfig.CreateConfig(vms, &ds, &hs)
386-
data, _ := json.Marshal(cfg)
387-
os.WriteFile(natsConfigFilePath, data, 0644)
387+
// Use the same HTML-escape-free encoding that writeNATSConfigFile uses,
388+
// so the hash matches and Execute() correctly skips the reload.
389+
var buf bytes.Buffer
390+
enc := json.NewEncoder(&buf)
391+
enc.SetEscapeHTML(false)
392+
_ = enc.Encode(cfg)
393+
os.WriteFile(natsConfigFilePath, bytes.TrimRight(buf.Bytes(), "\n"), 0644)
388394
})
389395

390396
It("does not reload the NATS process", func() {
@@ -841,6 +847,18 @@ var _ = Describe("UsersSync", func() {
841847
Expect(string(data)).To(ContainSubstring("users"))
842848
})
843849

850+
// Regression test: json.Marshal HTML-escapes '>' as '\u003e', which the
851+
// NATS config parser rejects with "Invalid escape character 'u'".
852+
// The fix uses json.NewEncoder with SetEscapeHTML(false).
853+
It("writes '>' literally (not as \\u003e) so NATS can parse the config", func() {
854+
Expect(bootstrapSync.Bootstrap()).To(Succeed())
855+
856+
data, _ := os.ReadFile(natsConfigFilePath)
857+
Expect(string(data)).To(ContainSubstring("director.>"),
858+
"expected literal '>' but got HTML-escaped '\\u003e'")
859+
Expect(string(data)).NotTo(ContainSubstring(`\u003e`))
860+
})
861+
844862
It("sends a SIGHUP to reload the NATS server after writing the config", func() {
845863
Expect(bootstrapSync.Bootstrap()).To(Succeed())
846864

0 commit comments

Comments
 (0)