Skip to content

Add enterprise network and perimeter security controls - #277

Merged
TFT444 merged 4 commits into
openshield-org:devfrom
ritiksah141:feat/private-link-controls-253
Aug 29, 2026
Merged

Add enterprise network and perimeter security controls#277
TFT444 merged 4 commits into
openshield-org:devfrom
ritiksah141:feat/private-link-controls-253

Conversation

@ritiksah141

@ritiksah141 ritiksah141 commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements the complete enterprise network and perimeter security backlog from issue #253 with ten evidence-rich Azure controls covering Private Link, critical PaaS exposure, Azure Firewall threat intelligence, Application Gateway WAF enforcement, diagnostics, managed rules, bot protection, and rate limiting.

What changed

  • Added AZ-NET-018 to detect supported Private Endpoint targets that retain public network access.
  • Added AZ-NET-019 for pending, rejected, or disconnected Private Endpoint connections.
  • Added AZ-NET-020 for approved Private Endpoints without a Private DNS zone group.
  • Added AZ-NET-021 for Private Endpoint FQDN evidence that resolves only to public addresses.
  • Added AZ-NET-022 for public Storage, Key Vault, Azure SQL, PostgreSQL, and App Service resources without an exact approved exception.
  • Added AZ-NET-023 to require Azure Firewall threat intelligence AlertAndDeny enforcement.
  • Added AZ-NET-024 to require Application Gateway WAF Prevention mode.
  • Added AZ-NET-025 to require Application Gateway access, performance, and firewall diagnostic logs.
  • Added AZ-NET-026 to require a current OWASP 3.2 or DRS 2.1+ base rule set plus Bot Manager 1.0+.
  • Added AZ-NET-027 to require an enabled WAF RateLimitRule on public Application Gateways.
  • Added service-specific, cached Azure collectors that preserve each API/permission failure as indeterminate instead of collapsing it into an empty inventory.
  • Added evidence source, UTC collection timestamp, observed and expected values, required permissions, remediation, and confidence to every finding.
  • Added safe operator-reviewed remediation playbooks for every new rule.
  • Added complete CIS, NIST CSF, ISO 27001, and SOC 2 mappings with explicit reviewed N/A-* CIS decisions instead of TBD-* placeholders.
  • Updated the Network Layer assurance audit, rule reference, documentation, and changelog for all 75 scanner rules.

False-finding safeguards

  • Empty service inventory is treated as not applicable.
  • Collector and permission failures are indeterminate and never create findings.
  • Malformed or missing endpoint, DNS, WAF mode, managed-rule, or public-access evidence is skipped as unknown.
  • Critical PaaS collectors preserve failure independently per service, so one denied API cannot create false findings or suppress valid results from another service.
  • Approved public-access exceptions require an exact case-insensitive Azure resource ID in OPENSHIELD_PUBLIC_PAAS_EXCEPTIONS; partial matches are not accepted.
  • Rate limiting is limited to public Application Gateways because the current repository has no authoritative Front Door, API Management, or third-party edge inventory. Unsupported services remain unknown rather than being inferred.

Verification

  • Focused Private Link, perimeter, assurance, engine, and mapping suite: 70 passed.
  • Full runnable backend suite: 598 passed, 2 skipped, 1 deselected.
  • The deselected vector-store purity test is unrelated to this change and cannot initialize ONNX CoreML in the managed sandbox because it cannot create its compilation working directory.
  • Ruff check passes across the repository.
  • Bandit reports no high-severity issues.
  • Python compilation passes.
  • Compliance and assurance JSON validation passes.
  • All ten remediation playbooks pass shell syntax validation.
  • Git diff whitespace validation passes.
  • DCO verification passes for the commit.

Type of change

  • New scan rules
  • Azure service collectors
  • Remediation playbooks
  • Documentation
  • Compliance mappings
  • Network assurance classification
  • Regression and failure-path tests

Testing

  • Tested against a real Azure subscription
  • Positive and negative evaluation paths covered
  • Empty inventory and unsupported state covered
  • Malformed evidence covered
  • API and permission failure paths covered
  • Local CI-equivalent backend checks pass
  • No hardcoded credentials or secrets

Checklist

  • All ten issue Implement remaining enterprise network and perimeter security rules #253 controls have collector, evaluation, evidence, remediation, mappings, and tests
  • API failures never become false PASS or FAIL results
  • Every commit includes a DCO Signed-off-by trailer
  • Rule discovery and Network Layer assurance include AZ-NET-018 through AZ-NET-027
  • No unresolved TBD-* framework mappings were introduced
  • Branch was created from the latest upstream dev
  • Branch name follows the project convention

Related issue

Closes #253

Files to review

  1. scanner/azure_client.py - failure-aware Private Link, PaaS, Application Gateway, WAF policy, and diagnostic collectors.
  2. scanner/rules/az_net_018.py through scanner/rules/az_net_027.py - the ten enterprise perimeter controls.
  3. tests/test_private_link_rules.py and tests/test_enterprise_perimeter_rules.py - positive, negative, empty, malformed, exception, and failure coverage.
  4. playbooks/cli/fix_az_net_018.sh through playbooks/cli/fix_az_net_027.sh - review-gated remediation guidance.
  5. compliance/frameworks/*.json and compliance/assurance/network_layer.json - verified mappings and OSI classifications.
  6. docs/private-link-controls.md, docs/rules-reference.md, and CHANGELOG.md - behavior, limitations, permissions, and release documentation.

@ritiksah141
ritiksah141 marked this pull request as ready for review August 16, 2026 23:31
@ritiksah141
ritiksah141 requested review from m-khan-97 and removed request for SHAURYAKSHARMA24 August 16, 2026 23:31
@ritiksah141 ritiksah141 self-assigned this Aug 16, 2026
TFT444
TFT444 previously approved these changes Aug 17, 2026

@TFT444 TFT444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

good to go from myside.

@ritiksah141

Copy link
Copy Markdown
Collaborator Author

Updated the branch with the latest upstream dev changes and resolved the compliance mapping and rules reference conflicts.

Verification after installing the merged requirements:

  • 770 tests passed and 2 skipped in the full sandboxed suite
  • The remaining ONNX vector-store purity test passed outside the filesystem sandbox
  • Merge commit: ff09713

The branch is now up to date with dev and all tests pass.

@ritiksah141
ritiksah141 requested a review from TFT444 August 20, 2026 17:07
TFT444
TFT444 previously approved these changes Aug 20, 2026

@TFT444 TFT444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

good to go

@m-khan-97 m-khan-97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the substantial work on the network and perimeter rules. I checked the collectors against the Azure behavior they are meant to represent. Two issues need to be corrected before we merge:

  1. AZ-NET-021 is not testing effective DNS resolution. custom_dns_configs contains the FQDN and private IP records Azure expects to be configured for the endpoint. It does not tell us what a workload in the VNet, or an on-premises resolver, actually resolves. As written, the rule can see a private endpoint IP even when the zone is unlinked, an A record is missing or stale, or forwarding is broken. Please add an explicit opt-in resolution probe from the relevant network context and compare the result with the endpoint IP. If that is outside the intended scope, rename and narrow the rule so it only claims to validate configuration evidence. Please include a regression case for broken effective DNS with a valid private customDnsConfig.

  2. AZ-NET-025 requires a log category that WAF_v2 cannot provide. get_waf_diagnostic_logging() always requires ApplicationGatewayPerformanceLog, but Microsoft supports that category only on Application Gateway v1. For v2, performance data comes from metrics. This would flag a correctly configured WAF_v2 gateway as noncompliant forever. Please make the required categories SKU-aware and add coverage for both v1 and v2.

GitHub CI is green. My focused local test run could not collect because this checkout is missing azure.mgmt.authorization; that is an environment limitation, not the reason for this review. These two findings come from comparing the production logic with the Azure contracts.

Please tag me when the changes are ready and I will review them again.

@ritiksah141

Copy link
Copy Markdown
Collaborator Author

@m-khan-97, addressed both requested changes in faceb53 and merged the latest dev in d89963e.

  • AZ-NET-021 is now explicitly limited to ARM customDnsConfigs evidence. The rule name, description, remediation, finding metadata, assurance catalog, compliance mappings, and documentation no longer claim effective DNS resolution. Added regression coverage showing that valid private configuration does not prove the state of a separate resolver path.
  • AZ-NET-025 is now SKU-aware. Application Gateway v1 requires access, performance, and firewall logs. WAF_v2 requires access and firewall logs because v2 performance telemetry is provided through Azure Monitor metrics. Added separate regression coverage for both SKU generations.

Verification after merging the current dev:

  • 775 tests passed, 2 skipped
  • Ruff check passed
  • Ruff format check passed
  • Compliance JSON and diff checks passed

The branch is current with dev and ready for another review.

TFT444
TFT444 previously approved these changes Aug 24, 2026

@TFT444 TFT444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the fixes for AZ-NET-021 and AZ-NET-022. Scoping 021 to ARM configuration evidence only is the right call, and the dedicated test confirms it makes no resolver-path claims. AZ-NET-022 correctly treats collection failures and unknown states as indeterminate. Approving once CI passes.

@parthrohit22 parthrohit22 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Deep-reviewed AZ-NET-018 through AZ-NET-027 plus the new azure_client.py collectors. Most of this is solid - the indeterminate-vs-compliant-vs-noncompliant discipline across 018-022 is exactly right and consistent with the codebase's established convention (az_net_016.py / get_all_azure_firewalls: API failure or missing evidence skips rather than flags), per-service failure isolation in get_critical_paas_inventory is a genuine robustness improvement, and evidence metadata via the shared _perimeter_common/_private_link_common helpers is consistently attached. Test coverage for 018-022 and 024-026 targets empty-inventory/API-failure/malformed-evidence paths, not just the happy path. RULE_IDs don't collide with anything existing, playbooks pass bash -n, framework mappings are complete across all four compliance JSONs.

Two real bugs though, one of them severe enough to block on its own. Left inline comments on both.

Also worth a look before merge, not blocking: playbooks/cli/fix_az_net_018.sh, _020.sh, _022.sh, _026.sh, _027.sh are echo-only guidance with no executable remediation, unlike the established convention elsewhere (fix_az_cmp_001.sh, fix_az_net_016.sh) which run a confirmed az CLI command. If that's deliberate for higher-risk changes it's a reasonable call, but it's worth stating explicitly somewhere (PR description or a code comment) so it doesn't read as an oversight - especially now that the one playbook in this batch that does execute a command (023) turned out to have an invalid one.

Full suite: 755 passed, 3 skipped, 2 failed - the 2 failures are test_observability.py hitting an unreachable postgres host in this environment, unrelated to this diff (confirmed that file isn't touched here).

Comment thread scanner/rules/az_net_023.py Outdated
)
continue
normalized = str(getattr(mode, "value", mode)).replace("_", "").lower()
if normalized == "alertanddeny":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This rule can never pass against a real subscription. I checked the actual Azure SDK enum (azure.mgmt.network.models.AzureFirewallThreatIntelMode) and it has exactly three values: Alert, Deny, Off. There is no AlertAndDeny mode anywhere in the Azure Firewall API. I ran this scan() against all three real enum values directly and every one is flagged HIGH - including Deny, which is the strictest, most secure setting a firewall can actually have. There is no configuration that satisfies this check today.

The test (test_net_023_requires_alert_and_deny) doesn't catch this because it feeds in a fabricated threat_intel_mode="AlertAndDeny" as the compliant case instead of testing the real Deny value - so the test and the rule share the same wrong assumption rather than one catching the other.

The playbook compounds this: fix_az_net_023.sh runs az network firewall update ... --threat-intel-mode AlertAndDeny, which isn't a valid value for that CLI flag either, so running the suggested remediation against a real subscription fails outright.

Suggest: compliant should be mode == "Deny" (optionally treat Alert as a lesser MEDIUM rather than folding both into one broken HIGH check), and the playbook needs --threat-intel-mode Deny.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 70f4be6. AZ-NET-023 now treats the real Azure Firewall Deny enum value as compliant, flags Alert and Off, reports Deny as the expected value, and uses --threat-intel-mode Deny in the remediation playbook. Regression coverage now uses the actual SDK enum values.

Comment thread scanner/rules/az_net_027.py Outdated
@ritiksah141

Copy link
Copy Markdown
Collaborator Author

@parthrohit22, both blocking findings are addressed in 70f4be6, and the latest dev is merged in 040284f.

  • AZ-NET-023 now uses Azure Firewall threat intelligence mode Deny as the compliant state. Alert and Off are covered as noncompliant real enum values, and the remediation command now passes --threat-intel-mode Deny.
  • AZ-NET-027 now evaluates rate limiting only when the Application Gateway is public and WAF is enabled. A public gateway without WAF is covered by a regression test and is no longer reported as missing a WAF rate-limit rule.
  • I replied to both inline threads with the corresponding fix details.

Verification after merging current dev:

  • 778 backend tests passed, 2 skipped
  • 24 focused perimeter tests passed
  • Ruff check and format check passed
  • Frontend lint, page-data tests, accessibility checks, i18n checks, and production build passed
  • Playbook shell syntax passed

Could you please take another look?

@parthrohit22 parthrohit22 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Both blockers from my last review are fixed in 70f4be6, confirmed against the actual diff rather than taking the reply on faith.

AZ-NET-023: re-checked the real Azure SDK enum (AzureFirewallThreatIntelMode) - it now correctly checks normalized == "deny" instead of the fabricated "alertanddeny", which never existed. The playbook now runs --threat-intel-mode Deny, a valid CLI value, instead of the invalid AlertAndDeny it had before. The old test that fed in the fabricated value is gone; test_net_023_flags_real_non_deny_modes is parametrized on the two real non-compliant values (Alert, Off) and test_net_023_accepts_real_deny_mode_and_preserves_unknown covers the real compliant case plus missing/unavailable evidence.

AZ-NET-027: now gates on public_gateway(gateway) and waf_enabled(gateway), matching 024-026. test_net_027_ignores_public_gateway_without_waf covers exactly the case I flagged as untested - a public gateway with no WAF policy at all - and asserts zero findings instead of the old misleading "missing rate-limit rule in a nonexistent policy" result.

Pulled the PR head into a clean clone and ran it myself rather than trusting the reported numbers: tests/test_enterprise_perimeter_rules.py - 24/24 pass, including both new regression tests.

Approving.

parthrohit22
parthrohit22 previously approved these changes Aug 24, 2026

@parthrohit22 parthrohit22 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Both blockers from my last review are fixed in 70f4be6, confirmed against the actual diff rather than taking the reply on faith.

AZ-NET-023: re-checked the real Azure SDK enum (AzureFirewallThreatIntelMode) - it now correctly checks normalized == "deny" instead of the fabricated "alertanddeny", which never existed. The playbook now runs --threat-intel-mode Deny, a valid CLI value, instead of the invalid AlertAndDeny it had before. The old test that fed in the fabricated value is gone; test_net_023_flags_real_non_deny_modes is parametrized on the two real non-compliant values (Alert, Off) and test_net_023_accepts_real_deny_mode_and_preserves_unknown covers the real compliant case plus missing/unavailable evidence.

AZ-NET-027: now gates on public_gateway(gateway) and waf_enabled(gateway), matching 024-026. test_net_027_ignores_public_gateway_without_waf covers exactly the case I flagged as untested - a public gateway with no WAF policy at all - and asserts zero findings instead of the old misleading "missing rate-limit rule in a nonexistent policy" result.

Pulled the PR head into a clean clone and ran it myself rather than trusting the reported numbers: tests/test_enterprise_perimeter_rules.py - 24/24 pass, including both new regression tests.

Approving. nice work @ritiksah141

@parthrohit22

Copy link
Copy Markdown
Member

@TFT444 Have a look , re-requesting review from your side, I flagged it green , if you have same opinion, it's mergeable now.

@parthrohit22
parthrohit22 requested a review from TFT444 August 24, 2026 15:38
@ritiksah141

Copy link
Copy Markdown
Collaborator Author

@m-khan-97 , could be please review it as I have addressed you request.

@m-khan-97 m-khan-97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, Ritik. I verified the latest commit directly. The four previously reported fixes are correctly implemented, the focused suite passes 47/47 locally, and the authoritative CI run on 040284f is green. The cancelled jobs shown by GitHub belong to a duplicate run and are not a source failure.\n\nI found two remaining evidence-state gaps before I can approve:\n\n1. AZ-NET-023 treats every non-Deny value as a confirmed HIGH. Alert and Off are positive noncompliant evidence, but an unrecognized/future SDK value is not. Please flag only the known unsafe enum values and preserve any unexpected value as UNKNOWN. Add a regression case such as FutureMode that produces no finding.\n\n2. AZ-NET-027 uses getattr(rule, "state", "Enabled"), so a malformed or partially collected RateLimitRule with no state is silently assumed enabled and can make the gateway appear compliant. Please require explicit Enabled evidence. If a rate-limit rule exists but its state is unavailable and there is no explicitly enabled rule, preserve the result as UNKNOWN rather than inferring either compliance or a confirmed violation. Add missing-state and unknown-state regression cases.\n\nThese are small, shared trust-contract corrections rather than a redesign. Once they are covered and CI is green, tag me and I will re-review promptly.

@ritiksah141

ritiksah141 commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

@m-khan-97, addressed both evidence-state gaps in 4de1df8.

  • AZ-NET-023 now reports findings only for the known unsafe Azure values Alert and Off. Deny remains compliant, while missing or unrecognized values such as FutureMode are preserved as UNKNOWN with no finding.
  • AZ-NET-027 now requires explicit Enabled state evidence for compliance. An explicitly Disabled rule remains a confirmed violation. If a rate-limit rule exists but its state is missing or unrecognized, and no explicitly enabled rule exists, the result is preserved as UNKNOWN.
  • Added regression coverage for FutureMode, missing rate-limit state, unknown rate-limit state, and explicitly disabled state.

Verification on the current dev base:

  • 780 backend tests passed, 2 skipped
  • 26 focused perimeter tests passed
  • Ruff check and format check passed
  • Frontend lint, page-data tests, accessibility checks, i18n checks, and production build passed
  • DCO sign-off passed locally

Could you please review again as CI is green as well?

@ritiksah141
ritiksah141 requested a review from m-khan-97 August 24, 2026 16:23
@ritiksah141

Copy link
Copy Markdown
Collaborator Author

@m-khan-97, i have addressed the changes requested in the last review so could you please re-review it and see if its fine now cause with out your approval it cant not be merged.
@TFT444, @parthrohit22 could you two also please re review it and check if its mergable or not. Thank you

@ritiksah141
ritiksah141 force-pushed the feat/private-link-controls-253 branch from 4de1df8 to d8762bc Compare August 26, 2026 12:26
@ritiksah141

Copy link
Copy Markdown
Collaborator Author

@m-khan-97 @parthrohit22 @TFT444, coordination update for #277:

  • Rebased directly onto current dev at 5001d4a, which contains fix(platform): enforce container runtime gate #309.
  • Rebased PR head is d8762bc with the four substantive commits replayed directly on dev.
  • All commits retain ritiksah141 <ritiksah141@gmail.com> as author and committer with DCO sign-off.
  • Local verification passed: 787 backend tests passed, 2 skipped; Ruff passed; frontend lint, tests, accessibility, i18n, and build passed; all ten remediation playbooks passed shell syntax validation.
  • Fresh PR CI was rerun. Backend, frontend, rule validation, CodeQL, Bandit, Semgrep, Trivy, Gitleaks, SBOM, DCO, dependency review, and external Semgrep all passed.

The only failing check is SCA (pip-audit), which now reports CVE-2026-45830 and CVE-2026-45833 against the chromadb==0.4.24 dependency inherited from dev. GitHub currently lists no patched ChromaDB version for either advisory. CI Summary is red only because it depends on that audit job.

I have not suppressed these new HIGH and CRITICAL advisories inside #277 because that is a project-wide AI dependency decision and overlaps the existing Docker/AI dependency blocker. The #277 rebase and PR-specific validation are complete.

@TFT444 TFT444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Checked m-khan-97's two remaining items against the current HEAD.

AZ-NET-023: Already guarded. After normalizing the enum, the code does if normalized not in {"alert", "off"}: ... continue, so any unrecognized or future value logs a warning and skips without creating a finding. test_net_023_accepts_real_deny_mode_and_preserves_unknown feeds in "FutureMode" and asserts == [] — the regression case m-khan-97 asked for is present.

AZ-NET-027: No silent Enabled default. The code uses getattr(rule, "state", None), filters out None before normalizing, then checks len(normalized_states) != len(states) to detect missing states and route them to UNKNOWN. test_net_027_preserves_unknown_rate_limit_state is parametrized on both None and "FutureState" and asserts zero findings in each case.

Both fixes were in the code when m-khan-97 reviewed 040284f. CI is green, DCO signed, parthrohit22 approved. Approved.

@TFT444

TFT444 commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

@ritiksah141 ci failing have a look please after its good to go

@ritiksah141
ritiksah141 force-pushed the feat/private-link-controls-253 branch from d8762bc to 566e05a Compare August 28, 2026 16:18
@ritiksah141

Copy link
Copy Markdown
Collaborator Author

@m-khan-97 @Vishnu2707, PR #277 has now been rebased onto the latest dev at efc93d7, which includes the merged changes from #317 and #307.

Verification completed on the rebased head 566e05a:

  • All four PR commits retain ritiksah141 <ritiksah141@gmail.com> as author and committer with DCO sign-off
  • Production-equivalent local test run passed: 824 passed, 2 skipped
  • Ruff lint and formatting checks passed
  • Frontend lint, tests, accessibility, i18n, and production build passed
  • All playbook shell syntax checks passed
  • Fresh GitHub CI is fully green, including CI Summary, pip-audit, Trivy, CodeQL, Semgrep OSS, and Semgrep Cloud Platform

The requested evidence-state fixes remain present after the rebase. Please take another look when available. This should now be ready for final approval and merge.

@ritiksah141

Copy link
Copy Markdown
Collaborator Author

@m-khan-97, @TFT444 and @parthrohit22, could you please re approve it as all has been addressed and then re ran the CI and I used --force-with-lease, not a plain force push. It verifies that the remote branch has not changed unexpectedly since it was fetched, which helps avoid overwriting someone else’s newer work. So, nothing concering with the force push if you guys feel something up please lemme know

@parthrohit22

parthrohit22 commented Aug 28, 2026

Copy link
Copy Markdown
Member

@m-khan-97 Wanted to close the loop on your last review before pinging you again for a re-look.

Pulled the current head (566e05a) into a clean checkout and went through both items from your last CHANGES_REQUESTED directly against the live code, not just the reply text:

  • AZ-NET-023: confirmed deny is compliant, alert/off are the only values that produce a HIGH finding, and anything else (missing or an unrecognized future SDK value) is logged and skipped with no finding — matches what you asked for. test_net_023_accepts_real_deny_mode_and_preserves_unknown exercises None and a fabricated "FutureMode" value and asserts no finding in either case.
  • AZ-NET-027: confirmed there's no silent default to Enabled anymore — getattr(rule, "state", None) with None filtered out before normalizing, so a rate-limit rule with a missing or unrecognized state is preserved as UNKNOWN rather than being read as compliant or a confirmed violation. test_net_027_preserves_unknown_rate_limit_state is parametrized on both None and "FutureState" and asserts zero findings for each.

Re-ran the specific regression suite myself (test_enterprise_perimeter_rules.py + test_private_link_rules.py): 49/49 pass. Also re-ran the full backend suite (796 passed, 3 skipped, no regressions) and ruff check . clean. All 20 GitHub Actions checks are green on the current head. TFT444 has independently re-approved on this same head after checking both items against the live code as well.

Both blockers you raised look correctly closed out from where I'm sitting; if you get a chance to take another look and either re-approve or let us know if something's still off, that'd get this moving toward merge. Formally re-requesting your review as well so it shows up properly in your queue.

@m-khan-97

Copy link
Copy Markdown
Collaborator

@ritiksah141, thanks — the requested evidence-state fixes are preserved, Parth has independently verified them, and the current checks are green. Since #308 merged after this head was prepared, the branch is now one commit behind dev. Please do one final rebase onto current dev and rerun CI. Once that is green, I will complete the final review.

Signed-off-by: ritiksah141 <ritiksah141@gmail.com>
Signed-off-by: ritiksah141 <ritiksah141@gmail.com>
Signed-off-by: ritiksah141 <ritiksah141@gmail.com>
Signed-off-by: ritiksah141 <ritiksah141@gmail.com>
@ritiksah141
ritiksah141 force-pushed the feat/private-link-controls-253 branch from 566e05a to 6f127d4 Compare August 28, 2026 21:19
@ritiksah141

Copy link
Copy Markdown
Collaborator Author

@m-khan-97, the final rebase is complete. PR #277 is now based directly on current dev at c54d591, including #308, with rebased head 6f127d4.

The requested evidence-state fixes are preserved. All four PR commits retain ritiksah141 <ritiksah141@gmail.com> as author and committer with DCO sign-off.

Verification after the rebase:

  • Focused perimeter regression suite: 26 passed
  • Ruff lint and formatting: passed
  • Fresh GitHub CI: all 20 checks passed
  • Backend, frontend, rule validation, pip-audit, Trivy, CodeQL, Semgrep OSS, Semgrep Cloud, DCO, and CI Summary are green

Please complete the final review when available.

@m-khan-97 m-khan-97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, Ritik. I verified the final rebased head against current dev. Both remaining evidence-state fixes are preserved exactly as requested: AZ-NET-023 only treats the known unsafe Alert/Off values as confirmed findings, and AZ-NET-027 no longer assumes a missing or future rule state is enabled. The regression coverage remains in place, the branch is based on the merged severity contract, DCO is valid, and all 20 required checks are green. Approving from my side.

@m-khan-97

Copy link
Copy Markdown
Collaborator

@ritiksah141, a timing note: I completed the promised approval after verifying 6f127d4, but #230 merged into dev immediately afterward as dbbe0772. GitHub therefore now reports this branch one commit behind again. The new base commit is documentation/statistics automation and does not overlap the perimeter implementation, so my source approval stands; however, branch protection may still require an up-to-date head. Please use the repository’s update/rebase flow once more and let CI rerun before merge.

@TFT444 TFT444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

good to go

@TFT444
TFT444 merged commit df4e582 into openshield-org:dev Aug 29, 2026
20 checks passed
parthrohit22 added a commit to parthrohit22/openshield that referenced this pull request Aug 29, 2026
…Z-SECOPS-010

The dev merge into this branch pulled in openshield-org#277 (enterprise network and
perimeter controls) and openshield-org#320, both merged since this branch was last
updated. openshield-org#277's ten new AZ-NET-018..027 rules and the previously-merged
AZ-SECOPS-010 carry compliance mappings across all four framework
files with no mapping_type/evidence_type/primary_source/rationale/
review_status - this PR's own validate_mapping_pack.py correctly
rejects that, since neither rule existed yet when this PR's evidence-
schema requirement was written. Same root cause as the AZ-CMP-007 gap
from openshield-org#307 fixed earlier in this branch's history.

Filled in following the exact conventions of each entry's nearest
sibling in the same file:
- cis_azure_benchmark.json: mapping_type "not_applicable" for the
  N/A-* control_ids (all ten AZ-NET-0xx entries), "direct" for
  AZ-SECOPS-010's real numbered control (2.1.20) - matching this PR's
  existing framework-level-default convention for non-N/A CIS entries.
- nist_csf.json / iso27001.json / soc2.json: mapping_type "supporting",
  evidence_type "automated_configuration_scan", with primary_source/
  rationale text following the exact template of each file's sibling
  entries (e.g. AZ-NET-016, AZ-SECOPS-009).
- owner: null, review_status: "pending_review", review_date: null,
  matching every other unreviewed entry in these files.

Also cleans up a merge artifact in CHANGELOG.md: openshield-org#277 had added a
second, malformed top-level "## Unreleased" section (missing brackets,
no ### subsections) above the file's existing well-formed
"## [Unreleased]" - folded its one entry into the correct section.

Verified: mapping-pack validator clean (0 errors, was 55). Full
backend suite (892 passed, 5 skipped - pre-existing/environment-only).
alembic heads still resolves to exactly one head (3a76ff935bf6).
ruff check and format --check clean. Diff confirmed scoped to exactly
the 11 affected rule_ids across the 4 framework files plus the
CHANGELOG cleanup - no other entries touched.

Signed-off-by: Parth J Rohit <parthrohit60@gmail.com>
Signed-off-by: parthrohit22 <parthrohit60@gmail.com>
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.

Implement remaining enterprise network and perimeter security rules

4 participants