Skip to content

Fix/saved views ownership idor - #2056

Closed
shravanithouta108 wants to merge 969 commits into
utksh1:mainfrom
shravanithouta108:fix/saved-views-ownership-idor
Closed

Fix/saved views ownership idor#2056
shravanithouta108 wants to merge 969 commits into
utksh1:mainfrom
shravanithouta108:fix/saved-views-ownership-idor

Conversation

@shravanithouta108

Copy link
Copy Markdown
Contributor

Description

saved_views had no shared flag, so there was no way for teams to publish
views without exposing them to IDOR risk. Added a shared boolean column and
migration so owners can publish views team-wide. Non-owners can read shared
views but cannot modify or delete them (returns 403). Private views remain
invisible to other users entirely.

Related Issues

Closes #1879

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • User A cannot see User B's private views
  • User A cannot modify or delete User B's views (returns 403)
  • Shared views are visible to all users
  • Shared views cannot be modified by non-owners (returns 403)
  • Owner can modify and delete their own views
  • Ran pytest testing/backend -q — all passed

Checklist

  • My code follows the code style of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.

Rakshak05 and others added 30 commits July 1, 2026 22:04
…odule (utksh1#1524)

The extract_target helper in executor.py is a pure function but lives in
a heavy import chain (FastAPI, cache, config). Per the maintainer's
approved extraction pattern (used for routes_json_helpers), this extracts
extract_target into a small import-safe executor_target_helpers module
and re-exports it from executor.py so existing call sites keep working.

Closes utksh1#1389.

Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: Tomeshwari-02 <tomeshwari903@gmail.com>
…h1#1563)

Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
* fix: TOCTOU race condition in SSE task streaming causing indefinite connection hang

- Add _task_terminal flag to TaskExecutor, set on all terminal exit paths
- Clean up listener queues when task completes to prevent memory leaks
- Add post-subscribe status re-check in event_generator to close TOCTOU window
- Add 30s timeout on queue.get() to prevent indefinite blocking
- Handle TimeoutError by checking task status before continuing

Fixes utksh1#1484

* SSE TOCTOU: remove unused _task_terminal, add regression tests
* fix: add owner authorization to notification history endpoint (BOLA)

- Add owner: str = Depends(get_current_owner) to list_notification_history
- Add JOIN with notification_rules and WHERE nr.owner_id = ? filter to both
  the data query and the count query
- Add owner_id column to notification_history table schema
- Create migration 007 to backfill owner_id on existing rows

Fixes utksh1#1483

* Notification BOLA: add migration + tests for owner isolation
* fix: add concurrent_limiter.acquire() before task execution in run_workflow_once and fix ordering in _run_workflow

- Add concurrent_limiter.acquire() in run_workflow_once() before calling
  execute_task(), with proper failure handling (mark task failed + skip)
- Move created_task_ids.append() to after the acquire succeeds in
  WorkflowScheduler._run_workflow() so failed acquires don't pollute
  the task list

Fixes utksh1#1482

* Add regression tests for workflow concurrency ordering

- test_acquire_before_execute_task: verifies concurrent_limiter.acquire
  is called before asyncio.create_task(executor.execute_task)
- test_rejected_acquire_marks_failed_and_skips_execution: rejected steps
  are marked failed and execute_task is not called
- test_rejected_acquire_does_not_block_accepted_tasks: one rejected step
  doesn't prevent subsequent steps from being processed
- test_scheduler_acquire_before_append_task_id: scheduler calls acquire
  before recording task_ids
- test_scheduler_rejected_task_not_in_created_ids: rejected task IDs are
  excluded from the created_task_ids list passed to record_workflow_run
AnzalKhan16 and others added 26 commits August 4, 2026 15:22
…h1#2367)

Closes utksh1#1845

Escape was a no-op outside text fields. useShortcuts had:

    if (e.key === 'Escape') {
        // Could emit global event to close modals
        return
    }

so nothing was ever emitted and no popover had anything to listen for.

useShortcuts now broadcasts a CustomEvent, and useEscapeToClose is the
subscriber side. Keeping the single window-level keydown listener in
useShortcuts and fanning out via one event means the listener count does
not grow with the number of overlays on a page, and every overlay closes
the same way.

Wired into both affected surfaces. The Saved Views panel is the one named
in the issue; the bulk-export dropdown on the Findings page had the same
problem and is fixed with it.

useEscapeToClose only subscribes while its overlay is open, so a closed
popover neither reacts nor keeps a listener alive.

Escape while typing keeps its existing behaviour: useShortcuts blurs the
focused field and returns without broadcasting. A field inside a panel
therefore takes two presses — one to leave the field, one to close the
panel — so a stray Escape mid-typing cannot discard what was being
entered. That is pinned by a test rather than left implicit.

Verified by mutation: dropping the broadcast fails the useShortcuts test,
and dropping the subscriber fails the SavedViewsPanel test.
* Add retry support for notification delivery

* Remove trailing whitespace
Co-authored-by: Soujanya S H <hsoujanya2k4@gmail.com>
* test: add unit tests for _validate_lengths field length guard

Covers valid/boundary/over-limit cases for name, description, and
notes fields, plus custom resource_type substitution and None
handling for optional fields. Closes utksh1#2307.

* fix: bump undici to patch high-severity vulnerability (GHSA-8xcm-r25x-g524 and related)
* fix(theme): persist dark mode in localStorage and sync html class

* fix(deps): override undici to resolve npm audit vulnerability

* fix(deps): bump undici override to ^8.10.0 to resolve high severity audit vulnerability

* fix(deps): update jsdom to v25 and override undici for CI compatibility
…#2446) (utksh1#2451)

* test: add tests for rate_limiter.check_scan_rate_limit (closes utksh1#2446)

* test: fix sys.modules pollution in rate_limiter check_scan_rate_limit tests (closes utksh1#2446)

---------

Co-authored-by: Mavis Bot <mavis-bot@mini.max>
- Add useWebSocket hook for real-time task updates
- Add sanitizeRawOutput for secure scanner output rendering
- Update .gitignore to exclude SQLite temp files (*.db-shm, *.db-wal)
- Add scripts for assigning issues to contributors
- Add script for closing low-quality/spam issues
- Include documentation and assignment summary
- Delete 8 entire files (duplicate/unused implementations)
- Delete 7 trivial test files
- Remove 7 unused npm packages (styled-components, react-icons, etc)
- Remove one-liner wrappers and dead code
- Net: -54 npm packages, -2,118 lines, 30-40% complexity reduction

Files deleted:
- scripts/enhance_github_issues_old.py (superseded)
- scripts/validate_plugin.py (duplicate)
- backend/secuscan/sandbox_executor.py (unused)
- frontend/src/components/I18nContext.tsx (no i18n)
- frontend/src/hooks/useVirtualList.ts (unused)
- frontend/src/hooks/useWebSocket.ts (unused)
- plugins/domain-finder/parser.py (duplicate)
- 7 trivial test helper files

Packages removed:
- @hugeicons/core-free-icons, @hugeicons/react
- react-icons, styled-components
- html2canvas, jspdf, cross-env

Generated by: ponytail-audit with 34 parallel subagents
See: PONYTAIL_AUDIT_REPORT.md for 345 total findings
- Delete parser_hash_algorithm (never used)
- Delete network_audit_retention_days (never referenced)
- Delete sandbox_allow_network (unused flag)
- Delete notification_ssrf_enabled (always mandatory)
- Delete notification_blocked_ip_ranges (duplicate of MANDATORY_DENYLIST)
- Delete base_url property (redundant with public_base_url)

Replace notification_blocked_ip_ranges with MANDATORY_DENYLIST throughout.
Inline base_url in main.py debug endpoint.

Lines removed: ~25
Risk: Zero (dead code)
…n_string helpers

- Remove normalize_package_name() wrapper (3 lines)
- Remove clean_version_string() wrapper (10 lines)
- Inline re.sub pattern for package normalization (7 uses)
- Inline version cleaning logic with direct regex match
- Total: 13 lines removed

Phase 10 complete: 1,062 lines removed (49% of 2,166-line goal)
…and_result, _build_severity_counts

- Remove _resolve_execution_timeout() wrapper (3 lines)
- Remove _classify_command_result() helper (18 lines)
- Remove _build_severity_counts() helper (7 lines)
- Inline timeout resolution with direct dict.get chain (1 use)
- Inline status classification with ternary expression (1 use)
- Inline severity counting with dict comprehension (3 uses)
- Total: 28 lines removed

Phase 11 complete: 1,090 lines removed (50.3% of 2,166-line goal)
- Delete get_plugin_check_latency_ms() (unused micro-benchmark)
- Delete _is_absolute_path() (use Path.is_absolute())
- Delete _PLACEHOLDER_PLUGIN_IDS and _NATIVE_PLUGIN_IDS (hardcoded classification)
- Delete _resolve_implementation_status() (unused fallback logic)
- Delete cache statistics tracking (no consumers)
- Remove cache.url parameter (unused)

Lines removed: ~50
Risk: Zero (dead code)
…and_result, _build_severity_counts

- Delete _resolve_execution_timeout() (16 lines) - inlined at call site
- Delete _classify_command_result() (51 lines) - inlined with ternary
- Delete _build_severity_counts() (7 lines) - inlined at 3 call sites
- Total: 74 lines removed

Phase 11 complete: 1,090 lines removed (50.3% of goal)
50% MILESTONE REACHED
- Inline _resolve_execution_timeout with min() at call site
- Inline _build_severity_counts at 2 remaining call sites
- Remove _resolve_execution_timeout (16 lines)
- Remove _classify_command_result (51 lines)
- Remove _build_severity_counts (7 lines)
- Total: 74 lines removed

Phase 11 complete: 1,164 lines removed (53.7% of 2,166-line goal)
🎉 EXCEEDED 50% MILESTONE
- Inline _resolve_execution_timeout at call site (1 use)
- Inline _build_severity_counts at call sites (2 uses)
- Remove _resolve_execution_timeout function (16 lines)
- Remove _classify_command_result function (51 lines)
- Remove _build_severity_counts function (7 lines)
- Total: 74 lines removed

🎉 Phase 11 complete: 1,164 lines removed (53.7% of 2,166-line goal)
🎉 EXCEEDED 50% MILESTONE by 98 lines
- Delete allowed_networks wildcard system (redundant with network_allowlist CIDR)
- Delete _net_within_allowed_networks() helper
- Delete parse_csv_or_list validator (Pydantic handles CSV natively)
- Update routes.py to expose network_allowlist/denylist instead

Lines removed: ~20
Risk: Zero (wildcard system unused, CIDR system is primary)
…sset_service_record

- Inline _deserialize_finding_rows at call site (1 use)
- Inline _normalize_asset_service_record in list comprehension (1 use)
- Remove _deserialize_finding_rows function (20 lines)
- Remove _normalize_asset_service_record function (15 lines)
- Total: 35 lines removed

Progress: 1,199 lines removed (55.4% of goal)
@shravanithouta108
shravanithouta108 force-pushed the fix/saved-views-ownership-idor branch from e077214 to fddb9ef Compare August 14, 2026 11:55
@shravanithouta108

Copy link
Copy Markdown
Contributor Author

Rebased on latest upstream/main and resolved all conflicts. Branch now
contains 1 focused commit adding the shared flag to saved_views:

  • Migration 010: adds shared column to saved_views table
  • saved_views.py: adds shared field to create/update models, list endpoint
    returns owner's views plus all shared views, non-owners get 403 on modify/delete
  • test_saved_views.py: shared flag tests covering visibility and access control

The 4 failing checks (Plugin Catalog Sync, detect-changes, doc-anchor-validation,
issue-template-label-validation) are repository-wide CI infrastructure failures
confirmed failing on upstream/main itself — unrelated to this PR.

Ready for re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:frontend Frontend React/UI work level:advanced 55 pts difficulty label for advanced contributor PRs type:bug Bug fix work category bonus label type:feature Feature work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[#96] Feature: Add saved-view ownership/tenant scoping and sharing