feat: capture devcontainer email to local file#3097
Conversation
Route devcontainer Postfix mail through a local capture transport and write full raw emails to /var/log/carlos-mail-capture.eml instead of discarding them or attempting outbound SMTP delivery. The dev docs claimed local Postfix discarded mail, but the config never enforced discard or prevented normal outbound smtp delivery. This makes provider-to-patient email testing safe and observable: every accepted message is written locally and nothing leaves the container. - Add postfix-capture-mail pipe script (runs as nobody) that appends the full MIME message plus envelope metadata to the capture file - Register a devcapture pipe transport in master.cf and route all mail to it via transport_maps regexp plus default/relay/local_transport overrides (defense in depth); keep inet_interfaces=loopback-only - Add mail capture/show/clear/test helper subcommands and surface the capture-file location in start/restart/status - Rewrite docs/postfix-mail-server.md to describe capture behavior (not discard), with a privacy warning and routing-verification steps - Add a devcontainer README pointer and list the new mail subcommands in help-dev Relates to #3096 Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Ben-Heerema, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaces the prior Postfix discard-transport dev setup with a pipe-based mail capture pipeline. The Dockerfile installs a capture script and configures all Postfix transports to route through ChangesPostfix dev mail capture pipeline
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 review |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
|
✅ Action performedReview finished.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a local email capture system for CARLOS EMR development within the devcontainer, routing all outbound emails to a local capture file (/var/log/carlos-mail-capture.eml) instead of sending them externally. It also adds new helper commands to the mail script and updates the documentation. The review feedback highlights three permission-related issues where non-root devcontainer users (such as vscode) would encounter 'Permission denied' errors when trying to read, tail, or touch the capture file and Postfix logs. The reviewer suggests changing the capture file permissions to 0666 and handling file existence checks more gracefully in the mail script to prevent crashes under set -e.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 4/5
- In
.devcontainer/development/scripts/mail, setting the capture file tochmod 0666makes it world-writable instead of the documented0660, which can let unrelated processes/users tamper with captured mail in the dev environment and make debugging output untrustworthy—switch tochown nobody:nogroupfollowed bychmod 0660before merging.
Architecture diagram
sequenceDiagram
participant App as CARLOS Application
participant Port25 as localhost:25
participant Postfix as Postfix SMTP
participant DevCapture as devcapture Pipe Transport
participant CaptureScript as /usr/local/bin/postfix-capture-mail
participant CaptureFile as /var/log/carlos-mail-capture.eml
participant MailLog as /var/log/mail.log
participant MailHelper as mail CLI Helper
Note over App,MailHelper: Devcontainer Local Email Capture Flow
App->>Port25: Send email to localhost:25 (SMTP)
Port25->>Postfix: Accept connection (loopback-only)
Postfix->>Postfix: Check inet_interfaces=loopback-only
alt Connection from localhost
Postfix->>Postfix: Accept message
Postfix->>Postfix: Lookup transport_maps regexp:/etc/postfix/carlos-transport-regexp
Postfix->>Postfix: Match /.*/ to devcapture: transport
Note over Postfix: Defense in depth: default_transport/devcapture:, relay_transport/devcapture:, local_transport/devcapture: catch any misses
Postfix->>DevCapture: Route message via devcapture pipe
DevCapture->>CaptureScript: Execute as user nobody
Note over CaptureScript: Arguments: ${sender} ${recipient}
CaptureScript->>CaptureFile: Append envelope metadata + raw MIME message
CaptureScript-->>DevCapture: exit 0
DevCapture-->>Postfix: Delivery status sent
Postfix->>MailLog: Log delivery event
Note over CaptureFile: Writes: Captured-At, Envelope-From, Envelope-To, BEGIN RAW MESSAGE, stdin (MIME), END RAW MESSAGE
MailHelper->>CaptureFile: mail capture (tail -f)
MailHelper->>CaptureFile: mail show (cat)
MailHelper->>CaptureFile: mail clear (: > file)
MailHelper->>Postfix: mail test (sendmail test message)
else Connection from external host
Postfix->>Postfix: Reject connection (inet_interfaces=loopback-only)
Postfix->>MailLog: Log rejected connection
end
Note over App,CaptureFile: Captured emails contain full raw MIME including PHI - sensitive content warning
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The mail helper's ensure_capture_file widened the capture file to 0666, diverging from the 0660 the image sets and documents. Restore the image's nobody:nogroup ownership and 0660 mode when recreating the file so the Postfix pipe user can still append without making it world-writable. Guard the touch so a missing file never aborts the script under set -e. Addresses cubic-dev-ai review finding on the PR. Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
|
@coderabbitai review |
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant CARLOS as CARLOS Application
participant SMTP as Postfix smtpd
participant QMGR as Postfix Queue Manager
participant Transport as transport_maps (regexp)
participant Pipe as devcapture pipe transport
participant CaptureScript as postfix-capture-mail (nobody)
participant CaptureFile as /var/log/carlos-mail-capture.eml
participant Developer as Developer (mail helper)
Note over CARLOS,CaptureFile: Local email capture flow (devcontainer only)
CARLOS->>SMTP: SMTP connect localhost:25 (MAIL FROM, RCPT TO, DATA)
SMTP->>SMTP: inet_interfaces=loopback-only (container only)
SMTP->>QMGR: Accept and queue message
QMGR->>Transport: Lookup recipient
Transport-->>QMGR: /.*/ devcapture: (regexp match)
alt defense in depth (fallbacks)
QMGR->>QMGR: default_transport=devcapture:
QMGR->>QMGR: relay_transport=devcapture:
QMGR->>QMGR: local_transport=devcapture:
end
QMGR->>Pipe: trigger devcapture unix pipe
Pipe->>CaptureScript: argv=postfix-capture-mail ${sender} ${recipient}
Note over Pipe,CaptureScript: stdin carries full raw MIME message
CaptureScript->>CaptureScript: printf -- '...' (fixed bug: use printf --)
CaptureScript->>CaptureFile: Append envelope metadata + raw MIME + separators
CaptureScript-->>Pipe: exit 0
Pipe-->>QMGR: status=sent (no external delivery)
Developer->>CaptureScript: mail capture (tail -f)
CaptureScript->>CaptureScript: ensure_capture_file (chown nobody:nogroup 0660)
CaptureScript->>CaptureFile: tail -f updates (if CARLOS sends again)
|
@coderabbitai review |
There was a problem hiding this comment.
Sorry @Ben-Heerema, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.devcontainer/development/Dockerfile:
- Around line 135-143: The mail-capture append block is not safe under
concurrent deliveries, so multiple `devcapture` processes can interleave writes
and corrupt the captured MIME. Update the capture logic in the devcontainer
Dockerfile’s `devcapture` handler to take an exclusive lock around the entire
`printf`/`cat` append sequence to `$CAPTURE_FILE`, using a single locked
critical section so each message is written atomically.
- Around line 156-159: The postfix capture service definition in the Dockerfile
is using pipe flags that alter the message stream before it reaches
postfix-capture-mail. Update the RUN that appends the devcapture entry to
master.cf so the pipe flags no longer include F, keeping the stdin payload raw
for /usr/local/bin/postfix-capture-mail while preserving the rest of the service
configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f05f12c-7d22-44a2-829b-58dd04fb32aa
📒 Files selected for processing (5)
.devcontainer/README.md.devcontainer/development/Dockerfile.devcontainer/development/scripts/help-dev.devcontainer/development/scripts/maildocs/postfix-mail-server.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: cubic · AI code reviewer
⚠️ CI failures not shown inline (8)
GitHub Actions: DCO / DCO Sign-Off: feat: capture devcontainer email to local file
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mpr_json=$(curl -fsSL \�[0m
�[36;1m -H "Authorization: ***" \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m "https://api.github.com/repos/$REPO/pulls/$PR_NUMBER")�[0m
�[36;1mpr_commit_count=$(jq -r '.commits' <<< "$pr_json")�[0m
�[36;1mbase_ref=$(jq -r '.base.ref' <<< "$pr_json")�[0m
�[36;1m�[0m
�[36;1mif [ "$pr_commit_count" -gt 250 ]; then�[0m
�[36;1m echo "::error::CARLOS EMR DCO check cannot automatically verify $pr_commit_count commits because GitHub's pull request commits API is capped at 250 commits."�[0m
GitHub Actions: DCO / 0_DCO Sign-Off.txt: feat: capture devcontainer email to local file
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mpr_json=$(curl -fsSL \�[0m
�[36;1m -H "Authorization: ***" \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m "https://api.github.com/repos/$REPO/pulls/$PR_NUMBER")�[0m
�[36;1mpr_commit_count=$(jq -r '.commits' <<< "$pr_json")�[0m
�[36;1mbase_ref=$(jq -r '.base.ref' <<< "$pr_json")�[0m
�[36;1m�[0m
�[36;1mif [ "$pr_commit_count" -gt 250 ]; then�[0m
�[36;1m echo "::error::CARLOS EMR DCO check cannot automatically verify $pr_commit_count commits because GitHub's pull request commits API is capped at 250 commits."�[0m
GitHub Actions: Automatic Dependency Submission (Maven) / 0_submit-maven.txt: Automatic Dependency Submission (Maven)
Conclusion: failure
##[group]Run if command -v mvn &> /dev/null; then
�[36;1mif command -v mvn &> /dev/null; then�[0m
�[36;1m version=$(mvn -q -v)�[0m
�[36;1m echo "ignore-wrapper=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1melif [[ -x ./mvnw ]]; then�[0m
�[36;1m version=$(./mvnw -q -v)�[0m
�[36;1m echo "ignore-wrapper=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1melse�[0m
�[36;1m echo "::error::No Maven executable found. Please ensure Maven is installed or a Maven wrapper is present."�[0m
GitHub Actions: Automatic Dependency Submission (Maven) / submit-maven: Automatic Dependency Submission (Maven)
Conclusion: failure
##[group]Run if command -v mvn &> /dev/null; then
�[36;1mif command -v mvn &> /dev/null; then�[0m
�[36;1m version=$(mvn -q -v)�[0m
�[36;1m echo "ignore-wrapper=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1melif [[ -x ./mvnw ]]; then�[0m
�[36;1m version=$(./mvnw -q -v)�[0m
�[36;1m echo "ignore-wrapper=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1melse�[0m
�[36;1m echo "::error::No Maven executable found. Please ensure Maven is installed or a Maven wrapper is present."�[0m
GitHub Actions: SonarCloud Analysis / Build and analyze: feat: capture devcontainer email to local file
Conclusion: failure
##[group]Run if [ -z "$SONAR_TOKEN" ]; then
�[36;1mif [ -z "$SONAR_TOKEN" ]; then�[0m
�[36;1m echo "::error::SONAR_TOKEN is not available in this workflow run."�[0m
GitHub Actions: SonarCloud Analysis / Build and analyze: feat: capture devcontainer email to local file
Conclusion: failure
Node 20 is being deprecated. This workflow is running with Node 24 by default. If you need to temporarily use Node 20, you can set the ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true environment variable. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
Post job cleanup.
(node:3894) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
[command]/usr/bin/tar --posix -cf cache.tzst --exclude cache.tzst -P -C /home/runner/work/carlos/carlos --files-from manifest.txt --use-compress-program zstdmt
/usr/bin/tar: .sonar-cache/06f6abcba0e155ab188fa8a3d1d759b8/sonar-java-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/95f6c184575e3be4647b786069d6d682/sonar-securityjavafrontend-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/be77970dffb3af77b743873d2acfb1b2/sonar-dbdjavafrontend-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/c6ef680a93f0d72a08c373315db8eed6/sonar-xml-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/1d9ea99bf4667c88719b48067a250a82/sonar-architecturejavafrontend-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/0d405f7b8a964f21eae0e37f5ed11150/sonar-iac-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/c5530adf25b595550fc6dda8e33d4291/sonar-jasmin-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/b577e400c3b63c02eafc2c98c9f0f197/sonar-textenterprise-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/bc69b468a9274275cc4e49a3f8135d5d/sonar-architecture-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/38784d6093b56436b7506d76adff42dd/sonar-javaa3scontextcollector-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/a15aa695be750f8207145691ef64d6fe/sona...
GitHub Actions: SonarCloud Analysis / Build and analyze: feat: capture devcontainer email to local file
Conclusion: failure
884 Invalid character encountered in file /workspace/src/main/resources/oscar/form/prop/mentalHealthForm42.pdf at line 2 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 16:22:42.886 Invalid character encountered in file /workspace/src/main/webapp/webfonts/fa-solid-900.woff2 at line 2 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 16:22:42.900 Invalid character encountered in file /workspace/src/main/webapp/mcedt/mailbox/img/spinner.gif at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 16:22:42.900 Invalid character encountered in file /workspace/src/main/webapp/images/Logo_old.png at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 16:22:42.904 Invalid character encountered in file /workspace/src/main/webapp/form/graphics/SocialHistory.gif at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 16:22:42.904 Invalid character encountered in file /workspace/src/main/webapp/oscarReport/img/plusbottom.gif at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 16:22:42.907 Invalid character encountered in file /workspace/src/main/resources/oscar/form/prop/GrowthChartRourke2009Girlspg2.pdf at line 2 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 16:22:42.925 Invalid character encountered in file /workspace/src/main/webapp/images/icons/valid.png at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARN...
GitHub Actions: SonarCloud Analysis / 0_Build and analyze.txt: feat: capture devcontainer email to local file
Conclusion: failure
##[group]Run if [ -z "$SONAR_TOKEN" ]; then
�[36;1mif [ -z "$SONAR_TOKEN" ]; then�[0m
�[36;1m echo "::error::SONAR_TOKEN is not available in this workflow run."�[0m
🧰 Additional context used
📓 Path-based instructions (2)
{README.md,docs/**/*.md,src/main/webapp/**/*.jsp,src/main/webapp/**/*.html}
📄 CodeRabbit inference engine (GEMINI.md)
Refer to this system as 'CARLOS EMR' or 'CARLOS' in all user-facing content (UI, documentation, README files, issue templates, workflows)
Files:
docs/postfix-mail-server.md
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve all existing copyright headers in modified files; do not remove or change GPL version statements
Files:
docs/postfix-mail-server.md
🪛 Hadolint (2.14.0)
.devcontainer/development/Dockerfile
[info] 127-127: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[info] 156-156: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
[info] 162-162: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant CARLOS as CARLOS Application
participant SMTP as Postfix smtpd
participant QMGR as Postfix Queue Manager
participant Transport as transport_maps (regexp)
participant Pipe as devcapture pipe transport
participant CaptureScript as postfix-capture-mail (nobody)
participant CaptureFile as /var/log/carlos-mail-capture.eml
participant Developer as Developer (mail helper)
Note over CARLOS,CaptureFile: Local email capture flow (devcontainer only)
CARLOS->>SMTP: SMTP connect localhost:25 (MAIL FROM, RCPT TO, DATA)
SMTP->>SMTP: inet_interfaces=loopback-only (container only)
SMTP->>QMGR: Accept and queue message
QMGR->>Transport: Lookup recipient
Transport-->>QMGR: /.*/ devcapture: (regexp match)
alt defense in depth (fallbacks)
QMGR->>QMGR: default_transport=devcapture:
QMGR->>QMGR: relay_transport=devcapture:
QMGR->>QMGR: local_transport=devcapture:
end
QMGR->>Pipe: trigger devcapture unix pipe
Pipe->>CaptureScript: argv=postfix-capture-mail ${sender} ${recipient}
Note over Pipe,CaptureScript: stdin carries full raw MIME message
CaptureScript->>CaptureScript: printf -- '...' (fixed bug: use printf --)
CaptureScript->>CaptureFile: Append envelope metadata + raw MIME + separators
CaptureScript-->>Pipe: exit 0
Pipe-->>QMGR: status=sent (no external delivery)
Developer->>CaptureScript: mail capture (tail -f)
CaptureScript->>CaptureScript: ensure_capture_file (chown nobody:nogroup 0660)
CaptureScript->>CaptureFile: tail -f updates (if CARLOS sends again)
Address CodeRabbit review findings on the devcapture pipe: - Serialize the capture append under an exclusive flock so concurrent devcapture deliveries can no longer interleave and corrupt the captured MIME. The capture file's own append-mode fd is the lock object, so no separate lock file (and no extra permission setup) is needed; falls back to a plain append if flock is absent. - Drop the 'F' pipe flag (keep 'q'). 'F' prepended an mbox-style "From " envelope line before the capture script read stdin, polluting the raw MIME payload the capture file is meant to preserve. Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
|
@coderabbitai review |
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant Dev as Developer
participant Mail as mail CLI Helper
participant Carlos as CARLOS App
participant Postfix as Postfix SMTP
participant Capture as capture script (nobody)
participant CaptureFile as /var/log/carlos-mail-capture.eml
Note over Dev,CaptureFile: NEW: All devcontainer mail is captured locally
Dev->>Mail: mail start
Mail->>CaptureFile: ensure exists, chown nobody:nogroup 0660
Mail->>Postfix: service postfix start
Mail-->>Dev: "Capture file: ..."
Dev->>Mail: mail capture (or mail show / clear)
Carlos->>Postfix: SMTP to localhost:25 (seeded emailConfig)
Postfix->>Postfix: accept from loopback-only
Note over Postfix: transport_maps=regexp:/etc/postfix/carlos-transport-regexp<br/>default/relay/local=devcapture:
Postfix->>Capture: pipe: argv=postfix-capture-mail ${sender} ${recipient}
Capture->>CaptureFile: flock(9) 9>> capture.eml
Note over Capture: append envelope + raw MIME from stdin
Capture-->>Postfix: exit 0
Postfix-->>Carlos: delivered via pipe
alt Developer sends test message
Dev->>Mail: mail test
Mail->>Postfix: sendmail -f noreply@openo-dev.local patient@example.test
Postfix->>Capture: same pipe flow
Capture->>CaptureFile: append test message
Mail-->>Dev: "Run: mail capture"
Dev->>Mail: mail capture
Mail->>CaptureFile: tail -f
end
Dev->>Mail: mail clear
Mail->>CaptureFile: truncate to 0 bytes
Mail-->>Dev: "Cleared"
Keep capture-everything as the default (nothing leaves the container), but allow a developer to deliver a specific test message for real when they explicitly choose to. Real delivery requires BOTH a runtime env flag and an allowlisted recipient, so it cannot happen by accident. - Ship an empty recipient allowlist (/etc/postfix/carlos-send-allowlist); with CARLOS_MAIL_ALLOW_SEND=1 the 'mail' helper wires it into transport_maps ahead of the capture map, so only allowlisted recipients route to smtp while every other recipient is still captured. Empty allowlist => nothing sends. - Optional CARLOS_MAIL_RELAYHOST sets an SMTP relay for real delivery. - 'mail status' reports the active delivery mode and the current allowlist; 'mail start'/'restart' print a warning banner when send mode is active. - Document the opt-in flow with a PHI warning; update help-dev. Default behavior is unchanged: without the flag, transport routing is the same capture-only configuration as before. Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
|
@coderabbitai review |
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
2 issues found across 5 files
Confidence score: 4/5
- In
.devcontainer/development/scripts/mail, leavingrelayhostunchanged whenCARLOS_MAIL_RELAYHOSTis unset can make real-send mode silently reuse a stale relay from a prior devcontainer session, which could route mail through the wrong host — clearrelayhostexplicitly when the env var is empty before merging. - In
.devcontainer/development/Dockerfile, the capture metadata path currently keeps only the first envelope recipient when Postfix sends multiple recipients, so devcapture records can be incomplete and mislead debugging — either store all recipient arguments or enforce single-recipient delivery fordevcapturebefore merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".devcontainer/development/Dockerfile">
<violation number="1" location=".devcontainer/development/Dockerfile:133">
P3: Capture metadata drops all but the first envelope recipient when Postfix batches multiple recipients. Record all recipient arguments or force single-recipient delivery for `devcapture`.</violation>
</file>
<file name=".devcontainer/development/scripts/mail">
<violation number="1" location=".devcontainer/development/scripts/mail:35">
P2: Clear `relayhost` when `CARLOS_MAIL_RELAYHOST` is unset. Otherwise real-send mode can silently reuse a stale relay from an earlier devcontainer session.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Dev as Developer
participant Mail as Mail Helper Script
participant Postfix as Postfix Service
participant CaptureScript as Capture Script (postfix-capture-mail)
participant CapFile as Capture File (/var/log/carlos-mail-capture.eml)
participant Allowlist as Allowlist File
participant Carlos as CARLOS Application
participant ExtSMTP as External SMTP (relay)
Note over Dev,Carlos: Default Capture-Only Flow
Dev->>Mail: mail start
Mail->>Mail: configure_send_mode()<br/>CARLOS_MAIL_ALLOW_SEND unset →<br/>transport_maps = capture-only regexp
Mail->>Postfix: service postfix start
Postfix-->>Mail: started
Mail-->>Dev: Capture file location
Carlos->>Postfix: send email to localhost:25
Postfix->>Postfix: smtpd accepts (loopback-only)<br/>transport_maps: /.*/ devcapture:
Postfix->>CaptureScript: pipe message (flags=um, user=nobody)<br/>with envelope sender/recipient
CaptureScript->>CaptureScript: flock exclusive lock<br/>on capture file
CaptureScript->>CapFile: append raw MIME + envelope metadata<br/>(atomically)
CaptureScript-->>Postfix: exit 0
Postfix-->>Carlos: delivered to transport
Note over Dev,Carlos: Opt-in Real Delivery Flow
Dev->>Mail: CARLOS_MAIL_ALLOW_SEND=1 mail start
Mail->>Mail: configure_send_mode()<br/>if flag=1 and allowlist has entries →<br/>transport_maps = allowlist regexp,<br/>then capture regexp
alt Allowlist non-empty
Mail->>Allowlist: read entries
Allowlist-->>Mail: recipient patterns
Mail->>Postfix: service postfix start<br/>with relayhost if set
Postfix-->>Mail: started
Mail-->>Dev: warning: real send enabled
else Allowlist empty
Mail->>Postfix: service postfix start<br/>(transport still capture-only)
Postfix-->>Mail: started
Mail-->>Dev: warning: allowlist empty, nothing will send
end
Carlos->>Postfix: send email to localhost:25
Postfix->>Postfix: smtpd accepts<br/>transport_maps: check allowlist first
alt Recipient matches allowlist entry
Postfix->>ExtSMTP: smtp: deliver externally
ExtSMTP-->>Postfix: delivered
else No match
Postfix->>CaptureScript: pipe to devcapture
CaptureScript->>CapFile: append captured email
end
Postfix-->>Carlos: delivered (mode noted)
Note over Dev: Developer Commands (test)
Dev->>Mail: mail test
Mail->>Mail: generate test message via sendmail
Mail->>Postfix: send test message
Postfix->>CaptureScript: pipe to devcapture (default)
CaptureScript->>CapFile: append test message
Mail-->>Dev: "Sent. Run: mail capture"
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…capture)
Address cubic review findings on the opt-in real-delivery path:
- Set relayhost from CARLOS_MAIL_RELAYHOST on every `mail start`/`restart`,
clearing it when the variable is unset, so a stale relay from an earlier
devcontainer session can't be silently reused in real-send mode.
- Set devcapture_destination_recipient_limit=1 so Postfix invokes the capture
pipe once per recipient; the script's ${recipient} (and the Envelope-To
metadata line) is then always the single correct address even when Postfix
batches multiple recipients for a message.
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
|
@coderabbitai review |
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant Dev as Developer (shell)
participant Mail as mail helper script
participant Postfix as Postfix MTA
participant CapturePipe as capture pipe (postfix-capture-mail)
participant CaptureFile as /var/log/carlos-mail-capture.eml
participant Allowlist as /etc/postfix/carlos-send-allowlist
participant ExtSMTP as External SMTP
Note over Dev,ExtSMTP: Devcontainer mail capture system (PR #3096)
rect rgb(240, 240, 240)
Note over Dev,Postfix: Initial setup (mail start / restart)
Dev->>Mail: mail start / mail restart
Mail->>Mail: configure_send_mode()
alt CARLOS_MAIL_ALLOW_SEND=1
Mail->>Postfix: postconf transport_maps=regexp:allowlist, regexp:carlos-transport-regexp
Mail->>Postfix: postconf relayhost=$CARLOS_MAIL_RELAYHOST
Note over Mail: Warns that real send is enabled
else default (flag unset)
Mail->>Postfix: postconf transport_maps=regexp:carlos-transport-regexp
end
Mail->>Postfix: service postfix start/restart
Postfix-->>Mail: started
Mail-->>Dev: status + capture file location
end
Note over Dev,ExtSMTP: Per-email flow (CARLOS app sends via localhost:25)
CarApp->>Postfix: SMTP (localhost:25) | to: patient@example.test
Postfix->>Postfix: Accept, inet_interfaces=loopback-only
Postfix->>Postfix: Lookup transport_maps
alt Default (capture-only mode)
Postfix->>Postfix: Match /.*/ –> devcapture:
Postfix->>CapturePipe: Pipe delivery (flags=q user=nobody)
Note over CapturePipe: Arguments: ${sender} ${recipient}
CapturePipe->>CapturePipe: flock on capture file
CapturePipe->>CaptureFile: Write envelope metadata + raw MIME
CaptureFile-->>CapturePipe: success (exit 0)
CapturePipe-->>Postfix: delivery complete
Postfix-->>CarApp: 250 OK (queued as captured)
else Opt-in send mode (CARLOS_MAIL_ALLOW_SEND=1)
Postfix->>Postfix: Match against allowlist first
alt Recipient matches allowlist (e.g., /^me@test\.com$/ smtp:)
Postfix->>ExtSMTP: smtp delivery to external MTA
ExtSMTP-->>Postfix: 250 OK
Note over Postfix: Recipient was delivered externally
else Recipient does NOT match allowlist
Postfix->>Postfix: Fallback to carlos-transport-regexp (capture)
Postfix->>CapturePipe: Pipe delivery
CapturePipe->>CaptureFile: Write captured email
CaptureFile-->>CapturePipe: success
CapturePipe-->>Postfix: done
Note over Postfix: Captured locally even in send mode
end
Postfix-->>CarApp: 250 OK
end
rect rgb(245, 245, 245)
Note over Dev,CaptureFile: Inspection commands
Dev->>Mail: mail capture / mail show
Mail->>CaptureFile: tail / cat (via ensure_capture_file)
CaptureFile-->>Dev: raw MIME content
end
|
There was a problem hiding this comment.
Sorry @Ben-Heerema, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
@yingbull I think this is working but I tried to restart the container off this branch and was confused. Can you help in how I would go about testing this? |
|
you should be able to script a test to the smtp port? |
|
To replicate the manual mail-capture test for this PR:
docker exec -it carlos-tomcat-dev mail start
docker exec -it carlos-tomcat-dev mail capture
The captured message is written to: This verifies that CARLOS sends to local Postfix and the devcontainer captures the message locally, including headers, body, and attachments, instead of delivering it externally. |
|
@yingbull I am happy with the testing now! It should be working as currently intended now. |
2 participants
Summary
/var/log/carlos-mail-capture.emlinstead of discarding them or attempting outbound SMTP delivery.mail capture,mail show,mail clear, andmail testhelper commands.Confined to
.devcontainer/anddocs/; no production or CARLOS application email code changes.Why
The dev docs claimed local Postfix discarded all mail, but the config never enforced discard or prevented normal outbound SMTP delivery. Capturing mail locally by default gives a reliable, safe way to inspect generated provider-to-patient emails (headers, body, base64 attachments) without anything leaving the container.
Behavior
Default (no env flag): CARLOS sends to
localhost:25→ Postfix accepts localhost-only → atransport_mapsregexp (/.*/ → devcapture) plusdefault/relay/local_transport=devcaptureoverrides route every recipient to thedevcapturepipe → the pipe writes the raw MIME (atomically, underflock) to the capture file.inet_interfaces=loopback-onlykeeps port 25 container-only. Nothing is delivered externally.Opt-in real delivery (advanced): real send requires both
CARLOS_MAIL_ALLOW_SEND=1atmail startand an entry in/etc/postfix/carlos-send-allowlist(which ships empty). When enabled, the allowlist is consulted ahead of the capture map, so only allowlisted recipients go tosmtp:while everything else is still captured. OptionalCARLOS_MAIL_RELAYHOSTsets an SMTP relay.mail statusreports the active mode;mail start/restartwarn loudly when send mode is on. The empty default allowlist means the env flag alone sends nothing.Helper commands
mail start|stop|restart|status|log|capture|show|clear|test— capture file location and delivery mode are surfaced bystatus.Acceptance criteria
/var/log/carlos-mail-capture.eml; no accepted mail can use normal outbound SMTP.Fromline).nobody:nogroup0660(not world-writable); port 25 stays loopback-only.Testing
postconf -nroutes mail todevcaptureby default.configure_send_modefor: flag unset (capture-only), flag set + empty allowlist (still captured, warns), flag set + entry + relayhost (allowlisted → smtp, others captured).mailand the embedded capture script passbash -n/sh -n.Privacy / PHI
The capture file holds full raw email content (possible PHI-like test data, attachments). Do not commit it; clear with
mail clear. Never add a real patient address to the send allowlist or enable send mode with real patient data — opt-in delivery is for synthetic test data and your own test inbox only.Relates to #3096 (provider-to-patient email investigation). This does not fix #3096; it makes the local email path safe to test.
🤖 Generated with Claude Code