The counts needs to be modified manually since we're not running
triggers during migrations. The schema version needed to be bumped.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
While preparing the command, attempt to verify that the template
expansion happens in a way that will result in a non-shell-injection
command. I don't presume to say that this is a 100% prevention, and the
script itself can always do dumb shit with the file path later.
Nonetheless, we should make a best-effort attempt.
Equally, this could generate false positives for commands that are
strangely written but in fact safe. I think this is acceptable; external
versioning is currently used by approximately 0.02% of users, and
presumably most of them have a setup that is sane.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Verify that block requests have a hash and that it's correct. This helps
prevent certain races and ensure that only expected data is ever
returned in response to a request.
(In Syncthing prior to 1.28.1 the block hash was omitted for encrypted
requests from trusted devices. This breaks compatibility with that
specific config on those versions.)
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Ensure file was a file before the shortcut as well as after... (This was
implied when talking to a correct implementation, but not enforced.)
Make our file opening operations safe by default by ensuring the last
path component is not a symlink.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
We had a few places where we had perhaps too much of an opinion on the
permissions on created files and directories, sometimes fuled by a
misconception about how permissions work in both Unix and Windows. Recap
on the ground rules:
- On all unixes, all file & directory creation (`Mkdir`, `MkdirAll`,
`Create`, `WriteFile`, `Open`) has the given permission bits filtered
via the user's umask. The proper permissions for us to use are in almost
all cases 0o666 for files and 0o777 for directories, strange as that may
look at the call site.
- On Windows, there is no umask but in turn all of the permission bits
except the user write bit are ignored. The absence of user write bit is
converted into the read only attribute. This means that what is proper
for Unix above is also proper for Windows.
- We make an exception when creating files for certificate keys and the
config / database directories, as those contain secrets we think should remain closed
even if the user generally collaborates with other users on the system.
(Also removal of a bugfixed copy of MkdirAll for Windows that hasn't
been necessary for a few years.)
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
- Allow zero-sized requests since they are sent by all current versions
of Syncthing.
- Stop sending zero-sized requests since that's stupid.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
lib/ur brings in a lot of dependencies we don't need in e.g.
stcrashreceiver, who only needs the small failure reporting structs.
Make those part of the lean `contract` package instead.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
The runtime prints a lot of context for crashes due to bad pointers etc,
which is required to understand the crash, but this context comes before
the `fatal error: ...` line. Currently those lines get filtered out and
not included in the crash report. This change modifies the criteria so
that we start collecting crash data also at a line that begins with
`runtime:`, and tweaks the parsing later to look for the specific
`panic:` or `fatal error:` which may come later as the subject.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
This makes sure the user running Syncthing, and hence Synchting itself,
has read/write/execute on directories in .stversions. The other
permission bits remain copied from the source directory, ensuring
whatever group and other permissions were set remain in effect.
Closes#10695.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
The locking logic for upgrades got inverted in the lockfile changes. If
we got the lock it means Syncthing wasn't already running, so we can do
a direct upgrade. If we failed to get the lock it means Syncthing was
running and we should tell the REST interface to do the upgrade.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
* infrastructure:
fix(stcrashreceiver): allow extra pre/post data in version line
chore(stcrashreceiver): improve logging
chore(stdiscosrv): prewarm counters at startup
In the common case (sparse files enabled, not reusing old data) we'd
optimise away pulling & writing zero blocks. However in the corner cases
we'd go through the whole processing of pulling the block over the
network which is of course entirely unnecessary.
Now, instead, always take an optimised path for all-zeroes blocks. In
the clean case we do nothing, otherwise we materialise a block of zeroes
and write it directly.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
These headers should not be inspected when running with a TLS listener.
Additionally, we should really enable them individually instead of
trusting the proxy to filter out the unused variants, but baby steps.
Reported by multiple AI vuln scanners.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
This adds a new folder-level configuration `FullBlockIndex`. It controls
whether we maintain the block index for a given folder -- currently
that's always true, now it becomes possible to turn off. The block index
is used for lookup of blocks across files and folders. Effectively, when
syncing a change, for each block, we check:
1. Is the block already present in the old version of the file? If so,
we can reuse (copy) it without network transfer. **This check is always
possible.**
2. Is the block already present in any other file in this folder or
other folders? If so we can copy it. **This check is only possible with
the full block index.**
3. We must transfer the block over the network.
Maintaining the full block index is costly in time, I/O and database
size. With this PR, maintaining the full block index becomes the default
for send-receive and receive-only folders only, with it disabled for
send-only and receive-encrypted folders. The block index is never useful
for encrypted folders, as blocks are encrypted separate for each file.
It is also not useful for send-only folders by themselves, though the
data in the send-only folder could be reused by other receive-type
folders if it were enabled.
For very large folders it may make sense to disable the full block index
regardless of folder type and just accept the resulting decrease in data
reuse.
Disabling or enabling the option in the GUI causes the index to be
destroyed or rebuilt accordingly.
https://github.com/syncthing/docs/pull/1005
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Hopefully deflakes TestHTTPLogin on Windows, where it currently often
times out, presumably in the config saving stage after already having
started a shutdown of the API and being CPU constrained due to password
hashing.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
The test expected the stopped scanner to produce at most numHashers
additional results, but there's also the case where a directory is
encountered (which doesn't require hashing) and sent directly.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
There was a race condition where using IndexUpdate would trigger a pull,
which would sync the delete we are looking for, making the completion
100%. By doing the insert directly into the database we are not
triggering these things and get the expected completion percentage
always.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
These have been flaky for a long time, seemingly because the multiple
connection code slightly changed the timing of cluster config sending by
moving them to the connection promotion loop. This adds some resiliency
to that, instead of assuming that the CC:s will be immediately available
after adding the connection.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
* infrastructure:
chore(stdiscosrv): smooth retry-after delays over a slightly larger normal distribution
chore(stdiscosrv): optionally delay shutdown
chore(stdiscosrv): adjust desired seen and unseen rate separately
chore(sqlite): reduce max open connections, keep them open permanently (fixes#10592)
Reduces connection churn, possibly tickling concurrency bug on Windows.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
The glob in **/go.sum fails in some builds because there are a lot of files in ** due to things like the zig cache directory. We can be more specific. Also, avoid a huge build context sent to Docker for the container builds.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Due to a thinko, this optimisation was wildly incorrect and would read
to lack of block reuse when syncing files.
(We do not insert a blocklist per device, but only a single one. We
can't use the fact of whether the insert happened as a criteria for
inserting blocks.)
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Jakob Borg