Skip to content

feat(ports): wire portsAttributes into forwarding decisions#248

Merged
skevetter merged 6 commits into
mainfrom
4f43-103f-ws-ports-attributes
May 6, 2026
Merged

feat(ports): wire portsAttributes into forwarding decisions#248
skevetter merged 6 commits into
mainfrom
4f43-103f-ws-ports-attributes

Conversation

@skevetter

Copy link
Copy Markdown
Contributor

Summary

Wires all 5 devcontainer portsAttributes fields into runtime behavior per the devcontainer spec:

  • onAutoForward: ignore skips forwarding entirely; silent/empty logs at debug level; notify/openBrowser logs at info level
  • label: surfaced in forwarder log output for port identification
  • protocol: included in forwarder log metadata (http/https)
  • requireLocalPort: enforced on the host-side forwarder — if the specified local port is unavailable, forwarding is skipped rather than falling back to a random port
  • elevateIfNeeded: logs a warning when forwarding privileged ports (<1024) on Linux, advising the user to run with elevated permissions

The requireLocalPort check was intentionally removed from the container-side watcher (pkg/netstat/watcher.go) because it cannot meaningfully test host port availability from inside the container. Enforcement happens exclusively in the host-side forwarder (pkg/tunnel/forwarder.go).

Files changed:

  • cmd/agent/container/credentials_server.go — propagate all 5 fields through the container-side port resolver
  • pkg/netstat/watcher.go — pass attributes to forwarding decisions, remove broken container-side requireLocalPort check
  • pkg/netstat/watcher_test.go — add tests for attribute propagation
  • pkg/tunnel/forwarder.go — enforce requireLocalPort, elevateIfNeeded, onAutoForward, label, and protocol at the forwarding layer

skevetter added 2 commits May 6, 2026 10:02
Add RequireLocalPort and ElevateIfNeeded to PortForwardAttribute struct
and propagate them through the container-side resolver. Implement
requireLocalPort check in the watcher (skips forwarding when the local
port is unavailable), elevateIfNeeded warning in the host-side forwarder
for privileged ports, differentiated log levels for onAutoForward
(silent=debug, notify/openBrowser=info), and protocol metadata in
forwarder log output.
The requireLocalPort check in watcher.go called IsAvailable inside the
container where the port is already in LISTEN state, making it always
fail. The correct enforcement point is the host-side forwarder which
already implements this correctly. Also removes the tautological test
that never exercised the actual watcher logic.
@netlify

netlify Bot commented May 6, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit a85d279
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/69fb5b86e429210009b01aa0

@github-actions github-actions Bot added the size/m label May 6, 2026
@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@skevetter has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 15 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7fb830f2-e39f-448a-ab0b-78d9a10e6e42

📥 Commits

Reviewing files that changed from the base of the PR and between 7cd689f and a85d279.

📒 Files selected for processing (9)
  • cmd/agent/container/credentials_server.go
  • cmd/runusercommands.go
  • cmd/up/up.go
  • cmd/up/up_client.go
  • e2e/tests/ssh/ports_attributes_test.go
  • e2e/tests/ssh/testdata/ports-attributes/.devcontainer.json
  • pkg/netstat/watcher.go
  • pkg/netstat/watcher_test.go
  • pkg/tunnel/forwarder.go

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Verifies that when requireLocalPort=true and the host port is already
occupied, port forwarding is skipped rather than falling back to another
port.
@github-actions github-actions Bot added size/l and removed size/m labels May 6, 2026
@skevetter
skevetter enabled auto-merge (squash) May 6, 2026 15:43
@skevetter
skevetter merged commit 90dee39 into main May 6, 2026
54 checks passed
@skevetter
skevetter deleted the 4f43-103f-ws-ports-attributes branch May 6, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant