fix(directory): harden VMACS query construction

[rlorenzo]

What

Hardening changes to VMACSService.Search, plus a rewrite of the advisory Development-merge CI check carried on this branch.

VMACSService:

1. URL-encode the login ID. Uri.EscapeDataString(loginID ?? string.Empty) before interpolating into the query URL, so reserved characters can't alter the request. Also drops a no-op .ToString().
2. Source the AUTH token from config. Read from the Credentials:VmacsAuthToken config section (AWS SSM Parameter Store) instead of a hardcoded constant in source, matching how DB and other secrets are sourced.
3. Extract a testable query builder. Request-path construction moved into BuildSearchPath, covered by unit tests for the null, space, +, &, and = encoding cases.

Why

Both hardening items surfaced during the CodeQL/agy review work. loginID is a DB-sourced campus login (AaudUser.LoginId, passed by both DirectoryController call sites), so exploitability is low. Moving the AUTH token out of source removes a hardcoded credential from version control and brings it in line with the rest of the app's secret handling.

Deploy note

Requires the Credentials:VmacsAuthToken parameter to exist in AWS SSM Parameter Store (under /Shared/Credentials/) before this deploys. Until it does, VMACS phone/pager/unit enrichment in the Directory search returns empty (the endpoint still returns results). Verified on dev: a Directory search now shows the VMACS-sourced "M: ... (VMACS)" line, confirming the token loads from SSM and the lookup succeeds.

CI: Development merge check

Unrelated to VMACS, but carried on this branch: a rewrite of the advisory Development merge check workflow so it actually works for our flow.

Why it was needed: we merge into Development with a direct git merge + push (npm run merge), not through a PR, so the old "find a merged PR into Development" detection never matched and the check was meaningless.

Now: a single check per PR, green when the head commit is in Development (detected by ancestry, with a merged-PR squash fallback), red when it isn't. It re-evaluates on every PR commit, and because merging into Development adds no PR commit, a push-on-Development run re-runs any still-red PR so its check flips green once the branch lands there. Advisory only; it never blocks merging.

Scope note

The agy review also suggested reading the response stream directly instead of string→bytes→MemoryStream, and a full static→DI refactor of VMACSService. Those remain out of scope here; recommend a separate ticket for the static→DI refactor.

[codecov-commenter]

Bundle Report

Bundle size has no change ✅

[codecov-commenter]

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.50%. Comparing base (4564dae) to head (fa5b4a6).

Files with missing lines	Patch %	Lines
web/Areas/Directory/Services/VMACSService.cs	57.14%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #226   +/-   ##
=======================================
  Coverage   44.49%   44.50%           
=======================================
  Files         895      895           
  Lines       51778    51783    +5     
  Branches     4834     4836    +2     
=======================================
+ Hits        23041    23045    +4     
- Misses      28165    28166    +1     
  Partials      572      572           

Flag	Coverage Δ
backend	44.65% <57.14%> (+<0.01%)	⬆️
frontend	41.32% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[Copilot]

Pull request overview

This PR hardens the Directory area’s VMACS lookup by URL-encoding the loginID value before it’s interpolated into the outbound query string, preventing reserved characters from altering the request.

Changes:

• Encode loginID via Uri.EscapeDataString(loginID ?? string.Empty) before building the VMACS query URL.
• Remove a no-op .ToString() call on an already-string interpolated value.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[rlorenzo]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

📝 Walkthrough

Walkthrough

VMACSService now reads the VMACS AUTH token from configuration, URL-encodes loginID, and builds the trust query through a helper. New tests cover the encoded find= value and the AUTH= parameter.

Changes

VMACS trust query update

Layer / File(s)	Summary
Encode query and use token constant web/Areas/Directory/Services/VMACSService.cs	Adds a configuration-backed VmacsAuthToken, changes Search to call BuildSearchPath(loginID, VmacsAuthToken), and builds the query with an escaped loginID plus the passed auth token.
Validate query string helper test/Areas/Directory/VMACSServiceTest.cs	Adds tests for BuildSearchPath covering URL-encoded loginID inputs and inclusion of the provided AUTH token.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• bsedwards

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: hardening VMACS query construction in the directory service.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/vmacs-url-encode-loginid

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[rlorenzo]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@web/Areas/Directory/Services/VMACSService.cs`:
- Around line 27-28: The URL construction in VMACSService is still embedded in
the lookup logic, so pull the request-building portion out into a small helper
method (for example, in the same service near the login lookup path) that takes
the login ID and returns the encoded query string. Then add focused tests
against that helper to cover null, spaces, +, &, and = cases so the
Uri.EscapeDataString behavior and request format in the lookup path are
independently verifiable.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bd9770b9-c6b2-4fcf-885c-070c57819dc7

📥 Commits

Reviewing files that changed from the base of the PR and between 1a0996e and 81e0601.

📒 Files selected for processing (1)

• web/Areas/Directory/Services/VMACSService.cs

[rlorenzo]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@web/Areas/Directory/Services/VMACSService.cs`:
- Around line 56-60: BuildSearchPath currently escapes loginID but interpolates
authToken raw into the query string. Update the VMACSService.BuildSearchPath
method to URL-encode authToken the same way as encodedLoginId before composing
the AUTH parameter, so the generated search URL stays valid even if the token
contains reserved characters.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3ca30a2c-857d-46bb-ac12-ad21b0181103

📥 Commits

Reviewing files that changed from the base of the PR and between 81e0601 and 19f2941.

📒 Files selected for processing (2)

• test/Areas/Directory/VMACSServiceTest.cs
• web/Areas/Directory/Services/VMACSService.cs

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

[rlorenzo]

@bsedwards This removes the hardcoded auth token from the VMACS API and also fixes the GitHub action to check whether a branch has been merged into Development. The check must be green/red and will block branches if they are not merged into Development first.