Fix: do not decrement check-in countdown for nonexistent app names

[philphauler]

Summary

Fix a security issue where a nonexistent application name in the AMT (Application
Monitor Table) triggers the configured failure action, including processor reset.

Problem

HS_MonitorSingleApplication calls CFE_ES_GetAppIDByName for each active AMT
entry. When the name lookup fails, the code still falls through to decrement
CheckInCountdown. Once the countdown reaches zero, the configured action executes
-- including PROC_RESET.

A crafted AMT entry with AppName = "NONEXISTENT", CycleCount = 1, and
ActionType = PROC_RESET deterministically triggers a processor reset on the
first monitoring cycle after table activation.

Reported in #162.

Fix

Return early from HS_MonitorSingleApplication when CFE_ES_GetAppIDByName
fails. The error/debug events are still emitted (existing behavior preserved),
but the countdown is not decremented. Only apps that exist and have genuinely
stopped executing will trigger the failure action.

Testing

The fix is a 6-line change in a single function. The early return preserves
all existing event reporting behavior -- only the countdown decrement path
is guarded.

[philphauler]

One-file fix, 8 lines. The early return prevents a nonexistent app name from decrementing the check-in counter to zero and triggering a processor reset. Happy to adjust if the guard should live elsewhere.

[dzbaker]

CCB: 27 August 2026 - Team suspect this works as designed.

[dzbaker]

@philphauler Thank you for your contribution. Please resolve the workflow failures.

[ddstewar]

@philphauler Is this supposed to fully resolve #162, or is this only addressing one part of the reported issue?

[ddstewar]

@dzbaker
Returning to unassigned. Originating issue has been added to the security issue tracker and linked to this PR.

[philphauler]

Good catch, this PR fully prevents the reported PROC_RESET via HS_MonitorSingleApplication early return when GetAppIDByName fails in hs_monitors.c. That makes a nonexistent AppName inert even if the table still validates as good = 1, which satisfies the expected behavior OR clause of rejected during validation OR ignored at runtime.

The other half is HS_ValidateAMTable in hs_monitors.c where a bad AppName still counts as good. Adding a strict check there would make the load fail as bad = 1, which is the most secure early catch but would also reject deferred monitors for apps not yet started and require the app to exist at load time.

Keeping validation permissive and relying on the runtime guard is the universal clean that works for both forever bogus names like ZZZ_TRIG_APP and deferred monitors for future apps. Happy to keep this PR as the 1 file runtime fix that fully stops the reset, or add the strict validation in this same PR if you prefer the table to be rejected at load, let me know which you prefer.

[ddstewar]

Your fix is fine as is. And I do think we want to keep the table permissive. I was just curious what your take on it was.

Again, please fix the workflow issues so they do not fail and this can be approved. This may require updating the unit tests for the app to exercise the new branch of code you introduced so that we get full coverage.

[philphauler]

I have some update for it too, and yeah, will focus on that wf issue

[ddstewar]

@philphauler
First, I want to sincerely apologize. This PR should not have been progressed, and no work should have been requested of you on it. That was our error, and I'm sorry for the time you spent responding to review feedback.

Here's what happened, so it's fully transparent:

After review by a subject matter expert, the behavior this PR set out to change was determined to be working as intended. I was not present for that assessment. The Health & Safety Application Monitor is required to take the table-defined action when a monitored application is not executing, per the HS 2000-series functional requirements (HS 2000, HS 2000.2, HS 2000.2.1–.2.3, in docs/hs_FunctionalRequirements.csv). A table entry configured with a Processor Reset action will, by design, trigger a Processor Reset when the referenced application is not running.

Unfortunately, our earlier comments on this PR did not make that clear, and I recognize they were misleading, as I mis-interpreted them as well:

• The CCB comment of "CCB: 27 August 2026 - Team suspect this works as designed" was referring to the existing code working as designed — not to the proposed fix being correct or accepted. That distinction was not stated plainly, and I understand how it read otherwise, I took it as this fix was working as intended myself.
• The follow-up comment, "Thank you for your contribution. Please resolve the workflow failures," was made in haste and further muddied the situation. It implied, to me, the change was on a path to acceptance and asked you to invest more effort, when in fact the underlying behavior had been determined to be intended. That was a mistake on my part.

To be clear: the confusion here was ours, not yours. Your report and PR were well-constructed, and you responded to our feedback in good faith. We are actively standing up our vulnerability-response process, and this case exposed a gap in how findings and community PRs move through review and communication. We're using this as a direct input to close that gap so contributors get clear, consistent responses.

For the reasons above, we're going to close this PR as working as intended. The associated requirements are being refined separately under #154, and any future changes to this behavior would be driven through that effort.

Thank you again for your contribution and for your patience. We genuinely value your engagement with the project, and I apologize again for the mixed signals on our end.

[philphauler]

All g! Totally fine, I'm still learning my ways around as well and wanting to help. Any such feedback gets us further 🚀✨

[philphauler]

Thank you for the thorough explanation, @ddstewar. No apology needed — the CCB comment was genuinely ambiguous and I appreciate you taking the time to lay out what happened. Glad this is feeding into the vulnerability-response process refinement. I'll follow #154 for any future direction on this behavior.