diff --git a/README.md b/README.md index 1148700..aaa596a 100644 --- a/README.md +++ b/README.md @@ -231,7 +231,7 @@ cd test/functional-tests | Single successful upload (TC-081) | `upload.feature` | ✅ Complete | | Upload retry (TC-082/083) | `upload_retry.feature` | ✅ Complete | -**Coverage**: 66 / 66 applicable TCs implemented (19 not applicable — see `L2_TESTS.md`) +**Coverage**: 66 / 66 applicable TCs implemented (19 not applicable — see `test/L2_TESTS.md`) ## Rate Limiting & Recovery Mode diff --git a/c_sourcecode/src/upload/upload.c b/c_sourcecode/src/upload/upload.c index 34c9154..743edce 100644 --- a/c_sourcecode/src/upload/upload.c +++ b/c_sourcecode/src/upload/upload.c @@ -330,7 +330,7 @@ int upload_file(const char *filepath, const char *url, const char *dump_name, co } else { - CRASHUPLOAD_INFO("S3 %s Upload is successful\n", filepath); + CRASHUPLOAD_INFO("Upload is successful with TLS1.2 for %s\n", filepath); if (t2_enabled) { t2CountNotify("SYS_INFO_S3CoreUploaded", 1); diff --git a/openspec/config.yaml b/openspec/config.yaml new file mode 100644 index 0000000..38557b7 --- /dev/null +++ b/openspec/config.yaml @@ -0,0 +1,43 @@ +schema: spec-driven + +context: | + Repo: crashupload, an RDK crash dump discovery, packaging, and upload system with two active implementation paths: legacy shell scripts and a newer C binary. + + Use this context strategy to keep SDLC artifact generation accurate and token-efficient: + - Read OpenSpec docs first: openspec/project.md, openspec/runtime/**, openspec/pipeline/**, openspec/subsystems/**, openspec/integrations/**, openspec/diagrams/**, openspec/gaps/**. + - Read documentation second: README.md, c_sourcecode/README.md, unittest/README.md, docs/migration/**, and other repo README files relevant to the touched area. + - Read implementation third, but only the narrow slice needed for the task: uploadDumps.sh, runDumpUpload.sh, uploadDumpsUtils.sh, c_sourcecode/src/**, c_sourcecode/include/**, c_sourcecode/common/**, src/inotify-minidump-watcher.c, and related service files. + - Read verification and delivery context only when relevant: run_ut.sh, run_l2.sh, cov_build.sh, rdk_build.sh, test/L2_TESTS.md, unittest/**, test/functional-tests/**, and .github/workflows/*.yml. + + Repository reality to preserve in artifacts: + - Runtime is event/timer/script driven, not a single long-running daemon. + - uploadDumps.sh is the top-level dispatcher. + - mediaclient is binary-first when /usr/bin/crashupload is present; broadband and extender still rely on the legacy shell path. + - C implementation lives under c_sourcecode/ and legacy shell behavior remains authoritative for some device types. + - L1 means unit-test coverage centered on unittest/ and ./run_ut.sh. + - L2 means functional coverage centered on test/functional-tests/, test/L2_TESTS.md, and ./run_l2.sh after ./cov_build.sh --l2-test. + - CI context comes from .github/workflows/*.yml plus build/test entry scripts at repo root. + + Context minimization rules: + - Prefer the existing OpenSpec artifacts as the summary layer instead of re-reading large code trees. + - Pull only the docs and code slices directly related to the subsystem being changed. + - Do not load all docs or all source files by default; start with the nearest README or OpenSpec doc, then drill into exact code paths. + - Separate shell-path behavior, C-path behavior, test impact, and CI impact instead of merging them into a generic summary. + +rules: + proposal: + - Start from openspec/project.md and only add the specific OpenSpec/doc/code files needed for the affected subsystem. + - State whether the change touches shell runtime, C runtime, test harness, CI, documentation, or a combination. + - Keep repository background concise; prefer explicit file anchors over broad repo summaries. + design: + - Prefer openspec/runtime, openspec/pipeline, openspec/subsystems, and openspec/diagrams before raw implementation files. + - Pull docs/migration/** and relevant README files when they add architecture, deployment, or migration detail. + - Call out device-type routing differences and any shell-versus-C divergence. + specs: + - Use code-truth for behavioral claims: uploadDumps.sh, runDumpUpload.sh, c_sourcecode/src/**, service units, and workflows when behavior depends on them. + - Separate L1 unit-test expectations from L2 functional-test expectations. + - Mention CI behavior only if workflows, build scripts, or test scripts are affected. + tasks: + - Generate file-scoped, validation-oriented tasks. + - Reference the smallest sufficient context set for each task. + - Include concrete verification commands when applicable, choosing from ./cov_build.sh, ./run_ut.sh, ./cov_build.sh --l2-test, and ./run_l2.sh. diff --git a/openspec/integrations/client-integration-model.md b/openspec/integrations/client-integration-model.md new file mode 100644 index 0000000..865a370 --- /dev/null +++ b/openspec/integrations/client-integration-model.md @@ -0,0 +1,76 @@ +# Client and Integration Model + +## External Interaction Surfaces + +## 1) CLI / command interfaces +### uploadDumps.sh +- Primary external entry for services and helper watchers. +- Accepts positional arguments passed through to selected runtime path. + +### crashupload binary +- Fact: current main expects positional flags where argv[2] determines dump mode (0 minidump, 1 coredump) and optional argv[3]=secure, argv[4]=wait_for_lock. +- Inference: practical CLI contract is script-oriented rather than end-user friendly options. + +### inotify-minidump-watcher +- Usage includes directory, command, args, and file patterns. +- Executes configured command when matching file is created. + +## 2) System integration points +- systemd units and timer/path triggers. +- Filesystem directories as crash intake source and local spool. +- Marker/flag files in /tmp and /opt governing policy and control flow. + +## 3) Library/API integrations +- Property/config: getDevicePropertyData, getIncludePropertyData, GetPartnerId, GetModelNum, etc. +- Policy/config APIs: RFC parameter read/write, RBUS privacy parameter reads. +- Telemetry: T2 events for counters and split values. +- Upload/security: uploadutil and TLS/curl stack. + +## Request Lifecycle: Trigger to Completion +1. Trigger source invokes uploadDumps.sh. +2. Dispatcher selects C or legacy path. +3. Runtime initializes config/platform and lock. +4. Runtime verifies eligibility (dumps present, deferral/privacy/reboot/rate-limit rules). +5. Runtime discovers and normalizes artifacts. +6. Runtime archives payload with metadata/log context. +7. Runtime performs upload workflow. +8. Runtime records outcome markers and cleanup. +9. Runtime exits with status consumed by invoker policy. + +## Intended Caller Workflows +### Systemd boot workflow +- Timer fires after boot window, invoking minidump processing path via uploadDumps.sh. + +### Filesystem event workflow +- Path/inotify detects dump creation, invokes upload script. + +### Manual operator workflow +- invoke script or binary for diagnostics/retry scenarios. + +## Configuration and Environment Dependencies +- Files: + - /etc/device.properties + - /etc/include.properties + - /etc/breakpad-logmapper.conf +- Runtime dirs: + - /opt/minidumps, /minidumps, /var/lib/systemd/coredump, /opt/secure/* +- Runtime flags/markers: + - /tmp/.uploadMinidumps, /tmp/.uploadCoredumps + - /tmp/.minidump_upload_timestamps, /tmp/.deny_dump_uploads_till + - /tmp/set_crash_reboot_flag + - /opt/.upload_on_startup + +## Rate-Limit State Semantics +- /tmp/.deny_dump_uploads_till: global cooldown gate; if active, upload processing is blocked regardless of dump type. +- /tmp/.minidump_upload_timestamps: minidump-only rolling counter used to trigger cooldown when upload volume exceeds threshold. + +## Device-Side vs External Integration Boundaries +- Device-side only: + - file scanning, archive composition, local policy gates, cleanup. +- Externally integrated: + - RFC/RBUS data sources, telemetry backplane, crash portal endpoint/signed URL service, S3 upload endpoint. + +## Integration Risks for OpenSpec Planning +- Fact: behavior differs strongly by device type and runtime branch. +- Fact: some integration paths are stubs or unsupported in specific branches (for example broadband in upload.c). +- Inference: future changes should specify per-device/path applicability and rollback strategy. diff --git a/openspec/runtime/execution-model.md b/openspec/runtime/execution-model.md new file mode 100644 index 0000000..5828c48 --- /dev/null +++ b/openspec/runtime/execution-model.md @@ -0,0 +1,94 @@ +# Runtime Execution Model + +## Execution Modes +### Mode 1: Mediaclient preferred C path +- Fact: uploadDumps.sh selects crashupload binary if present. +- Fact: binary may fall back to legacy via script if exit code indicates failure and dispatcher chooses fallback. + +### Mode 2: Broadband/extender legacy path +- Fact: uploadDumps.sh directly invokes runDumpUpload.sh for broadband/extender. +- Fact: upload.c currently marks broadband branch unsupported for C upload_process. + +### Mode 3: Manual/diagnostic invocation +- Inference: crashupload can be executed directly with argument contract expected by main (argv[2] dump type flag; optional secure/wait_for_lock flags). +- Unknown: direct CLI contract stability across releases. + +## Detailed C Binary Flow (Observed) +1. Preconditions and argument check +- argc must be >= 3. +- argv[2] determines dump type branch: 0 minidump, 1 coredump. + +2. Locking and signal behavior +- lock file path set by dump type. +- SIGTERM handler removes lock file based on active mode. +- lock mode optional wait_for_lock causes blocking lock behavior. + +3. Initialization +- t2 init, config load, platform init, core log creation. + +4. Prerequisite gate +- dump presence check in expected directory. +- mediaclient deferral to 480 seconds uptime threshold. +- optional coredump completion wait logic tied to mutex flag file. + +5. Privacy gate +- mediaclient reads RBUS privacy mode. +- DO_NOT_SHARE skips upload, but cleanup path still executes. + +6. Startup cleanup and scan +- cleanup_batch runs before and after processing. +- scanner finds candidate files and ensures size stability. + +7. Per-dump processing +- sanitize/rename/telemetry extraction. +- metadata naming with mac/date/box/model/process context. +- archive creation with selected sidecar files. + +8. Rate limit and reboot checks +- skip upload if reboot flag present. +- global deny-window (`/tmp/.deny_dump_uploads_till`) is evaluated before dump-type specific checks and can block both minidump and coredump uploads while active. +- minidump upload-counter check (`/tmp/.minidump_upload_timestamps`) is only evaluated for minidump runs. +- when blocked, pending dumps in the active working directory can be removed. + +9. Upload loop +- sequential upload per archive. +- break on first upload error. + +10. Cleanup and termination +- cleanup batch rerun. +- release lock, uninit telemetry, exit with ret code. + +## Concurrency and Process Model +- Fact: single-process, mostly sequential processing. +- Fact: no explicit multithreading in processing path. +- Fact: inter-process concurrency controlled by flock lock files. +- Inference: throughput scales by trigger frequency and batch size rather than parallel upload workers. + +## State and File Semantics +### Locking +- /tmp/.uploadMinidumps +- /tmp/.uploadCoredumps + +### Rate limit and deny windows +- /tmp/.minidump_upload_timestamps +- /tmp/.deny_dump_uploads_till +- policy note: deny-window is global cooldown; minidump timestamp file is minidump-only counter state. + +### Startup/cleanup markers +- /tmp/.on_startup_dumps_cleaned_up_ +- /opt/.upload_on_startup + +### Reboot/flow flags +- /tmp/set_crash_reboot_flag +- /tmp/coredump_mutex_release (checked in prerequisites path) + +## Failure Handling Semantics +- Fact: lock contention in non-wait mode exits quickly. +- Fact: scan failures/no dumps route to cleanup and often zero exit. +- Fact: upload failure breaks remaining upload loop. +- Fact: cleanup is best-effort and often non-fatal. +- Unknown: upstream service restart policies and retry cadence on non-zero exits. + +## Execution Truth vs Expectations +- Fact: several comments indicate optimized 7-step conceptual flow, but implementation includes TODOs and mixed maturity. +- Inference: OpenSpec work should baseline on observed behavior rather than intended design commentary. diff --git a/openspec/subsystems/subsystem-analysis.md b/openspec/subsystems/subsystem-analysis.md new file mode 100644 index 0000000..4ef130e --- /dev/null +++ b/openspec/subsystems/subsystem-analysis.md @@ -0,0 +1,83 @@ +# Subsystem Responsibility Analysis + +## Subsystem Map +1. Trigger and orchestration subsystem + - systemd units, timer/path, optional inotify watcher, uploadDumps.sh dispatcher. + +2. Initialization and config subsystem + - system_init, config_manager, platform. + +3. Intake and qualification subsystem + - prerequisites, lock_manager, scanner. + +4. Packaging subsystem + - archive, file utilities, log mapping in scanner. + +5. Transport subsystem + - upload module with endpoint discovery, metadata POST, S3 PUT. + +6. Policy subsystem + - ratelimit, privacy mode check, reboot flag behavior. + +7. Operations and observability subsystem + - logger and telemetry interface wrappers. + +## Subsystem A: Trigger and orchestration +- Facts: + - systemd timer and path units invoke uploadDumps.sh. + - uploadDumps.sh chooses C vs legacy runtime branch and fallback. +- Inferences: + - This layer is the primary operational policy switch for migration strategy. +- Unknowns: + - exact service installation/enabling matrix per platform SKU. + +## Subsystem B: Initialization and config +- Facts: + - config_manager derives mode from argv and device properties. + - platform subsystem builds identifier set (mac/model/sha1) with fallbacks. +- Risks: + - mixed defaults and TODOs can hide platform-specific behavior gaps. + +## Subsystem C: Intake and qualification +- Facts: + - lock files ensure single active worker per dump type. + - prerequisites include dump-presence check and optional deferred upload. + - scanner enforces max file count and file-size stability gate. +- Inference: + - stability gate mitigates racing with producer write completion. + +## Subsystem D: Packaging +- Facts: + - archive_create_smart creates tar.gz and can include mapped process logs. + - filename normalization logic handles long names and container delimiter cleanup. +- Unknown: + - full fidelity vs legacy script for all edge-case filename conventions. + +## Subsystem E: Transport +- Facts: + - endpoint resolution path includes RFC and device property fallback. + - upload is two-step (metadata then object upload) with telemetry markers. +- Inference: + - control plane and data plane are split, enabling signed URL workflows. + +## Subsystem F: Policy (rate limit/privacy/reboot) +- Facts: + - deny-window and upload timestamp files in /tmp govern upload throttling. + - deny-window (`/tmp/.deny_dump_uploads_till`) is a global cooldown gate and can block both minidump and coredump uploads while active. + - upload timestamp counter (`/tmp/.minidump_upload_timestamps`) is only used for minidump rate-threshold evaluation. + - reboot flag can skip upload to avoid conflict during reboot transitions. + - privacy DO_NOT_SHARE bypasses upload for mediaclient path. +- Assumption: + - product-level privacy compliance also depends on paths outside this component. + +## Subsystem G: Observability +- Facts: + - component emits logs and T2 counters/values. + - telemetry calls are feature-flag/build-flag dependent wrappers. +- Unknown: + - production log routing and marker governance ownership. + +## Cross-Cutting Concerns +- Device type branching significantly changes behavior. +- Build flags (RFC_API_ENABLED, RBUS_API_ENABLED, T2_EVENT_ENABLED, L2_TEST) alter effective runtime semantics. +- Legacy and C implementations coexist and must be jointly reasoned during change planning. diff --git a/L2_TESTS.md b/test/L2_TESTS.md similarity index 97% rename from L2_TESTS.md rename to test/L2_TESTS.md index b67e4e6..631081a 100644 --- a/L2_TESTS.md +++ b/test/L2_TESTS.md @@ -80,8 +80,8 @@ sh cov_build.sh --l2-test | TC-047 | Upload-on-startup mode (minidump-on-bootup) | Cleanup | ⚠️ Partial | Upload-on-startup flow exists in `main.c` via `minidump-on-bootup-upload.service`; exact behaviour may differ from shell on-startup path | ✅ Implemented | `test_cleanup_batch.py` :: `test_upload_on_startup_flag_removed_for_coredump_mode` | | TC-048 | Upload count ≤ 10 → ALLOW_UPLOAD | Rate Limiting | ✅ Yes | `is_upload_limit_reached()` in `ratelimit.c` counts timestamp file lines; ≤ 10 entries → returns `ALLOW_UPLOAD` | ✅ Implemented | `test_ratelimit_allow.py` :: `test_upload_allowed_when_count_at_or_below_limit` | | TC-049 | Upload count > 10 within window → STOP_UPLOAD | Rate Limiting | ✅ Yes | > 10 lines within `RECOVERY_DELAY_SECONDS` window → `is_upload_limit_reached()` returns `STOP_UPLOAD` | ✅ Implemented | `test_ratelimit.py` :: `test_upload_blocked_when_count_exceeds_10` | -| TC-050 | Rate limiting applied to minidump path only | Rate Limiting | ✅ Yes | `ratelimit_check_unified()` called in `main.c` only for `DUMP_TYPE_MINIDUMP` branch; coredump path skips rate limiting | ✅ Implemented | `test_ratelimit_allow.py` :: `test_coredump_not_rate_limited_by_minidump_counter` | -| TC-051 | Recovery time not yet reached → uploads still blocked | Rate Limiting | ✅ Yes | `is_recovery_time_reached()` reads deny-till timestamp from `/tmp/.deny_dump_uploads_till`; still inside window → blocked | ✅ Implemented | `test_ratelimit.py` :: `test_upload_blocked_when_deny_file_active` | +| TC-050 | Minidump counter check is minidump-only | Rate Limiting | ✅ Yes | `is_upload_limit_reached()` reads `/tmp/.minidump_upload_timestamps` only for `DUMP_TYPE_MINIDUMP`; coredump path bypasses this counter check when deny-window is not active | ✅ Implemented | `test_ratelimit_allow.py` :: `test_coredump_not_rate_limited_by_minidump_counter` | +| TC-051 | Recovery time not yet reached → global cooldown blocks uploads | Rate Limiting | ✅ Yes | `is_recovery_time_reached()` reads deny-till timestamp from `/tmp/.deny_dump_uploads_till`; if still inside window, both minidump and coredump runs are blocked | ✅ Implemented | `test_ratelimit.py` :: `test_upload_blocked_when_deny_file_active`, `ratelimit_gtest.cpp` :: `RatelimitCheckUnified_CoredumpType_RecoveryWindow_BlockUpload` | | TC-052 | Recovery time reached → uploads unblocked | Rate Limiting | ✅ Yes | `is_recovery_time_reached()` returns `true` after window expires → uploads resume normally | ✅ Implemented | `test_ratelimit_allow.py` :: `test_recovery_time_expired_unblocks_upload` | | TC-053 | Timestamp written to rate limit log after upload | Rate Limiting | ✅ Yes | `set_time()` in `ratelimit.c` appends current timestamp entry to rate limit log file | ✅ Implemented | `test_ratelimit_allow.py` :: `test_no_deny_file_allows_upload_to_proceed` | | TC-054 | Rate limit counter resets after recovery period | Rate Limiting | ✅ Yes | Timestamps older than `RECOVERY_DELAY_SECONDS` are not counted; counter effectively resets after recovery period | ✅ Implemented | `test_ratelimit_allow.py` :: `test_rate_limit_resets_after_recovery_period` | diff --git a/L2_TODO.md b/test/L2_TODO.md similarity index 99% rename from L2_TODO.md rename to test/L2_TODO.md index 34d2a8c..8897afe 100644 --- a/L2_TODO.md +++ b/test/L2_TODO.md @@ -48,7 +48,7 @@ Tracks which applicable TCs still need L2 functional tests. | TC-047 | Upload-on-startup mode — `/opt/.upload_on_startup` removed on coredump run | | TC-048 | Upload count ≤ 10 → ALLOW_UPLOAD | | TC-049 | Upload count > 10 within window → STOP_UPLOAD | -| TC-050 | Rate limiting applied to minidump path only | +| TC-050 | Minidump counter check is minidump-only | | TC-051 | Recovery time not yet reached → uploads still blocked | | TC-052 | Recovery time expired → uploads unblocked | | TC-054 | Rate-limit counter reset after recovery period | diff --git a/test/functional-tests/features/ratelimit.feature b/test/functional-tests/features/ratelimit.feature index 37f2131..53b0112 100644 --- a/test/functional-tests/features/ratelimit.feature +++ b/test/functional-tests/features/ratelimit.feature @@ -23,7 +23,7 @@ Feature: Rate limiting — allow and block upload paths ratelimit_check_unified() enforces two independent limits: 1. Deny-window check (is_recovery_time_reached): reads /tmp/.deny_dump_uploads_till. 2. Per-minidump upload-count check (is_upload_limit_reached): reads - /tmp/.minidump_upload_timestamps (minidump path only). + /tmp/.minidump_upload_timestamps (counter check only for minidump mode). When either check returns BLOCK, remove_pending_dumps() cleans the working dir. Background: @@ -53,8 +53,9 @@ Feature: Rate limiting — allow and block upload paths And the binary exits with return code 0 # TC-050 - Scenario: Rate-limit upload-count check is skipped for coredump mode + Scenario: Rate-limit upload-count check is skipped for coredump mode when deny-window is inactive Given /tmp/.minidump_upload_timestamps has 11 lines (would block a minidump run) + And /tmp/.deny_dump_uploads_till does not exist When the binary runs in coredump mode Then the upload-count else-branch returns ALLOW_UPLOAD immediately And /tmp/.deny_dump_uploads_till is NOT created by the rate limiter diff --git a/test/functional-tests/tests/README.md b/test/functional-tests/tests/README.md index 5e485ed..519a8cd 100644 --- a/test/functional-tests/tests/README.md +++ b/test/functional-tests/tests/README.md @@ -1,8 +1,8 @@ # Functional Test Implementations This directory contains the pytest-based L2 functional test suite for the -crasupload C binary. Each `.py` file exercises one or more TC-IDs from -[uploadDumps_TestCases.md](../../../uploadDumps_TestCases.md) by running the +crashupload C binary. Each `.py` file exercises one or more TC-IDs from +[uploadDumps_TestCases.md](../../uploadDumps_TestCases.md) by running the compiled binary directly against a set of controlled filesystem conditions. Gherkin-format specifications for all tests are in [../features/](../features/). @@ -119,4 +119,4 @@ The `CRASHUPLOAD_BINARY` environment variable overrides the path. - **Implemented**: 66 / 66 (100%) - **Not applicable** (shell-only / unimplemented stubs): 19 -See [../../../L2_TESTS.md](../../../L2_TESTS.md) for the full applicability matrix. +See [../../L2_TESTS.md](../../L2_TESTS.md) for the full applicability matrix. diff --git a/test/functional-tests/tests/test_ratelimit_allow.py b/test/functional-tests/tests/test_ratelimit_allow.py index 38cd19a..1871884 100644 --- a/test/functional-tests/tests/test_ratelimit_allow.py +++ b/test/functional-tests/tests/test_ratelimit_allow.py @@ -24,8 +24,10 @@ ---------- `ratelimit_check_unified()` in `ratelimit.c` enforces two checks in sequence: - Step 1 — deny-window check (is_recovery_time_reached): - Reads `/tmp/.deny_dump_uploads_till`. If absent OR expired → ALLOW_UPLOAD. + Step 1 — deny-window check (is_recovery_time_reached): + Reads `/tmp/.deny_dump_uploads_till`. + • If absent OR expired → ALLOW_UPLOAD + • If active (future timestamp) → STOP_UPLOAD for both minidump and coredump Step 2 — per-minidump count check (is_upload_limit_reached): Reads `/tmp/.minidump_upload_timestamps`. @@ -49,11 +51,11 @@ (boundary condition: the rate-limiter should NOT trigger at 10 or fewer). is_upload_limit_reached() returns ALLOW_UPLOAD → rate limit not triggered. -TC-050 Rate limiting applied to minidump path only - ──────────────────────────────────────────────────── +TC-050 Minidump upload-count check is minidump-only + ───────────────────────────────────────────────────── Even when `/tmp/.minidump_upload_timestamps` has 11 lines (which would block - a minidump run), a coredump run (argv[2]="1") skips the count check entirely - inside ratelimit_check_unified() → ALLOW_UPLOAD returned → not blocked. + a minidump run), a coredump run (argv[2]="1") skips the count check entirely + inside ratelimit_check_unified() and is allowed when no deny-window is active. """ import os @@ -186,8 +188,8 @@ def test_coredump_not_rate_limited_by_minidump_counter( and sets `status = ALLOW_UPLOAD` without reading MINIDUMP_TIMESTAMPS_FILE. A minidump run with the same precondition (11 entries, within window) - WOULD be blocked (TC-049 verifies this). This test confirms the - symmetry: coredump ignores the minidump counters completely. + WOULD be blocked (TC-049 verifies this). This test confirms that + coredump ignores the minidump counter file itself. Setup: • DENY_UPLOADS_FILE absent — deny-window check passes. @@ -198,8 +200,9 @@ def test_coredump_not_rate_limited_by_minidump_counter( Assertions: • exit(0) - • DENY_UPLOADS_FILE was NOT created by the rate limiter. - (For coredump ALLOW_UPLOAD path, set_time() is never called.) + • DENY_UPLOADS_FILE was NOT created by the rate limiter. + (This case validates the minidump counter bypass when deny-window + is absent; active deny-window behavior is validated separately.) """ _ensure_system_init_prereqs() os.makedirs(SECURE_COREDUMP_PATH, exist_ok=True) @@ -227,11 +230,12 @@ def test_coredump_not_rate_limited_by_minidump_counter( f"counter, got {result.returncode}\n" f"stdout={result.stdout}\nstderr={result.stderr}" ) - # Rate limiter must NOT have fired for coredump + # In this no-deny-window setup, counter-based rate limiting must not + # fire for coredump mode. assert not os.path.exists(DENY_UPLOADS_FILE), ( "TC-050: DENY_UPLOADS_FILE was created for a coredump run — " - "ratelimit_check_unified(COREDUMP) should return ALLOW_UPLOAD " - "regardless of minidump timestamp count" + "ratelimit_check_unified(COREDUMP) should bypass the minidump " + "counter check when deny-window is absent" ) finally: Path(dump_path).unlink(missing_ok=True) diff --git a/uploadDumps_TestCases.md b/test/uploadDumps_TestCases.md similarity index 100% rename from uploadDumps_TestCases.md rename to test/uploadDumps_TestCases.md diff --git a/unittest/ratelimit_gtest.cpp b/unittest/ratelimit_gtest.cpp index 108ed0e..3dc3ab3 100644 --- a/unittest/ratelimit_gtest.cpp +++ b/unittest/ratelimit_gtest.cpp @@ -298,7 +298,7 @@ TEST_F(RateLimitTest, RatelimitCheckUnified_NoFiles_AllowUpload) { TEST_F(RateLimitTest, RatelimitCheckUnified_RecoveryNotReached_BlockUpload) { // Set recovery time in future - time_t future = time(NULL) + 600; + long future = (long)time(NULL) + 600L; CreateDenyFile("/tmp/.deny_dump_uploads_till", future); int ret = ratelimit_check_unified(DUMP_TYPE_MINIDUMP); @@ -317,12 +317,22 @@ TEST_F(RateLimitTest, RatelimitCheckUnified_UploadLimitReached_BlockUpload) { } TEST_F(RateLimitTest, RatelimitCheckUnified_CoredumpType_AllowUpload) { - // Coredump should always allow (no rate limiting) + // Coredump is allowed when no global deny-window is active. int ret = ratelimit_check_unified(DUMP_TYPE_COREDUMP); EXPECT_EQ(ret, ALLOW_UPLOAD); } +TEST_F(RateLimitTest, RatelimitCheckUnified_CoredumpType_RecoveryWindow_BlockUpload) { + // The recovery window is global, so coredumps are blocked while it is active. + long future = (long)time(NULL) + 600L; + CreateDenyFile("/tmp/.deny_dump_uploads_till", future); + + int ret = ratelimit_check_unified(DUMP_TYPE_COREDUMP); + + EXPECT_EQ(ret, RATELIMIT_BLOCK); +} + TEST_F(RateLimitTest, RatelimitCheckUnified_BothChecksPassed_AllowUpload) { // Recovery time passed and upload limit not reached CreateTimestampFile("/tmp/.minidump_upload_timestamps", 5);