Skip to content

docs: fix inaccuracies in CLI reference#1611

Merged
Mossaka merged 2 commits intomainfrom
copilot/fix-docs-accuracies-cli-reference
Apr 2, 2026
Merged

docs: fix inaccuracies in CLI reference#1611
Mossaka merged 2 commits intomainfrom
copilot/fix-docs-accuracies-cli-reference

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 2, 2026

Four factual errors in the CLI reference documentation.

  • --dns-servers default: Was 8.8.8.8,8.8.4.4 — actually auto-detected from host resolvers via detectHostDnsServers(), falls back to Google DNS only on failure
  • --keep-containers cleanup note: Referenced awf-copilot — correct container name is awf-agent
  • --mount defaults: Described as "host filesystem at /host (read-only)" — actually selective bind mounts (system bins ro, workspace/tmp rw, whitelisted $HOME subdirs rw, select /etc files only)
  • --allow-host-ports default: Described as "allows all ports (future versions will default to 80,443)" — default is already 80,443

Copilot AI linked an issue Apr 2, 2026 that may be closed by this pull request
Fix four documentation inaccuracies in cli-reference.md:

1. --dns-servers default: auto-detected from host, not hardcoded Google DNS
2. --keep-containers cleanup: container is awf-agent, not awf-copilot
3. --mount defaults: selective bind mounts, not blanket host FS mount
4. --allow-host-ports default: already 80,443, not "all ports"

Closes #1617

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/7a16c283-4134-46fe-9703-ed39027a5af0

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix inaccuracies in CLI reference documentation docs: fix inaccuracies in CLI reference Apr 2, 2026
Copilot AI requested a review from Mossaka April 2, 2026 18:28
@Mossaka Mossaka requested a review from Copilot April 2, 2026 18:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the CLI reference documentation to correct several factual inaccuracies around defaults and behavior for DNS configuration, container cleanup, mounting, and host port access.

Changes:

  • Corrects --dns-servers default behavior to reflect host resolver auto-detection with Google DNS fallback.
  • Fixes --keep-containers cleanup note to reference the correct container name.
  • Updates --mount “default mounts” description to reflect selective bind mounts, and corrects --allow-host-ports default behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


:::note
Requires manual cleanup: `docker stop awf-squid awf-copilot && docker network rm awf-net`
Requires manual cleanup: `docker stop awf-squid awf-agent && docker network rm awf-net`
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

The manual cleanup command stops containers but doesn’t remove them; docker network rm awf-net will fail if the (even stopped) containers are still attached to the network. Consider switching to docker rm -f awf-squid awf-agent (or docker stop ... && docker rm ...) before removing the network, consistent with other docs in the repo.

Suggested change
Requires manual cleanup: `docker stop awf-squid awf-agent && docker network rm awf-net`
Requires manual cleanup: `docker rm -f awf-squid awf-agent && docker network rm awf-net`

Copilot uses AI. Check for mistakes.
- Host filesystem at `/host` (read-only)
- User home directory (read-write)
**Default mounts (selective bind mounts, not a blanket host FS mount):**
- System binaries (`/usr`, `/bin`, `/sbin`, `/lib`, `/lib64`, `/opt`, `/sys`, `/dev`) at `/host` (read-only)
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

The wording “System binaries (...) at /host” is a bit inaccurate: these are bind-mounted under /host/<dir> (e.g. /usr/host/usr, /bin/host/bin, etc.), not mounted as a single filesystem at /host. Rephrase to “under /host” (or spell out the /host/usr, /host/bin, … paths) to avoid confusion.

Suggested change
- System binaries (`/usr`, `/bin`, `/sbin`, `/lib`, `/lib64`, `/opt`, `/sys`, `/dev`) at `/host` (read-only)
- System binaries (`/usr`, `/bin`, `/sbin`, `/lib`, `/lib64`, `/opt`, `/sys`, `/dev`) under `/host` (read-only)

Copilot uses AI. Check for mistakes.
:::tip[Best Practice]
Always explicitly specify `--allow-host-ports` to ensure consistent behavior across versions.
:::
- Without `--allow-host-ports`: Only ports 80 and 443 are allowed
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

This section implies that omitting --allow-host-ports always results in only 80/443 being allowed, but there’s an important exception: when --allow-domains includes the special localhost keyword, the CLI auto-enables host access and auto-populates a default dev port list (3000, 4200, 5173, 8080, etc.) unless the user explicitly sets --allow-host-ports. Please document that exception here (or in the preceding --enable-host-access warning) so readers don’t get surprised by the broader default when using localhost.

Suggested change
- Without `--allow-host-ports`: Only ports 80 and 443 are allowed
- Without `--allow-host-ports`: Only ports 80 and 443 are allowed in the normal case
- Exception: If `--allow-domains` includes the special `localhost` keyword and you do not set `--allow-host-ports`, host access is auto-enabled and a default set of common development ports is allowed

Copilot uses AI. Check for mistakes.
@Mossaka Mossaka marked this pull request as ready for review April 2, 2026 21:12
@Mossaka Mossaka merged commit a9b90c0 into main Apr 2, 2026
4 checks passed
@Mossaka Mossaka deleted the copilot/fix-docs-accuracies-cli-reference branch April 2, 2026 21:12
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.

docs: fix inaccuracies in CLI reference

3 participants