Skip to content

[RUN-5039] Support docker --env-file; bake release version into image User-Agent - #105

Open
smartinellibenedetti wants to merge 5 commits into
mainfrom
env-file-support
Open

smartinellibenedetti wants to merge 5 commits into
mainfrom
env-file-support

Conversation

@smartinellibenedetti

@smartinellibenedetti smartinellibenedetti commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

About this change

Jira Ticket: RUN-5039

Purpose of the Changes

  • The Docker image already worked with --env-file (it only reads process.env), but docker run --env-file is stricter than dotenv: it keeps surrounding quotes and CRLF verbatim. src/config.ts now cleans env values (trim, strip one pair of surrounding quotes, empty → unset, trailing / dropped from URLs) so a dotenv-style file doesn't silently break requests.
  • Runlayer (internal) only supports --env-file. Docs now describe that flow: the connector reads ~/.rundeck-mcp/.env, so internal users just create the file there. Customers can still use -e variables or --env-file.
  • Docs: --env-file usage and format rules (SETUP.md, README, TECHNICAL-CAPABILITIES.md, CLAUDE.md, skills), .env.example rewritten as the shared template, .gitignore ignores .env.* except .env.example.
  • CI/Dockerfile: RUNDECK_MCP_VERSION build-arg bakes the release version into the image's User-Agent (the docker-build job does its own fresh checkout, so it can't rely on the build job's patch).

Kind of Change

  • Bug Fix
  • Enhancement/New Feature/Behavior
  • Maintenance/Refactor
  • Other... docs, CI

Development Checklist

  • npm run validate passes locally (build + test + integration validations)
  • Unit tests added/updated for modified code
  • New/changed tools, resources, or prompts documented in CLAUDE.md if applicable
  • New environment variables documented in CLAUDE.md's Environment Variables table (no new variables)
  • Docs download / RUNDECK_DOCS_PATH behavior unaffected, or changes called out below

npm test passes (28 suites, 394 tests); npm run validate was not run.

Testing

Testing setup:

  1. npm test
  2. docker build -t rundeck-mcp:envtest . then sh ci/docker-smoke-test.sh rundeck-mcp:envtest (new section 6 covers --env-file with a quoted URL and CRLF).
  3. docker run -d -i --env-file ~/.rundeck-mcp/.env rundeck-mcp:envtest, then docker exec <container> env | grep RUNDECK_.

Acceptance Criteria:

  • Container started with --env-file has the variables in its environment and reports healthy
  • Quoted/CRLF/trailing-slash values are normalized (unit tests)
  • Smoke test passes, including the new --env-file section
  • Runlayer connector reading ~/.rundeck-mcp/.env verified end to end (not tested from here)

🤖 Generated with Claude Code

smartinellibenedetti and others added 2 commits October 1, 2026 16:29
- config: normalize env values (trim, CR, one pair of surrounding quotes,
  empty -> unset, trailing slash on URLs) since `docker run --env-file`
  passes quotes/CRLF through verbatim; add tests
- smoke test: add --env-file case (quoted URL + CRLF)
- docs: document --env-file usage, recommended ~/.rundeck-mcp/.env location,
  format rules, and Runlayer setup; update .env.example, skills, CLAUDE.md
- gitignore: ignore .env.* except .env.example
- Dockerfile/CircleCI: pass RUNDECK_MCP_VERSION build-arg so the image's
  User-Agent carries the release version

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Runlayer (internal) only supports --env-file, configured to read the
standard ~/.rundeck-mcp/.env, so users just create the file there.
Customers can still use -e variables or --env-file.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 22:38
@smartinellibenedetti smartinellibenedetti changed the title Support docker --env-file; bake release version into image User-Agent [RUN-5039] Support docker --env-file; bake release version into image User-Agent Oct 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Registry normalization can accept unusable empty credentials, and several env-file instructions are inaccurate.

Review effort: Balanced
Findings: 2 Medium severity · 4 Low severity

Open (6)
What changed in this PR

Adds Docker --env-file support and embeds release versions into Docker image User-Agent headers.

Changes:

  • Normalizes core Rundeck environment configuration.
  • Documents env-file and Runlayer setup.
  • Updates Docker/CircleCI builds and smoke tests.
File Description
src/​config.ts Normalizes configuration values.
src/​__tests__/​config.test.ts Tests normalization behavior.
Dockerfile Bakes User-Agent version.
.circleci/​config.yml Supplies image release version.
ci/​docker-smoke-test.sh Exercises env-file startup.
.env.example Provides shared env template.
.gitignore Ignores env files.
README.md Documents env-file usage.
SETUP.md Adds Docker and Runlayer instructions.
TECHNICAL-CAPABILITIES.md Documents normalization.
CLAUDE.md Updates architecture guidance.
.claude/​skills/​rundeck-mcp-docker-setup/​SKILL.md Adds env-file setup guidance.
.claude/​skills/​rundeck-mcp-docker-build/​SKILL.md Adds env-file configuration example.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread SETUP.md
Comment thread src/config.ts Outdated
Comment thread .claude/skills/rundeck-mcp-docker-setup/SKILL.md Outdated
Comment thread .env.example Outdated
Comment thread README.md Outdated
Comment thread TECHNICAL-CAPABILITIES.md Outdated
@elquimeras

Copy link
Copy Markdown

Tested the image built from this branch locally (docker run -i --rm --env-file ~/.rundeck-mcp/.env):

  • initialize and tools/list work (9 tools); docs are fetched on startup.
  • docs_search returns results.
  • api_call works with the env-file config against a Rundeck 6.3 instance (read-only GETs: system/info, projects, project/{p}/jobs, runnerManagement/runners), including the upstream 403 returned as a normal tool response.

Not covered: write/destructive calls and the Runlayer connector (it currently fails before reaching the container because $HOME isn't expanded in its --env-file path, which is a connector config issue, not this PR).

@elquimeras elquimeras left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please take a look the copilot comments there's a few that could be important

…stry values, fix docs

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Entrypoint normalization remains inconsistent and the new Docker behaviors are not fully asserted by CI.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
Resolved since last review (5)
Previously missed (2)

In code that hasn't changed since last review

Medium severity CI does not verify build argument updates USER_AGENT

.circleci/​config.yml:167

No CI check verifies that this build argument actually changed the compiled USER_AGENT. Because sed exits successfully when its search text is absent, a later edit to the constant can make tagged builds silently publish rundeck-mcp/SNAPSHOT while every current smoke test passes. Import USER_AGENT from the built image and compare it with RUNDECK_MCP_VERSION after at least one local image build.

Medium severity Smoke test omits normalized URL and token configuration

ci/​docker-smoke-test.sh:127

This assertion only proves that the server can initialize; initialization does not consume RUNDECK_URL or RUNDECK_TOKEN, and the earlier log check only exercises RUNDECK_DOCS_PATH. Consequently, this new smoke section still passes if URL quote/CR/trailing-slash normalization is broken in the compiled image. Read configManager.getConfig() inside the built container and assert the normalized URL and token before the protocol check.

Comment thread docker-entrypoint.sh Outdated
…RUNDECK_DOCS_PATH

Strip surrounding quotes and treat empty/whitespace values as unset, matching
src/config.ts. Smoke test covers quoted branch, quoted-empty path, and quoted
non-empty path.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Release metadata lacks image-level verification, and credential guidance overstates isolation from agents.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Smoke tests do not verify the compiled image User-Agent version

Dockerfile:36

The new image-version behavior is not asserted by the smoke tests: the unit test only compares the header with the source constant, and branch builds legitimately use SNAPSHOT. If this sed target drifts or the build arg stops propagating, tagged images still build successfully with the wrong User-Agent. Add a smoke assertion that reads USER_AGENT from the compiled image and compares it with RUNDECK_MCP_VERSION.

Medium severity Restart diagnostic is misleading for intentionally empty environment values

src/​config.ts:37

Quoted-empty and whitespace-only values are intentionally treated as unset here, but src/tools/api.ts:75-79 still checks the raw process.env strings. For values such as RUNDECK_TOKEN='""', API calls now report that the variable “exists ... but wasn't loaded” and suggest restarting, even though restarting cannot fix an intentionally empty value. Base that diagnostic on the normalized value, or omit the restart note for values normalized to unset.

Low severity External env file location is overstated as an access boundary

.claude/​skills/​rundeck-mcp-docker-setup/​SKILL.md:183

Keeping the env file outside the project does not ensure that an agent never sees it: this skill itself instructs the agent to write the file, and an agent with filesystem or shell access may read files under the user's home directory. Since the file contains a live token, describe this location as reducing accidental repository-context and Git exposure rather than as an access boundary.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The version bake can silently fail, and the setup guidance incorrectly promises that the agent cannot see credentials.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Fail release build when User-Agent replacement is missing

Dockerfile:36

This replacement can silently stop working: sed exits successfully when the placeholder is absent, so a later refactor of USER_AGENT would still publish a release image labeled SNAPSHOT. Guard the expected source text before replacing it (or assert the compiled value afterward) so the release build fails instead of shipping the wrong User-Agent.

Low severity Do not claim repository storage prevents agent token access

.claude/​skills/​rundeck-mcp-docker-setup/​SKILL.md:183

The claim that the agent never sees this token is incorrect: this skill asks the user for the token in Step 4 and then instructs the agent to write it here. Keeping the file outside the repository prevents accidental commits, but it does not create an agent-access boundary; avoid promising otherwise so users understand the credential exposure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants